ljharb / qs

A querystring parser with nesting support
BSD 3-Clause "New" or "Revised" License
8.56k stars 728 forks source link

RFC 3986 reserved characters missing #163

Open martinheidegger opened 8 years ago

martinheidegger commented 8 years ago

stringify is supposed to be implementing RFC 3986 but RFC 3986 specifies reserved characters as:

reserved    = gen-delims / sub-delims
gen-delims  = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                  / "*" / "+" / "," / ";" / "="

However $ (as pointed out in request#2129) will be encoded as %24. I guess this is a bug?

ljharb commented 8 years ago

It seems like that linked issue comes from querystring, not qs, but I'd certainly like to comply with RFC 3986.

A PR, with at least failing tests, would be very appreciated.

I may make this "bugfix" a breaking change, however, just to be safe.

vpanjganj commented 7 years ago

@ljharb can we just fix with something like this ? (it's on MDN doc for encodeURI)

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

@vpanjganj that's very unlikely to be the best solution, but i'd first need failing test cases to evaluate it.

dinoboff commented 6 years ago

I think #, [ and ] should be encoded in a query string; see RFC 3986 section 3:

unreserved = ALPHA / DIGIT / "-" / "." / "_" / "˜"
[...]
pct-encoded = "%" HEXDIG HEXDIG
[...]
sub-delims = "!" / "$" / "&" / "’" / "(" / ")" / "*" / "+" / "," / ";" / "="
[...]
pchar = unreserved / pct-encoded / sub-delims / ":" / "@"
[...]
query = *( pchar / "/" / "?" )
dinoboff commented 6 years ago

Note that URLSearchParams encode all reserved chars but *:

const qs = new URLSearchParams();

qs.set('sub-delims', "!$%'()*+,;=");
qs.set('other', ":@");
console.assert(qs.toString() === "sub-delims=%21%24%25%27%28%29*%2B%2C%3B%3D&other=%3A%40")

It's not because they are valid that they shouldn't be encoded.

ps: I don't think it's a bug, but a feature request.