dart-lang/path

pathContext.fromUri() mishandles encoded colons after drive letters

Closed this issue · 4 comments

DanTup commented

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.

DanTup commented

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.

DanTup commented

@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.

DanTup commented

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.