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

Fixes image name including # fail to render #84334

Merged
merged 2 commits into from Nov 13, 2019
Merged

Conversation

@jeanp413
Copy link
Contributor

jeanp413 commented Nov 9, 2019

This PR fixes #84197

@@ -83,7 +83,7 @@ function normalizeRequestPath(requestUri: URI) {

// Modern vscode-resources uris put the scheme of the requested resource as the authority
if (requestUri.authority) {
return URI.parse(requestUri.authority + ':' + requestUri.path);
return URI.parse(requestUri.authority + ':' + encodeURIComponent(requestUri.path).replace(/%2F/g, '/'));

This comment has been minimized.

Copy link
@mjbvz

mjbvz Nov 11, 2019

Contributor

Which operating system did you test this on. The change looks good but I'm not sure if it will work on windows

This comment has been minimized.

Copy link
@jeanp413

jeanp413 Nov 11, 2019

Author Contributor

Ubuntu 18.04

This comment has been minimized.

Copy link
@mjbvz

mjbvz Nov 13, 2019

Contributor

What happens for windows style paths though (ones that use \)?

This comment has been minimized.

Copy link
@jeanp413

jeanp413 Nov 13, 2019

Author Contributor

I don't have a windows VM at hand to be able to build from sources and test it but looking at the uri.ts file it seems that \ characters are converted to / before constructing the uri object

if (isWindows) {
path = path.replace(/\\/g, _slash);
}

and converted back again with the fsPath getter

so the requestUri.path doesn't have any \ character

@mjbvz mjbvz added this to the November 2019 milestone Nov 11, 2019
@mjbvz mjbvz merged commit ef4d7e4 into microsoft:master Nov 13, 2019
@jeanp413 jeanp413 deleted the jeanp413:fix-84197 branch Nov 13, 2019
@jeanp413

This comment has been minimized.

Copy link
Contributor Author

jeanp413 commented Nov 18, 2019

@mjbvz I tested this today and seems that #84667 broke this again.
I tested both PRs separately and they fix both issues (#84197 and #83165) so you need to revert one of them.
The issue is that URI.parse is called twice here and here

  • #84667 fixes it by encoding the uri twice in the extension so a filename #.png will be encoded as "vscode-resource://file///home/jeanpierre/Desktop/vstest/%2523.png?version=1574091136161" (notice %23 is encoded as %2523)
  • This PR fixes just by encoding the path again before calling URI.parse the second time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.