Repository navigation
Failure to read files from workspace roots via DTD on Windows #5210
Description
Not sure if this is a DTD or Dart-Code issue, but on beta we have test failures because we try to read files with slightly different formats of file URI (for example encoded colon vs not), but get a failure for a not-encoded colon:
==> {"id":"3","jsonrpc":"2.0","method":"FileSystem.readFileAsString","params":{"uri":"file:///c%3A/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/main.dart"}}
<== {"jsonrpc":"2.0","result":{"type":"FileContent","content":"<SNIP>"},"id":"3"}
==> {"id":"4","jsonrpc":"2.0","method":"FileSystem.readFileAsString","params":{"uri":"file:///C:/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/main.dart"}}
<== {"jsonrpc":"2.0","error":{"code":142,"message":"Permission denied","data":{"request":{"id":"4","jsonrpc":"2.0","method":"FileSystem.readFileAsString","params":{"uri":"file:///C:/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world/bin/main.dart"}}}},"id":"4"}
Workspace roots were sent with the colon:
==> {"id":"2","jsonrpc":"2.0","method":"FileSystem.getIDEWorkspaceRoots","params":{}}
<== {"jsonrpc":"2.0","result":{"type":"IDEWorkspaceRoots","ideWorkspaceRoots":["file:///c%3A/Dev/Dart-Code/Dart-Code/src/test/test_projects/hello_world"]},"id":"2"}
Activity
[-]Failure to read files from workspace roots via DTD[/-][+]Failure to read files from workspace roots via DTD on Windows[/+]DanTup commentedon Aug 1, 2024
I think this is a DTD bug here:
https://github.com/dart-lang/sdk/blob/d47288f3c0c42501e7db9b36252528966cdcb72b/pkg/dtd_impl/lib/src/service/file_system_service.dart#L70-L72
It's using
.pathon the URI which means there is no decoding happening. If it was using.toFilePath()(or context.fromUri()) then the colons would be unescaped and the paths match.@kenzieschmoll @bkonyi FYI - I'll send a fix to DTD, but since this missed 3.5, this could affect anyone trying to use this API depending on how their URIs are encoded.
bkonyi commentedon Aug 1, 2024
This API is so annoying... it's something we repeatedly get burned by. I wonder if there's a way to create a lint for this... it'd be great if
.pathwould could actually throw an exception when used withfile://URIs.I think this is a fix we could cherry pick into the stable branch.
DanTup commentedon Aug 1, 2024
I'm not sure how well a lint would work (since it doesn't know if it's a file URI and maybe
.pathis common on others... in fact I'm using it in the tests I'm writing :))I would be happy with an exception too, but I think that's quite a breaking change 😄
Slightly related is dart-lang/sdk#54917 so I may fix that at the same time.
bkonyi commentedon Aug 1, 2024
Yeah, it's mostly me dreaming of a better world where we don't have to worry about messing up our file path encoding on Windows every time we use
Uri... 😄 maybe it's worth creating aFileUriin our projects that extendsUriand throws whenpathis accessed... Someday I'll get fed up enough to do something 😆DanTup commentedon Aug 1, 2024
I've seen projects do this... I thought maybe extension types would help here, but I haven't tried it out (possibly it's too easy to just cast back and forth).
The real fix is for Windows to just support POSIX paths! (and for paths to be case-sensitive EVERYWHERE) 😄
Got a fix open at https://dart-review.googlesource.com/c/sdk/+/378461
[dtd] Fix permission checks to handle differences in file URI escaping
DanTup commentedon Aug 28, 2024
Fixed by dart-lang/sdk@516b6b2