pathContext.fromUri() mishandles encoded colons after drive letters
Closed this issue · 4 comments
This came up via dart-lang/sdk#53000. I had swapped some code from doing uri.toFilePath()
to pathContext.fromUri(uri)
to support testing Windows-style paths on MacOS.
However it turns out there is a difference in behaviour when the colon after a drive letter is percent-encoded:
final fileUris = [
'file:///C%3A/Dev/Test%20Projects/dart_application_3',
'file:///C:/Dev/Test%20Projects/dart_application_3'
];
for (final fileUri in fileUris) {
final path1 = Uri.parse(fileUri).toFilePath();
final path2 = pathContext.fromUri(fileUri);
print('Uri.parse($fileUri).toFilePath():\n $path1');
print('pathContext.fromUri($fileUri):\n $path2');
}
Uri.parse(file:///C%3A/Dev/Test%20Projects/dart_application_3).toFilePath():
C:\Dev\Test Projects\dart_application_3
pathContext.fromUri(file:///C%3A/Dev/Test%20Projects/dart_application_3):
\C:\Dev\Test Projects\dart_application_3
Uri.parse(file:///C:/Dev/Test%20Projects/dart_application_3).toFilePath():
C:\Dev\Test Projects\dart_application_3
pathContext.fromUri(file:///C:/Dev/Test%20Projects/dart_application_3):
C:\Dev\Test Projects\dart_application_3
Since toFilePath()
handles this (and VS Code has been producing URIs in this format forever), I believe the encoding of the colon is valid (although I've failed to find anything that seems explicit in the spec) and should be handled.
I found this:
https://url.spec.whatwg.org/#windows-drive-letter
A Windows drive letter is two code points, of which the first is an ASCII alpha and the second is either U+003A (:) or U+007C (|).
Though I found this comment in the VS Code source (which claims it is not incorrect):
* *Note* that the implementation will encode _aggressive_ which often leads to unexpected,
* but not incorrect, results. For instance, colons are encoded to `%3A` which might be unexpected
* in file-uri.
So now I'm less certain. I'm going to file an issue with the VS Code LSP client and see what response I get there.
@natebosch thanks! Any issues with me bumping this in the Dart SDK's DEPs?
@natebosch thanks! Any issues with me bumping this in the Dart SDK's DEPs?
No concerns at all - I would bump it soon if no one else did 😄
After we bump in the SDK and it gets synced to google I will also get it published.
Thanks! I've opened https://dart-review.googlesource.com/c/sdk/+/319522. You (or another Googler) will need to merge once you're happy. Please let me know if any issues come up that you think may be related.