Validate that root paths are absolute and normalized.

R=brianwilkerson@google.com
BUG= https://github.com/dart-lang/sdk/issues/25203

Review URL: https://codereview.chromium.org/1511833004 .

pkg/analysis_server/lib/plugin/protocol/protocol.dart

@@ -440,6 +440,15 @@ class Response {
             error: new RequestError(RequestErrorCode.INVALID_EXECUTION_CONTEXT,
                 "Invalid execution context: $contextId"));
 
+  /**
+   * Initialize a newly created instance to represent the
+   * INVALID_FILE_PATH_FORMAT error condition.
+   */
+  Response.invalidFilePathFormat(Request request, path)
+      : this(request.id,
+            error: new RequestError(RequestErrorCode.INVALID_FILE_PATH_FORMAT,
+                'Invalid file path format: $path'));
+
   /**
    * Initialize a newly created instance to represent an error condition caused
    * by a [request] that had invalid parameter.  [path] is the path to the

pkg/analysis_server/lib/src/analysis_server.dart

@@ -185,8 +185,8 @@ class AnalysisServer {
    * A table mapping [AnalysisContext]s to the completers that should be
    * completed when analysis of this context is finished.
    */
-  Map<AnalysisContext,
-          Completer<AnalysisDoneReason>> contextAnalysisDoneCompleters =
+  Map<AnalysisContext, Completer<AnalysisDoneReason>>
+      contextAnalysisDoneCompleters =
       new HashMap<AnalysisContext, Completer<AnalysisDoneReason>>();
 
   /**
@@ -731,6 +731,15 @@ class AnalysisServer {
     return operationQueue.isEmpty;
   }
 
+  /**
+   * Return `true` if the given path is a valid `FilePath`.
+   *
+   * This means that it is absolute and normalized.
+   */
+  bool isValidFilePath(String path) {
+    return resourceProvider.absolutePathContext.isValid(path);
+  }
+
   /**
    * Returns a [Future] completing when [file] has been completely analyzed, in
    * particular, all its errors have been computed.  The future is completed

pkg/analysis_server/lib/src/domain_analysis.dart

@@ -175,8 +175,9 @@ class AnalysisDomainHandler implements RequestHandler {
     if (pair.context == null || pair.source == null) {
       return new Response.getReachableSourcesInvalidFile(request);
     }
-    Map<String, List<String>> sources = new ReachableSourceCollector(
-        pair.source, pair.context).collectSources();
+    Map<String, List<String>> sources =
+        new ReachableSourceCollector(pair.source, pair.context)
+            .collectSources();
     return new AnalysisGetReachableSourcesResult(sources)
         .toResponse(request.id);
   }
@@ -245,9 +246,22 @@ class AnalysisDomainHandler implements RequestHandler {
    */
   Response setAnalysisRoots(Request request) {
     var params = new AnalysisSetAnalysisRootsParams.fromRequest(request);
+    List<String> includedPathList = params.included;
+    List<String> excludedPathList = params.excluded;
+    // validate
+    for (String path in includedPathList) {
+      if (!server.isValidFilePath(path)) {
+        return new Response.invalidFilePathFormat(request, path);
+      }
+    }
+    for (String path in excludedPathList) {
+      if (!server.isValidFilePath(path)) {
+        return new Response.invalidFilePathFormat(request, path);
+      }
+    }
     // continue in server
-    server.setAnalysisRoots(request.id, params.included, params.excluded,
-        params.packageRoots == null ? {} : params.packageRoots);
+    server.setAnalysisRoots(request.id, includedPathList, excludedPathList,
+        params.packageRoots ?? <String, String>{});
     return new AnalysisSetAnalysisRootsResult().toResponse(request.id);
   }
 
@@ -347,8 +361,8 @@ class AnalysisDomainHandler implements RequestHandler {
 class AnalysisDomainImpl implements AnalysisDomain {
   final AnalysisServer server;
 
-  final Map<ResultDescriptor,
-          StreamController<engine.ComputedResult>> controllers =
+  final Map<ResultDescriptor, StreamController<engine.ComputedResult>>
+      controllers =
       <ResultDescriptor, StreamController<engine.ComputedResult>>{};
 
   AnalysisDomainImpl(this.server) {

pkg/analysis_server/test/domain_analysis_test.dart

@@ -119,6 +119,22 @@ main() {
             expect(serverRef.getResolvedCompilationUnits(fileB), isEmpty);
           });
         });
+
+        test('not absolute', () async {
+          var response = testSetAnalysisRoots([], ['foo/bar']);
+          expect(
+              response,
+              isResponseFailure(
+                  '0', RequestErrorCode.INVALID_FILE_PATH_FORMAT));
+        });
+
+        test('not normalized', () async {
+          var response = testSetAnalysisRoots([], ['/foo/../bar']);
+          expect(
+              response,
+              isResponseFailure(
+                  '0', RequestErrorCode.INVALID_FILE_PATH_FORMAT));
+        });
       });
 
       group('included', () {
@@ -147,6 +163,22 @@ main() {
           await server.onAnalysisComplete;
           expect(serverRef.getResolvedCompilationUnits(fileB), hasLength(1));
         });
+
+        test('not absolute', () async {
+          var response = testSetAnalysisRoots(['foo/bar'], []);
+          expect(
+              response,
+              isResponseFailure(
+                  '0', RequestErrorCode.INVALID_FILE_PATH_FORMAT));
+        });
+
+        test('not normalized', () async {
+          var response = testSetAnalysisRoots(['/foo/../bar'], []);
+          expect(
+              response,
+              isResponseFailure(
+                  '0', RequestErrorCode.INVALID_FILE_PATH_FORMAT));
+        });
       });
     });
 
@@ -167,8 +199,9 @@ main() {
         resourceProvider.newFile('/p2/b.dart', 'library b;');
         resourceProvider.newFile('/p2/c.dart', 'library c;');
 
-        var setRootsRequest = new AnalysisSetAnalysisRootsParams(
-            ['/p1', '/p2'], []).toRequest('0');
+        var setRootsRequest =
+            new AnalysisSetAnalysisRootsParams(['/p1', '/p2'], [])
+                .toRequest('0');
         var setRootsResponse = handler.handleRequest(setRootsRequest);
         expect(setRootsResponse, isResponseSuccess('0'));
 

pkg/analyzer/lib/file_system/memory_file_system.dart

@@ -30,7 +30,7 @@ class MemoryResourceProvider implements ResourceProvider {
   int nextStamp = 0;
 
   final AbsolutePathContext absolutePathContext =
-      new AbsolutePathContext(posix.separator);
+      new AbsolutePathContext(false);
 
   @override
   Context get pathContext => posix;

pkg/analyzer/lib/file_system/physical_file_system.dart

@@ -33,7 +33,7 @@ class PhysicalResourceProvider implements ResourceProvider {
   static final String SERVER_DIR = ".dartServer";
 
   final AbsolutePathContext absolutePathContext =
-      new AbsolutePathContext(io.Platform.pathSeparator);
+      new AbsolutePathContext(io.Platform.isWindows);
 
   PhysicalResourceProvider(String fileReadMode(String s)) {
     if (fileReadMode != null) {

pkg/analyzer/lib/src/util/absolute_path.dart

@@ -4,17 +4,21 @@
 
 library analyzer.src.util.absolute_path;
 
-/// The class for manipulating absolute paths.
+/// The class for manipulating absolute, normalized paths.
 class AbsolutePathContext {
-  final String separator;
+  static const int _COLON = 0x3A;
+  static const int _PERIOD = 0x2e;
+  static const int _LOWER_A = 0x61;
+  static const int _LOWER_Z = 0x7A;
+  static const int _UPPER_A = 0x41;
+  static const int _UPPER_Z = 0x5A;
 
+  final bool _isWindows;
+  String separator;
   int _separatorChar;
 
-  AbsolutePathContext(this.separator) {
-    if (separator.length != 1) {
-      throw new ArgumentError.value(
-          separator, 'separator', 'must be exactly one character long');
-    }
+  AbsolutePathContext(this._isWindows) {
+    separator = _isWindows ? r'\' : '/';
     _separatorChar = separator.codeUnitAt(0);
   }
 
@@ -55,6 +59,14 @@ class AbsolutePathContext {
         : path.substring(0, lastIndex);
   }
 
+  /// Return `true` if the given [path] is valid.
+  ///
+  ///     context.isNormalized('/foo/bar');        // -> true
+  ///     context.isNormalized('/foo/bar/../baz'); // -> false
+  bool isValid(String path) {
+    return _isAbsolute(path) && _isNormalized(path);
+  }
+
   /// Return `true` if [child] is a path beneath [parent], and `false`
   /// otherwise. Both the [child] and [parent] paths must be absolute paths.
   ///
@@ -94,6 +106,57 @@ class AbsolutePathContext {
     return null;
   }
 
+  /// Return `true` if the given [path] is absolute.
+  ///
+  ///     _isAbsolute('/foo/bar');   // -> true
+  ///     _isAbsolute('/');          // -> true
+  ///     _isAbsolute('foo/bar');    // -> false
+  ///     _isAbsolute('C:\foo\bar'); // -> true
+  ///     _isAbsolute('C:\');        // -> true
+  ///     _isAbsolute('foo\bar');    // -> false
+  bool _isAbsolute(String path) {
+    if (_isWindows) {
+      return path.length >= 3 &&
+          _isAlphabetic(path.codeUnitAt(0)) &&
+          path.codeUnitAt(1) == _COLON &&
+          path.codeUnitAt(2) == _separatorChar;
+    } else {
+      return path.isNotEmpty && path.codeUnitAt(0) == _separatorChar;
+    }
+  }
+
+
+  /// Return `true` if the given absolute [path] is normalized.
+  ///
+  ///     _isNormalized('/foo/bar');        // -> true
+  ///     _isNormalized('/foo/..bar');      // -> true
+  ///     _isNormalized('/');               // -> true
+  ///     _isNormalized('/foo/bar/../baz'); // -> false
+  ///     _isNormalized('/foo/bar/..');     // -> false
+  bool _isNormalized(String path) {
+    int periodCount = 0;
+    for (int c in path.codeUnits) {
+      if (c == _PERIOD) {
+        periodCount++;
+        continue;
+      }
+      if (c == _separatorChar) {
+        if (periodCount == 1 || periodCount == 2) {
+          return false;
+        }
+      }
+      periodCount = 0;
+    }
+    return periodCount != 1 && periodCount != 2;
+  }
+
+  /// Returns whether [char] is the code for an ASCII letter (uppercase or
+  /// lowercase).
+  static bool _isAlphabetic(int char) {
+    return char >= _UPPER_A && char <= _UPPER_Z ||
+        char >= _LOWER_A && char <= _LOWER_Z;
+  }
+
   /// Return `true` if [str] starts with the given [prefix].
   ///
   /// The check is done from the end of [prefix], because absolute paths

pkg/analyzer/test/src/util/absolute_path_test.dart

@@ -18,7 +18,7 @@ main() {
 
 @reflectiveTest
 class AbsolutePathContextPosixTest {
-  AbsolutePathContext context = new AbsolutePathContext(r'/');
+  AbsolutePathContext context = new AbsolutePathContext(false);
 
   void test_append() {
     expect(context.append(r'/path/to', r'foo.dart'), r'/path/to/foo.dart');
@@ -38,6 +38,27 @@ class AbsolutePathContextPosixTest {
     expect(context.dirname(r'/'), r'/');
   }
 
+  void test_isValid_absolute() {
+    expect(context.isValid(r'/foo/bar'), isTrue);
+    expect(context.isValid(r'/foo'), isTrue);
+    expect(context.isValid(r'/'), isTrue);
+    expect(context.isValid(r''), isFalse);
+    expect(context.isValid(r'foo/bar'), isFalse);
+  }
+
+  void test_isValid_normalized() {
+    expect(context.isValid(r'/foo/bar'), isTrue);
+    expect(context.isValid(r'/foo/..bar'), isTrue);
+    expect(context.isValid(r'/foo/.bar/baz'), isTrue);
+    expect(context.isValid(r'/foo/...'), isTrue);
+    expect(context.isValid(r'/foo/.../bar'), isTrue);
+    expect(context.isValid(r'/foo/.bar/.'), isFalse);
+    expect(context.isValid(r'/foo/bar/../baz'), isFalse);
+    expect(context.isValid(r'/foo/bar/..'), isFalse);
+    expect(context.isValid(r'/foo/./bar'), isFalse);
+    expect(context.isValid(r'/.'), isFalse);
+  }
+
   void test_isWithin() {
     expect(context.isWithin(r'/root/path', r'/root/path/a'), isTrue);
     expect(context.isWithin(r'/root/path', r'/root/other'), isFalse);
@@ -57,7 +78,7 @@ class AbsolutePathContextPosixTest {
 
 @reflectiveTest
 class AbsolutePathContextWindowsTest {
-  AbsolutePathContext context = new AbsolutePathContext(r'\');
+  AbsolutePathContext context = new AbsolutePathContext(true);
 
   void test_append() {
     expect(context.append(r'C:\path\to', r'foo.dart'), r'C:\path\to\foo.dart');
@@ -77,6 +98,29 @@ class AbsolutePathContextWindowsTest {
     expect(context.dirname(r'C:\'), r'C:\');
   }
 
+  void test_isValid_absolute() {
+    expect(context.isValid(r'C:\foo\bar'), isTrue);
+    expect(context.isValid(r'c:\foo\bar'), isTrue);
+    expect(context.isValid(r'D:\foo\bar'), isTrue);
+    expect(context.isValid(r'C:\foo'), isTrue);
+    expect(context.isValid(r'C:\'), isTrue);
+    expect(context.isValid(r''), isFalse);
+    expect(context.isValid(r'foo\bar'), isFalse);
+  }
+
+  void test_isValid_normalized() {
+    expect(context.isValid(r'C:\foo\bar'), isTrue);
+    expect(context.isValid(r'C:\foo\..bar'), isTrue);
+    expect(context.isValid(r'C:\foo\.bar\baz'), isTrue);
+    expect(context.isValid(r'C:\foo\...'), isTrue);
+    expect(context.isValid(r'C:\foo\...\bar'), isTrue);
+    expect(context.isValid(r'C:\foo\.bar\.'), isFalse);
+    expect(context.isValid(r'C:\foo\bar\..\baz'), isFalse);
+    expect(context.isValid(r'C:\foo\bar\..'), isFalse);
+    expect(context.isValid(r'C:\foo\.\bar'), isFalse);
+    expect(context.isValid(r'C:\.'), isFalse);
+  }
+
   void test_isWithin() {
     expect(context.isWithin(r'C:\root\path', r'C:\root\path\a'), isTrue);
     expect(context.isWithin(r'C:\root\path', r'C:\root\other'), isFalse);