URL fragment is incorrectly included in qs value when used client-side

[CydeWeys]

query_string.parse('email=fake@example.com&username=Dopey#fragment')

returns:

{email: "fake@example.com", username: "Dopey#fragment"}

node-querystring doesn't understand what a URL fragment is, and it incorrectly includes the URL fragment as part of the value of the last-submitted query string value when it should instead by ignoring it entirely.

[CydeWeys]

This gives an idea of how we're using query_string.parse() in our actual code, which results in the above-seen error:

query_string.parse(document.URL.split('?')[1]);

It may well be that we should be dropping the fragment on our end before passing it into query_string.parse(), but really, why isn't there some method we can call that will look at the entire URL and extract just the querystring? Is this not the most common use case? Why require every person using this library to independently write some string-parsing to remove the hostname, path, and fragments parts out of the URL? Shouldn't there just be a parseURL method that's written once, correctly, and we can call that?

[CydeWeys]

This fixes the issue for me, but it's a very cumbersome and unwieldy way to call query_string.parse() in the client-side situation, especially because every invocation of it would necessarily need to look like this. It's almost like we'd want to wrap the library function to always call it this way:

query_string.parse(document.URL.split('?')[1].split('#')[0]);

[buschtoens]

I don't really consider this to be an issue of qs. If we go to these lenghts, we'd also have to include a fully fledged URI parser. But, as the name suggests, we only care about the query string part, which the users have to isolate themselves. ;)

Is there a specific reason for using document.URL? Just use window.location.search and you're fine.

[CydeWeys]

Yeah, window.location.search makes more sense. Thanks. Basically whoever wrote the code on our end that was calling node-querystring didn't do it properly.

[buschtoens]

No problem, man! :)

[sebs]

Sounds like a docs issue

[buschtoens]

If you can find the error, I'll fix it.

— Sent from Mailbox for iPhone

[CydeWeys]

By the way, it should really be this:

query_string.parse(window.location.search.substring(1))

The substring is necessary to trim out the leading question mark that is included in the query-string part of the URL according to JavaScript's understanding of URLs. Now this may be more properly a concern of node_querystring: trim off a leading question mark if there is one? Otherwise, every invocation from client-side code needs to deal with that question mark.

[buschtoens]

If I recall the specs correctly, the query part may contain question marks itself. However using them (unescaped) is a bad practice.

Maybe we should really run the input through a simple RegEx first.

[CydeWeys]

Can a query string parameter name really contain an unescaped question mark as the first character? If it can't, then it should be unambiguous what to do if the first character of the passed-in query string is a question mark: trim it. If that is somehow valid (even though everything except Letters (A–Z and a–z), numbers (0–9) and the characters '.','-','~' and '_' is supposed to be URL-encoded no matter where it appears in the query string), then another question becomes, why provide someone with a gun if you know that their intention is to shoot themselves in the foot with it?

[buschtoens]

In the past we've sticked to the dogma of parsing whatever goes in. Users should be responsible for providing the correct input, which in 99% of the cases requires no work at all.

Use window.location.search on the client side.

[CydeWeys]

@silvinci window.location.search includes the leading question mark in the query string, which query_string.parse() proceeds to choke on. My only remaining requested change on this issue is to strip off the leading question mark if it's the first character passed to parse().