Repository navigation
Unreserved characters are escaped when making a HTTP request #33037
Description
- Version: >=10.20.1
- Platform: all
- Subsystem: http
What steps will reproduce the bug?
const http = require('http');
const url = new URL('http://httpbin.org/anything?a=~');
url.search = url.searchParams.toString(); // trigger normalization
const request = http.get(url, response => {
const chunks = [];
response.on('data', chunk => {
chunks.push(chunk);
});
response.once('end', () => {
console.log(request._header);
// Actual:
// GET /anything?a=%7E
//
// Expected:
// GET /anything?a=~
});
})How often does it reproduce? Is there a required condition?
Always.
What is the expected behavior?
GET /anything?a=~
What do you see instead?
GET /anything?a=%7E
Additional information
According to tools.ietf.org/html/rfc3986#section-2.3 tilde is unreserved and should not encoded by URI producers.
Activity
bnoordhuis commentedon Apr 24, 2020
The
url.searchParams.toString()call is what changes~to%7E. I'm not 100% sure but it looks according to spec to me. Firefox behaves the same way, FWIW.The spec says the application/x-www-form-urlencoded serializer should be applied:
@himself65 I see you tagged this with confirmed-bug. Can you elaborate?
himself65 commentedon Apr 24, 2020
oh, sry for I didn't look up the spec🤦.
and just a little confused me because
Hamper commentedon Apr 24, 2020
Isn't that bug in URLSearchParams? Because in spec https://tools.ietf.org/html/rfc3986#section-2.3 we see that ~ is unreserved character and
Also in https://url.spec.whatwg.org/ we see that percent encoding must be used only for code points greater than U+007E (~) but not equal this symbol
bnoordhuis commentedon Apr 24, 2020
@Hamper https://url.spec.whatwg.org/#concept-urlencoded-byte-serializer is the mapping for individual bytes.
~must be percent-encoded according to that.jasnell commentedon Apr 24, 2020
The URL parser uses the WHATWG URL Standard as the normative reference and not rfc3986 and the two definitely differ with regards to some of the escaping rules.
That said, I do think this is a bug... not necessarily in our impl but in the URL Standard spec itself. Definitely warrants more investigation.
Specifically, look at section https://url.spec.whatwg.org/#url-parsing and search for "query state" ...
jasnell commentedon Apr 24, 2020
@bnoordhuis ... the section you reference deals specifically with
application/x-www-form-urlencodedserialization, which is different from bothURLandURLSearchParams. While the URL Standard does provide a specification forapplication/x-www-form-urlencodedparsing and serialization, it does not require or even recommend it's use forURLparsing and handling ofURLSearchParams. They are separate things. The issue opened here is in regards toURLparsing and notapplication/x-www-form-urlencodedbnoordhuis commentedon Apr 24, 2020
@jasnell I think you overlooked the line that says
url.search = url.searchParams.toString()- it's the.toString()that encodes to application/x-www-form-urlencoded and that's per spec.jasnell commentedon Apr 24, 2020
Ah, yep, there it is. As I said, I don't think this is a bug in our implementation but I do think it's a bug in the spec itself because given the rules defined elsewhere in the doc
~is handled differently. Let's keep this issue open and get clarification from the URL Standard folks on what the correct behavior should be./cc @annevk ... when you get a moment, could you help us resolve this issue? :-)
9 remaining items
szmarczak commentedon Apr 24, 2020
Does the WHATWG URL spec override the URL normalization in the HTTP spec?
jasnell commentedon Apr 24, 2020
@szmarczak ... that's a total grey area lol. All of the various HTTP specifications normatively reference the IETF URL specification in some way, and most HTTP server and middle box implementations follow suit. The WHATWG URL spec, however, is what browsers / useragents generally follow and the WHATWG spec has largely emerged as the de facto standard. There's always been a bit of contention on the areas where they differ.
bnoordhuis commentedon Apr 27, 2020
I think this issue should be closed until the WHATWG issue reaches a conclusion, if indeed it ever does. Right now this issue is completely inactionable.
szmarczak commentedon Apr 27, 2020
I strongly disagree. It's easy to forget the issue that way and we don't want that. A more appropriate thing to do would be rather to mark this issue as blocked.
bnoordhuis commentedon Apr 28, 2020
There are almost a thousand open issues. No one is going to look at this one once it drops off the first page or two, label or no label.
jasnell commentedon Apr 28, 2020
Quick update on whatwg/url#478 ... I have a general proposed fix on the table (to require that URLQueryParam stringification follow URL rules when
.searchParamsis used to mutate the URL). The WHATWG folks are working that through their process to determine how other implementers may feel about the change. If it looks like the change will have a good chance of making it through, I will open a PR there with a draft spec change. Once that is accepted, we'll need a PR here to fix and close this issue.Specifically, the proposed change is internal only, and would only impact using the
.searchParamsobject associated with theURLto mutate the URL.. that is...This would follow URL encoding rules:
But this would not, however...
Assuming the change proposal does not go through at the WHATWG, then this will need to be closed without changing behavior as our priority is on following the spec and we do not want to unilaterally diverge from that. It would, however, be good to add documentation to
url.md.@bnoordhuis
Except that it is not. See above.
doc: doc and test URLSearchParams discrepancy
jasnell commentedon May 4, 2020
Alrighty, well, it's looking like changes to the WHATWG spec aren't going to happen, which is fine. Added a PR that documents the discrepancy.
doc: doc and test URLSearchParams discrepancy
doc: doc and test URLSearchParams discrepancy
doc: doc and test URLSearchParams discrepancy