querystring.parse expands "+" always, regardless of the parser

[koblas]

• 8.11.3:
• MacOS 10.13.5:
• querystring:

The "+" is always decoded in a query string, you can note the behavior difference in the example below. Using the internal parser an "external" parser and the parser directly yield results that are different.

> querystring.parse('email=test+recipient')
{ email: 'test recipient' }
> querystring.parse('email=test+recipient', undefined, undefined, { decodeURIComponent: decodeURIComponent })
{ email: 'test recipient' }
> decodeURIComponent('test+recipient')
'test+recipient'

[mscdex]

The decoder functions (decodeURIComponent() or otherwise) are mainly used for resolving percent-encoded characters. '+' characters have been resolved to either spaces or '%20' strings (before strings get passed to decoder functions) in node since at least v0.7.5.

[koblas]

Not sure you understood --

decodeURIComponent() does resolve "+" characters correctly

querystring.parse() which says it uses decodeURIComponent by default (it doesn't), has a parser which decodes "+" signs incorrectly. In addition if you provide your own parser, it will continue to decode "+" signs.

Fundamentally, "+" signs should not be decoded by querystring.parse()

[mscdex]

querystring.parse() which says it uses decodeURIComponent by default (it doesn't), has a parser which decodes "+" signs incorrectly. In addition if you provide your own parser, it will continue to decode "+" signs.

It does use decodeURIComponent by default, but as I just said (emphasis mine):

'+' characters have been resolved to either spaces or '%20' strings (before strings get passed to decoder functions) in node since at least v0.7.5.

Fundamentally, "+" signs should not be decoded by querystring.parse()

That's debatable, but that would be changing behavior that has existed for the past 6 years, unless the change is incorporated in a backwards-compatible way (e.g. opt-in in some way).

[koblas]

Using the following two documents as reference:

• https://github.com/nodejs/node/blob/254058109f469f64b8ca23bb65a206abab380604/lib/querystring.js
• https://nodejs.org/api/querystring.html

From the docs:

decodeURIComponent The function to use when decoding percent-encoded characters in the query string. Default: querystring.unescape().
...
By default, the querystring.unescape() method will attempt to use the JavaScript built-in decodeURIComponent() method to decode. If that fails, a safer equivalent that does not throw on malformed URLs will be used.

The documentation doesn't mention query string handling of "+", it does mention that you can override the behavior of the decoder. When you override the decoder, you still have the same behavior.

[mscdex]

This is the key phrase from the documentation I believe (emphasis mine):

The function to use when decoding percent-encoded characters in the query string. Default: querystring.unescape()

[koblas]

In this instance, we're not dealing with percent-encoded characters, but rather the unfortunately ambiguous "+" character. I will agree that RFC3986 section 2.2 says that "+" should be percent-encoded, I would offer that querystring.parse(...{ decodeURIComponent: ... }) should allow you to override the decoding of the parameters.

[dmethvin]

The current behavior matches the HTML Living Standard. Browsers submit forms using Content-type: application/x-www-form-urlencoded regardless of whether the method is GET or POST. There shouldn't be a need to override the decoding unless it was improperly encoded by the sender. For that case, if you know the incoming input isn't right and you need literal +, could you s/\+/%2B/g before passing it to querystring() perhaps? I think that should be safe to do even on properly encoded strings since no plus signs should occur there. Repairing non-standard input seems like it should be outside the scope of a library function though.

https://url.spec.whatwg.org/#urlencoded-parsing

[sibyaugustine]

If we encode a string with encodeURIComponent() , It should be okay.
Example
encodeURIComponent('C++') => "C%2B%2B"
querystring.parse("C%2B%2B") => C++

But if we gives querystring.parse("C++") then it produce "C ". Which is acceptable it terms of html standards. What do you think ?

[watilde]

I read the above discussion and it seems like an expected behavior written in the querystring's documentation as @mscdex mentioned.

The function to use when decoding percent-encoded characters in the query string. Default: querystring.unescape()

In more detail, querystring.unescape works as same as decodeURIComponent which is the same behavior with whatwg's URL.

// decodeURIComponent converts `%20` to an empty string
decodeURIComponent('%20') // => " "

// whatwg-url has the same behavior:
Object.fromEntries(new URLSearchParams('email=test+recipent')) // => {email: "test recipent"}

Closing as resolved for now, but feel free to reopen when anyone has a point. Thank you.