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

Document Content Providers are being passed URI-unescaped paths when invoked through the DAP #172108

Open
thloke opened this issue Jan 24, 2023 · 14 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues

Comments

@thloke
Copy link

thloke commented Jan 24, 2023

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.74.3
  • OS Version: Win11 22H2 22621.1105

Steps to Reproduce:

  1. We have an implementation of the DAP for our language server, and in response to the stacktrace call, we return stackframes which contain URI-escaped filenames as the Path property of Source. Something like this:
    image

  2. When VSCode handles this response and calls our document content provider, it has un-escaped the URI. This generates an invalid filename.
    image

We'd expect that the Path sent to the document content provider is the same as the Path we sent as part of the stacktrace call.

@thloke
Copy link
Author

thloke commented May 9, 2023

@roblourens - any update on this? We're getting more issues in our extension because of this.

@minaskats
Copy link

This starts to be a very big problem for us. We can not debug the CODE of A/L making it a lot harder to solve our customers problems.

@mynhardt7
Copy link

Are this being worked on? It's been open for months and breaks the Microsoft AL debugger.

@DanielGoehler
Copy link

+1

@GhOnBc
Copy link

GhOnBc commented May 23, 2023

Starting to get very annoying.. Can we please have an update

@roblourens
Copy link
Member

Sorry, I investigated this a couple months ago and couldn't find a good way to fix it, I will take another look when I'm able

@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues labels May 23, 2023
@roblourens
Copy link
Member

@jrieken, I was reviewing some of the past issues here, #25852, #83645, #32026, etc. Do you have any suggestions what to do in this case?

Here I use URI.parse on this uri string which is passed back over DAP. Then that goes to the extension's document content provider, which is expecting percent-encoded slashes as part of the path.

@thloke, you also might want to look into whether you can work around this on your end, since this issue of handling escape sequences in URIs has a long history

@jrieken
Copy link
Member

jrieken commented Jun 5, 2023

I am very sorry for this but it is something we have acknowledged but cannot fix it anymore. #83645 (comment) summarises the problem in more detail and the only path forward is to allow URL (newly'ish standard object) in addition to URI. (Tho, don't assume URL is perfect and identical across browsers...)

@roblourens
Copy link
Member

roblourens commented Jun 5, 2023

That means that the document content provider would have to support taking a URL? Are we planning on doing this for extension API anywhere yet?

@jrieken
Copy link
Member

jrieken commented Jun 6, 2023

No plans. It doesn't necessarily mean to accept URL in the document content provider, more like allowing them in DAP. I believe we could create a good URI from this (using URI.from)

@roblourens
Copy link
Member

DAP isn't using URIs, it's using a string, so if there's a way to parse a uri string into components that I can pass to URI.from then I can already do that, but I think that is what URI.parse is which is the problem

@jrieken
Copy link
Member

jrieken commented Jun 8, 2023

but I think that is what URI.parse is which is the problem

Yeah, you could try new URL and then construct a URI via from from it but no guarantees that this doesn't break another thing

@roblourens
Copy link
Member

Do you think it would make sense to let an extension opt-into new behavior for the URIs it provides, with a flag like preserveEncodedURICharacters?

@jrieken
Copy link
Member

jrieken commented Jun 16, 2023

opt-into new behavior for the URIs it provides, with a flag like preserveEncodedURICharacters?

I'd say high risk because URL doesn't seem to decode at all. Look at the ü for instance. If you need inter-op with VS Code (open editors etc ppp) than it's better to stay with its URI world.

Screenshot 2023-06-16 at 17 22 11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

No branches or pull requests

7 participants