-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pathContext.fromUri() mishandles encoded colons after drive letters #545
Comments
I found this: https://url.spec.whatwg.org/#windows-drive-letter
Though I found this comment in the VS Code source (which claims it is not incorrect):
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? |
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. |
Gets a fix for supporting encoded colons in fromUri() (https://github.com/dart-lang/path/issues/148). Change-Id: I191b770f3ca3b616ee862a640ad2c17600871fbe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319522 Reviewed-by: Nate Bosch <nbosch@google.com> Commit-Queue: Nate Bosch <nbosch@google.com>
…le URIs in the LSP server This reverts be4189f plus adds an additional test to verify pkg:path to behaviour (to catch future regressions or if pkg:path has to be reverted, this will need reverting too). This relies on the fix made at https://github.com/dart-lang/path/issues/148 which rolled into the SDK in f1de897. Change-Id: I1dea45e2017f7505bc4aca97f6c07c1a6e445a5e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319523 Commit-Queue: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Fixes dart-lang/path#148 Using `Uri().toFilePath()` handles escaped colons in drive letters, but `Context.fromUri()` currently does not. Allow the percent encoded colon by checking both formats and adjusting to the correct index for the following character when the percent encoding makes the drive letter longer than the typical two characters.
This came up via dart-lang/sdk#53000. I had swapped some code from doing
uri.toFilePath()
topathContext.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:
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.The text was updated successfully, but these errors were encountered: