Skip to content
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

vscode-uri escapes colon in drive letter #75027

Closed
roblourens opened this issue Jun 6, 2019 · 6 comments
Closed

vscode-uri escapes colon in drive letter #75027

roblourens opened this issue Jun 6, 2019 · 6 comments
Assignees
Labels
*as-designed Described behavior is as designed under-discussion Issue is under discussion for relevance, priority, approach uri

Comments

@roblourens
Copy link
Member

We are trying to use vscode-uri in the debug adapter, and one issue is that vscode-uri uri-encodes the colon after the drive letter in a windows file uri which causes issues downstream.

I see #2990 (comment) but I'm worried that will just cause other issues by not encoding other characters. And I'm wondering why we do this by default, this file URI scheme spec calls out file:///c:/foo as being a valid file URI. https://tools.ietf.org/html/rfc8089

@jrieken
Copy link
Member

jrieken commented Jun 7, 2019

issues by not encoding other characters

Like which? Do you have a sample?

@jrieken jrieken added the uri label Jun 7, 2019
@jrieken
Copy link
Member

jrieken commented Jun 7, 2019

which causes issues downstream

What issues are those?

@roblourens
Copy link
Member Author

If we use toString(true) then we would get "file:///c:/foo blah/bar" instead of "file:///c:/foo%20blah/bar" for example which Chrome doesn't recognize.

Also Chrome uses the format "file:///c:/foo/bar" so if we ask it to set a breakpoint in "file:///c%3A/foo/bar" it doesn't recognize that script.

We can work around it on our end, this is more just a question about why it's done that way.

@jrieken
Copy link
Member

jrieken commented Jun 7, 2019

I always only looked https://tools.ietf.org/html/rfc3986 and there is no clear word on how aggressive to encode. My guarantee is that we decode whatever gets encoded, e.g preserving the identify. The challenge is when urls as strings get send around esp to different systems or when uris are compared in string form. All of that wasn't part of the original work...

We can consider making this change (I have already done some similar changes this milestone) but I am always afraid that vscode has persist uris as string that it compare such uris - as strings, not revising them first.

@jrieken jrieken added this to the June 2019 milestone Jun 7, 2019
@jrieken jrieken added the under-discussion Issue is under discussion for relevance, priority, approach label Jun 20, 2019
@jrieken jrieken modified the milestones: June 2019, July 2019 Jun 24, 2019
@jrieken
Copy link
Member

jrieken commented Jul 8, 2019

Closing as designed, after the recent avalanche of regressions after some URI changes I am too scared to change more here. For the future we can consider adding another overload to the toString-function

@jrieken jrieken added the *as-designed Described behavior is as designed label Jul 8, 2019
@vscodebot
Copy link

vscodebot bot commented Jul 8, 2019

The described behavior is how it is expected to work. If you disagree, please explain what is expected and what is not in more detail. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Jul 8, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed under-discussion Issue is under discussion for relevance, priority, approach uri
Projects
None yet
Development

No branches or pull requests

2 participants