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

support query encoding in http, http, ftp and fix path decode issues #73849

Merged
merged 9 commits into from
Jun 3, 2019

Conversation

jrieken
Copy link
Member

@jrieken jrieken commented May 16, 2019

This PR is for #25852 and #45515

@jrieken
Copy link
Member Author

jrieken commented May 31, 2019

@isidorn these changes make some tests in debug land fail:

assert.equal(firstStackFrame.getSpecificSourceName(), '.../b/c/d/internalModule.js');

It seems like the debug source is build on the (bug) behaviour described in #45515, e.g when the path component contains an encoded / (%2f) it should not be decoded but kept as is

@jrieken jrieken added this to the June 2019 milestone May 31, 2019
@jrieken jrieken changed the title support query encoding in http, http, ftp support query encoding in http, http, ftp and fix path decode issues May 31, 2019
@jrieken jrieken added the uri label Jun 3, 2019
@isidorn
Copy link
Contributor

isidorn commented Jun 3, 2019

Looking at it I am not sure why do we encode the path component here

@weinand introduced the encoding of the uri for the debug source so he would be better to comment here.

@jrieken
Copy link
Member Author

jrieken commented Jun 3, 2019

If the goal is to create new uris from existing components, than you should use URI.from

@isidorn
Copy link
Contributor

isidorn commented Jun 3, 2019

Yeah that seems like the way to go.
I can do the changes if @weinand agrees

@weinand
Copy link
Contributor

weinand commented Jun 3, 2019

@isidorn yes, please adapt.

@jrieken
Copy link
Member Author

jrieken commented Jun 3, 2019

Great - let me know if you need help

@isidorn
Copy link
Contributor

isidorn commented Jun 3, 2019

@jrieken thanks, it would be great if you could reveiw this PR I just created.
#74781

@jrieken jrieken merged commit 8fc6542 into master Jun 3, 2019
@jrieken jrieken deleted the joh/uri-query branch June 3, 2019 13:56
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants