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

Insiders regression: "(" and ")" in Uri's are not escaped (leading to extension failures) #83490

Closed
sean-mcmanus opened this issue Oct 29, 2019 · 8 comments
Assignees
Labels
*duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster uri

Comments

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented Oct 29, 2019

  • VSCode Version: 1.40.0-insiders
  • OS Version: Windows (probably others)

Steps to Reproduce:

  1. Use some LSP extension like the C/C++ one to open a document.

Bug: The Uri our extension receives doesn't have the "(" and ")" escaped with the %hex encoding, which causes error squiggles in our extension to fail to appear (microsoft/vscode-cpptools#4505 ) because file comparisons fail because our internal "URI from path" code is encoding the "(" and ")". Let us know if we're expected to ship a fix to account for this change in Uri encoding.

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

@jrieken jrieken added info-needed Issue requires more information from poster uri labels Oct 29, 2019
@jrieken
Copy link
Member

jrieken commented Oct 29, 2019

because file comparisons fail because our internal "URI from path" code is encoding the "(" and ")". Let us know if we're expected to ship a fix to account for this change in Uri encoding.

I would like to know more. What is your internal "URI from path" thing? Isn't that vscode.Uri.file and isn't that the vscode-uri-npm module? We did make changes to our uri-implementation to be more aligned with the whatwg-URL about what characters to encode and what not. I am aware that's a risky change and therefore I need more information to make a judgement, esp wrt the URI implementation you are using

@sean-mcmanus
Copy link
Contributor Author

The "URI from path" code is written in C++ in our LSP process. We get the URI via LSP in many cases, so we can't call the same vscode.Uri.file APIs. We have some ideas on how we could fix the issue on our end, such as storing/comparing "paths" instead of URIs or dynamically doing different URI encoding based on what VS Code does.

@jrieken
Copy link
Member

jrieken commented Oct 30, 2019

The "URI from path" code is written in C++ in our LSP process.

So, is that a custom implementation or a library implementation? I am asking because all those changes were made to align with library implementations and to be compatible with them - often we over encoded things that are decoded in other implementation and therefore are loosing data along the way

@jrieken
Copy link
Member

jrieken commented Oct 30, 2019

/duplicate of #83610

@vscodebot vscodebot bot added the *duplicate Issue identified as a duplicate of another issue(s) label Oct 30, 2019
@vscodebot
Copy link

vscodebot bot commented Oct 30, 2019

Thanks for creating this issue! We figured it's covering the same as another one we already have. Thus, we closed this one as a duplicate. You can search for existing issues here. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Oct 30, 2019
@jrieken
Copy link
Member

jrieken commented Oct 30, 2019

closing this as dupe, we will revert our uri changes as extension programmed against our behaviour.

@jrieken
Copy link
Member

jrieken commented Oct 30, 2019

fixed via 58479e8

@sean-mcmanus
Copy link
Contributor Author

Thanks.

@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster uri
Projects
None yet
Development

No branches or pull requests

2 participants