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

querystring inconsistent behaviour #33910

Closed
scucchiero opened this issue Jun 16, 2020 · 5 comments
Closed

querystring inconsistent behaviour #33910

scucchiero opened this issue Jun 16, 2020 · 5 comments
Labels
querystring Issues and PRs related to the built-in querystring module.

Comments

@scucchiero
Copy link

The module querystring presents odd behaviour and inconsistent across platforms.

On NodeJs v14.2 x86_64 GNU/Linux
Parsing an object with an empty array into a querystring yields & with no element.

const querystring = require('querystring')
> querystring.stringify({ a:2, b:[] })
'a=2&' // expected result --> a=2 or a=2&b= 

On Firefox 77.0.1 (64-bit)
Yet, when using the module in a react application with client side rendering the result is as follows:

import querystring from 'querystring'
> querystring.stringify({ a:2, b:[] }) // 'a=2'  

This issue can lead to some of the following malformed URLs which may fault some server parsers. (My experience was with WSGI server)

@jasnell
Copy link
Member

jasnell commented Jun 16, 2020

Note that the version running within Firefox is likely a browserified version that is derived from the Node.js version. That's a bit of a guess without seeing the code but it is possible for there to be differences in the way they are implemented. We do not provide support for browserified ports of the core modules.

@sapics
Copy link
Contributor

sapics commented Jun 17, 2020

I guess that a=2 would be more expected one, because querystring.stringify({ a: [], b: [] }) is empty string.
I try to fix and make a PR.

@sapics
Copy link
Contributor

sapics commented Jun 17, 2020

I create a PR #33918. There were some errors at early commit, now it is fixed.
If there are something I was missing or you have any additional information, I'd appreciate it if you make a comment here or PR ;)

@scucchiero
Copy link
Author

great job mate, that was pretty quick.
so far that is the only things that I caught.
I will make sure to post anything else me or my team notice.

@targos
Copy link
Member

targos commented Dec 27, 2020

Fixed by #33918

@targos targos closed this as completed Dec 27, 2020
@targos targos added the querystring Issues and PRs related to the built-in querystring module. label Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
querystring Issues and PRs related to the built-in querystring module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants