Ensure that dtd handles relative paths correctly.

Change-Id: I1ebf22a9ae258741580660a8eaf77aa286e9affa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362400
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Commit-Queue: Dan Chevalier <danchevalier@google.com>

pkg/dtd/lib/src/dart_tooling_daemon.dart

@@ -256,6 +256,8 @@ class DartToolingDaemon {
   ///
   /// If [uri] does not have a file scheme, then an [RpcException] with
   /// [RpcErrorCodes.kExpectsUriParamWithFileScheme] is thrown.
+  ///
+  /// The returned uris will be `file://` Uris.
   Future<UriList> listDirectoryContents(Uri uri) async {
     final result = await call(
       kFileSystemServiceName,
@@ -293,6 +295,8 @@ class DartToolingDaemon {
   }
 
   /// Gets the IDE workspace roots for the FileSystem service.
+  ///
+  /// The returned uris will be `file://` Uris.
   Future<IDEWorkspaceRoots> getIDEWorkspaceRoots() async {
     final result = await call(
       kFileSystemServiceName,
@@ -310,6 +314,8 @@ class DartToolingDaemon {
   ///
   /// [depth] is the maximum depth that each IDE workspace root directory tree
   /// will be searched for project roots.
+  ///
+  /// The returned uris will be `file://` Uris.
   Future<UriList> getProjectRoots({
     int depth = defaultGetProjectRootsDepth,
   }) async {

pkg/dtd/test/file_system_service_test.dart

@@ -231,8 +231,11 @@ void main() {
           ]);
           final roots = await client.getIDEWorkspaceRoots();
           expect(
-            roots.ideWorkspaceRoots,
-            containsAll([fooDirectory.uri, barDirectory.uri]),
+            roots.ideWorkspaceRoots.map((e) => p.normalize(e.path)),
+            containsAll(
+              [fooDirectory.uri, barDirectory.uri]
+                  .map((e) => p.normalize(e.path)),
+            ),
           );
         });
       });
@@ -423,6 +426,186 @@ void main() {
         });
       });
     });
+
+    group('relative paths', () {
+      test('normalizes paths when setting pub root', () async {
+        final relativePath = p.join(fooDirectory.path, '..', 'bar', 'a.txt');
+        final simplifiedPath = p.join(barDirectory.path, 'a.txt');
+
+        await client.call(
+          'FileSystem',
+          'setIDEWorkspaceRoots',
+          params: {
+            'secret': dtdSecret,
+            'roots': [
+              'file://$relativePath',
+            ],
+          },
+        );
+        final roots = await client.getIDEWorkspaceRoots();
+        expect(
+          roots.ideWorkspaceRoots.map((e) => e.path),
+          [simplifiedPath],
+        );
+      });
+
+      test('prevents access outide of workspace roots for relative paths',
+          () async {
+        await client.setIDEWorkspaceRoots(dtdSecret, [fooDirectory.uri]);
+        expect(
+          () => client.call(
+            'FileSystem',
+            'readFileAsString',
+            params: {'uri': p.join('${fooDirectory.uri}', '..', 'a.txt')},
+          ),
+          throwsAnRpcError(RpcErrorCodes.kPermissionDenied),
+        );
+        expect(
+          () => client.call(
+            'FileSystem',
+            'writeFileAsString',
+            params: {
+              'uri': p.join('${fooDirectory.uri}', '..', 'a.txt'),
+              'contents': 'abc',
+              'encoding': 'utf-8',
+            },
+          ),
+          throwsAnRpcError(RpcErrorCodes.kPermissionDenied),
+        );
+        expect(
+          () => client.call(
+            'FileSystem',
+            'listDirectoryContents',
+            params: {
+              'uri': p.join('${fooDirectory.uri}', '..'),
+            },
+          ),
+          throwsAnRpcError(RpcErrorCodes.kPermissionDenied),
+        );
+      });
+
+      test('allows access to relative paths with ide workspace roots',
+          () async {
+        await client.setIDEWorkspaceRoots(dtdSecret, [fooDirectory.uri]);
+
+        final writeResult = await client.call(
+          'FileSystem',
+          'writeFileAsString',
+          params: {
+            'uri': p.join(
+              fooDirectory.uri.toString(),
+              'C',
+              'D',
+              '..',
+              '..',
+              'C',
+              'd.txt',
+            ),
+            'contents': 'abc',
+            'encoding': 'utf-8',
+          },
+        );
+
+        expect(writeResult.result, {'type': 'Success'});
+        final readResult = await client.call(
+          'FileSystem',
+          'readFileAsString',
+          params: {
+            'uri': p.join(
+              fooDirectory.uri.toString(),
+              'C',
+              'D',
+              '..',
+              '..',
+              'C',
+              'd.txt',
+            ),
+          },
+        );
+        expect(readResult.result, {'type': 'FileContent', 'content': 'abc'});
+
+        final listResult = await client.call(
+          'FileSystem',
+          'listDirectoryContents',
+          params: {
+            'uri': p.join(
+              fooDirectory.uri.toString(),
+              'C',
+              'D',
+              '..',
+              '..',
+              'C',
+            ),
+          },
+        );
+
+        expect(listResult.result, {
+          'type': 'UriList',
+          'uris': containsAll([
+            'file://${fooDirectory.uri.toFilePath()}C/pubspec.yaml',
+            'file://${fooDirectory.uri.toFilePath()}C/d.txt',
+          ]),
+        });
+      });
+
+      final invalidDirectories = [
+        {
+          'dir': './',
+          'error':
+              throwsAnRpcError(RpcErrorCodes.kExpectsUriParamWithFileScheme),
+        },
+        {
+          'dir': '/',
+          'error':
+              throwsAnRpcError(RpcErrorCodes.kExpectsUriParamWithFileScheme),
+        },
+        {
+          'dir': '../',
+          'error':
+              throwsAnRpcError(RpcErrorCodes.kExpectsUriParamWithFileScheme),
+        },
+        {
+          'dir': 'file:///~/',
+          'error': throwsAnRpcError(RpcErrorCodes.kPermissionDenied),
+        },
+      ];
+      for (final invalidDirectory in invalidDirectories) {
+        test('prevents use of invalid uri: ${invalidDirectory['dir']}', () {
+          final dir = invalidDirectory['dir'] as String;
+          final error = invalidDirectory['error'] as Matcher;
+          expect(
+            () => client.call(
+              'FileSystem',
+              'readFileAsString',
+              params: {'uri': '${dir}a.txt'},
+            ),
+            error,
+          );
+          expect(
+            () => client.call(
+              'FileSystem',
+              'writeFileAsString',
+              params: {
+                'uri': '${dir}a.txt',
+                'contents': 'abc',
+                'encoding': 'utf-8',
+              },
+            ),
+            error,
+          );
+          expect(
+            () => client.call(
+              'FileSystem',
+              'listDirectoryContents',
+              params: {
+                'uri': dir,
+              },
+            ),
+            error,
+          );
+        });
+      }
+    });
   });
 
   group('unrestricted', () {
@@ -463,9 +646,13 @@ void main() {
 
         final readResult = await client.readFileAsString(aFile.uri);
         expect(readResult.content, fileContents);
-        expect((await client.getIDEWorkspaceRoots()).ideWorkspaceRoots, [
-          barDirectory.uri,
-        ]);
+        expect(
+            (await client.getIDEWorkspaceRoots())
+                .ideWorkspaceRoots
+                .map((e) => p.normalize(e.path)),
+            [
+              p.normalize(barDirectory.uri.path),
+            ]);
       },
     );
   });

pkg/dtd_impl/dtd_protocol.md

@@ -743,7 +743,9 @@ the different service methods.
 
 Response for communicating a list of URIs.
 
-Used by `FileSystem.listDirectoryContents`.
+The returned uris will be `file://` Uris.
+
+Used by `FileSystem.listDirectoryContents`, and `FileSystem.getProjectRoots`.
 
 ```json
 {
@@ -772,6 +774,8 @@ Used by `FileSystem.readFileAsString`
 
 Response for communicating workspace roots.
 
+The returned uris will be `file://` Uris.
+
 Used by `FileSystem.getIDEWorkspaceRoots`
 
 ```json

pkg/dtd_impl/lib/src/service/file_system_service.dart

@@ -9,6 +9,7 @@ import 'package:collection/collection.dart';
 import 'package:dtd_impl/src/constants.dart';
 import 'package:json_rpc_2/json_rpc_2.dart';
 import 'package:dtd/dtd.dart';
+import 'package:path/path.dart' as path;
 
 import '../dtd_client.dart';
 
@@ -61,8 +62,11 @@ class FileSystemService {
   void _ensureIDEWorkspaceRootsContainUri(Uri uri) {
     // If in unrestricted mode, no need to do these checks.
     if (unrestrictedMode) return;
-
-    if (_ideWorkspaceRoots.any((root) => uri.path.startsWith(root.path))) {
+    if (_ideWorkspaceRoots.any(
+      (root) =>
+          path.isWithin(root.path, uri.path) ||
+          path.equals(root.path, uri.path),
+    )) {
       return;
     }
 
@@ -81,7 +85,7 @@ class FileSystemService {
     }
     final newRoots = <Uri>[];
     for (final root in parameters['roots'].asList.cast<String>()) {
-      final rootUri = Uri.parse(root);
+      final rootUri = Uri.parse(path.normalize(root));
       if (rootUri.scheme != 'file') {
         throw RpcErrorCodes.buildRpcException(
           RpcErrorCodes.kExpectsUriParamWithFileScheme,
@@ -163,7 +167,7 @@ class FileSystemService {
 
   Uri _extractUri(Parameters parameters) {
     final uriString = parameters['uri'].asString;
-    final uri = Uri.parse(uriString);
+    final uri = Uri.parse(path.normalize(uriString));
     if (uri.scheme != 'file') {
       throw RpcErrorCodes.buildRpcException(
         RpcErrorCodes.kExpectsUriParamWithFileScheme,