Skip to content

Commit

Permalink
[dtd] Fix permission checks to handle differences in file URI escaping
Browse files Browse the repository at this point in the history
Fixes Dart-Code/Dart-Code#5210
Fixes dart-lang#54917
Fixes dart-lang#55476

Change-Id: I492a4f876ea75972e9971f61d67d2ecf84e7b4c0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378461
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
  • Loading branch information
DanTup authored and Commit Queue committed Aug 28, 2024
1 parent 1243b9d commit 516b6b2
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 9 deletions.
40 changes: 36 additions & 4 deletions pkg/dtd/test/file_system_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,38 @@ void main() {
throwsAnRpcError(RpcErrorCodes.kExpectsUriParamWithFileScheme),
);
});

group(
'windows paths',
() {
// Test paths in various formats. We will test all combinations of
// these both as the set roots and the readFile path to ensure the
// calling client doesn't need to use the same escaping as the
// editor.
final roots = [
Uri.parse('file:///C:/foo'),
Uri.parse('file:///C%3A/foo'),
Uri.parse('file:///c:/foo'),
];
final files = roots
.map((uri) => uri.replace(path: '${uri.path}/file.txt'))
.toList();

for (final root in roots) {
for (final file in files) {
test('can read $file in $root', () async {
await client.setIDEWorkspaceRoots(dtdSecret, [root]);
expect(
() => client.readFileAsString(file),
// Expect file does not exist (NOT a permission error).
throwsAnRpcError(RpcErrorCodes.kFileDoesNotExist),
);
});
}
}
},
skip: !Platform.isWindows,
);
});

group('writeFileAsString', () {
Expand Down Expand Up @@ -438,13 +470,13 @@ void main() {
params: {
'secret': dtdSecret,
'roots': [
'file://$relativePath',
Uri.file(relativePath).toString(),
],
},
);
final roots = await client.getIDEWorkspaceRoots();
expect(
roots.ideWorkspaceRoots.map((e) => e.path),
roots.ideWorkspaceRoots.map((e) => e.toFilePath()),
[simplifiedPath],
);
});
Expand Down Expand Up @@ -542,8 +574,8 @@ void main() {
expect(listResult.result, {
'type': 'UriList',
'uris': containsAll([
'file://${fooDirectory.uri.toFilePath()}C/pubspec.yaml',
'file://${fooDirectory.uri.toFilePath()}C/d.txt',
'${fooDirectory.uri}C/pubspec.yaml',
'${fooDirectory.uri}C/d.txt',
]),
});
});
Expand Down
12 changes: 7 additions & 5 deletions pkg/dtd_impl/lib/src/service/file_system_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,13 @@ class FileSystemService extends InternalService {
void _ensureIDEWorkspaceRootsContainUri(Uri uri) {
// If in unrestricted mode, no need to do these checks.
if (unrestrictedMode) return;
if (_ideWorkspaceRoots.any(
(root) =>
path.isWithin(root.path, uri.path) ||
path.equals(root.path, uri.path),
)) {

final requestedPath = uri.toFilePath();
if (_ideWorkspaceRoots.any((root) {
final rootPath = root.toFilePath();
return path.isWithin(rootPath, requestedPath) ||
path.equals(rootPath, requestedPath);
})) {
return;
}

Expand Down

0 comments on commit 516b6b2

Please sign in to comment.