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

Square brackets should be included in QUERY_ENCODE_SET #541

Closed
Nashenas88 opened this issue Aug 13, 2019 · 3 comments
Closed

Square brackets should be included in QUERY_ENCODE_SET #541

Nashenas88 opened this issue Aug 13, 2019 · 3 comments

Comments

@Nashenas88
Copy link

Per page 18 of RFC 3986:

A host identified by an Internet Protocol literal address, version 6
[RFC3513] or later, is distinguished by enclosing the IP literal
within square brackets ("[" and "]"). This is the only place where
square bracket characters are allowed in the URI syntax.

Given this, I think they should be included at the lowest level encode set, QUERY_ENCODE_SET. I think SIMPLE_ENCODE_SET could also be used in places where the ip address specification above would be valid, so I wouldn't recommend it there (though I could also be interpreting this incorrectly).

This would also more closely follow the behavior of Javascript's encodeURI. Mozilla's documentation also mentions handling of RFC3986:

Also note that if one wishes to follow the more recent RFC3986 for URLs, which makes square brackets reserved (for IPv6) and thus not encoded when forming something which could be part of a URL (such as a host), the following code snippet may help:

function fixedEncodeURI(str) {
    return encodeURI(str).replace(/%5B/g, '[').replace(/%5D/g, ']');
}

So by not encoding square brackets in SIMPLE_ENCODE_SET, hosts could be encoded while leaving the brackets unchanged, while for other parts of the URL, it would be encoded.

@nox
Copy link
Contributor

nox commented Aug 13, 2019

This crate implements the WHATWG URL Living Standard, not the RFCs. The relevant spec for percent-encoding of queries is this.

@Nashenas88
Copy link
Author

Sorry I was not aware of that. I'll close this issue. Thanks for the quick reply!

@nox
Copy link
Contributor

nox commented Aug 13, 2019

Check the result of new URL("http://foo?bar[blah]=hello") in the browser of your choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants