From ba8306735d7efa8aabfbf4d8f9a618bd00bfbcbd Mon Sep 17 00:00:00 2001 From: Dan Chevalier Date: Mon, 15 Apr 2024 14:29:03 +0000 Subject: [PATCH] Ensure that dtd handles relative paths correctly. Change-Id: I1ebf22a9ae258741580660a8eaf77aa286e9affa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362400 Reviewed-by: Kenzie Davisson Commit-Queue: Dan Chevalier --- pkg/dtd/lib/src/dart_tooling_daemon.dart | 6 + pkg/dtd/test/file_system_service_test.dart | 197 +++++++++++++++++- pkg/dtd_impl/dtd_protocol.md | 6 +- .../lib/src/service/file_system_service.dart | 12 +- 4 files changed, 211 insertions(+), 10 deletions(-) diff --git a/pkg/dtd/lib/src/dart_tooling_daemon.dart b/pkg/dtd/lib/src/dart_tooling_daemon.dart index f5e4703bf84..109fd679723 100644 --- a/pkg/dtd/lib/src/dart_tooling_daemon.dart +++ b/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 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 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 getProjectRoots({ int depth = defaultGetProjectRootsDepth, }) async { diff --git a/pkg/dtd/test/file_system_service_test.dart b/pkg/dtd/test/file_system_service_test.dart index 30a23300b8b..6ab40a78582 100644 --- a/pkg/dtd/test/file_system_service_test.dart +++ b/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), + ]); }, ); }); diff --git a/pkg/dtd_impl/dtd_protocol.md b/pkg/dtd_impl/dtd_protocol.md index 535e8b19ed1..6ef17d075a0 100644 --- a/pkg/dtd_impl/dtd_protocol.md +++ b/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 diff --git a/pkg/dtd_impl/lib/src/service/file_system_service.dart b/pkg/dtd_impl/lib/src/service/file_system_service.dart index 0cbdb732d7f..ca24a2af18a 100644 --- a/pkg/dtd_impl/lib/src/service/file_system_service.dart +++ b/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 = []; for (final root in parameters['roots'].asList.cast()) { - 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,