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

Stringifying using different arrayFormats, then parsed, create different things #345

Open
wholien opened this issue Nov 15, 2019 · 16 comments · Fixed by #361
Open

Stringifying using different arrayFormats, then parsed, create different things #345

wholien opened this issue Nov 15, 2019 · 16 comments · Fixed by #361

Comments

@wholien
Copy link

wholien commented Nov 15, 2019

It seems stringifying my metadata object with arrayFormat: brackets, then parsing it, creates a different object to stringifying with arrayFormat: indices then parsing that string.

> const qs = require('qs');

> metadata = [{key: "a", value: "2"}, {key: "b", value: "3"}]
[ { key: 'a', value: '2' }, { key: 'b', value: '3' } ]

> brackets = qs.stringify({"metadata": metadata}, { arrayFormat: "brackets"})
'metadata%5B%5D%5Bkey%5D=a&metadata%5B%5D%5Bvalue%5D=2&metadata%5B%5D%5Bkey%5D=b&metadata%5B%5D%5Bvalue%5D=3'

> parsed_brackets = qs.parse(brackets)
{ metadata: [ { key: [Array], value: [Array] } ] }

> parsed_brackets["metadata"]
[ { key: [ 'a', 'b' ], value: [ '2', '3' ] } ]

> indices = qs.stringify({"metadata": metadata}, { arrayFormat: "indices"})
'metadata%5B0%5D%5Bkey%5D=a&metadata%5B0%5D%5Bvalue%5D=2&metadata%5B1%5D%5Bkey%5D=b&metadata%5B1%5D%5Bvalue%5D=3'

> parsed_indices = qs.parse(indices)
{ metadata: [ { key: 'a', value: '2' }, { key: 'b', value: '3' } ] }

Am I doing something wrong? Is this expected? Is this just an issue with stringifying/parsing nested objects?

@Darwin-Gowago
Copy link

Having the same issues here. Is there any update on this?

@ljharb
Copy link
Owner

ljharb commented Nov 22, 2019

It's entirely reasonable that the same string produces different results when you use different array formats.

In this case, it seems like "brackets" does have a bug, whether in stringify, parse, or both.

@wholien
Copy link
Author

wholien commented Nov 22, 2019

judging from other query string libraries, it might require specifying the arrayFormat in both stringify and parse

@ljharb
Copy link
Owner

ljharb commented Nov 23, 2019

ohhhh right, it absolutely does :-) i missed that. indices is the default arrayFormat.

@wholien what happens if you specify brackets in your first parse?

@wholien
Copy link
Author

wholien commented Nov 25, 2019

@ljharb doing it with brackets:

> metadata = [{key: "a", value: "2"}, {key: "b", value: "3"}]
[ { key: 'a', value: '2' }, { key: 'b', value: '3' } ]

> brackets = qs.stringify({"metadata": metadata}, { arrayFormat: "brackets"})
'metadata%5B%5D%5Bkey%5D=a&metadata%5B%5D%5Bvalue%5D=2&metadata%5B%5D%5Bkey%5D=b&metadata%5B%5D%5Bvalue%5D=3'

> parsed_brackets = qs.parse(brackets, { arrayFormat: "brackets"}) // <---- here
{ metadata: [ { key: [Array], value: [Array] } ] }

> parsed_brackets["metadata"]
[ { key: [ 'a', 'b' ], value: [ '2', '3' ] } ]

I was reading through the readme and some of the parse code - doesn't look like there is an option for setting arrayFormats, though I could be wrong.

@ljharb
Copy link
Owner

ljharb commented Nov 25, 2019

Seems like we do have an issue here then.

I agree with the general premise that for any format parse accepts, or stringify produces, the other should be able to round trip (ie, stringify(parse(q)) === q modulo normalizations, and parse(stringify(q)) =~ q where =~ is a reasonable deep equal algorithm modulo normalizations).

Happy to accept a PR here, but I'm not sure concretely what it should change.

Failing test cases are probably the best place to start!

@nigeldunne
Copy link

On the most recent version, an array stringified and parsed returns a string:

> qs.stringify({ foo: [ 'a', 'b' ] }, { arrayFormat: 'comma' })
'foo=a%2Cb'
> qs.parse('foo=a%2Cb', { comma: true })
{ foo: 'a,b' }

where as previously an array is returned:

> qs.parse('foo=a%2Cb', { comma: true })
{ foo: [ 'a', 'b' ] }

@ljharb
Copy link
Owner

ljharb commented Apr 15, 2020

@nigeldunne in this case, %2c isn't a comma to be used for splitting. if you parse foo=a,b it will split properly.

I'd say the issue is that stringify needs to produce an unescaped comma when arrayFormat is "comma".

@nigeldunne
Copy link

@ljharb sounds reasonable. I think the key is your point about the ability to do a "round trip".

@ljharb
Copy link
Owner

ljharb commented May 3, 2020

Fixed in #361, I think?

@ljharb ljharb closed this as completed May 3, 2020
@nigeldunne
Copy link

nigeldunne commented May 4, 2020

@ljharb The "round trip" still does not work when arrayFormat is "comma":

qs.stringify({ foo: [ 'a', 'b' ] }, { arrayFormat: 'comma' })
'foo=a%2Cb'
qs.parse('foo=a%2Cb', { comma: true })
{ foo: 'a,b' } //expecting { foo: [ 'a', 'b' ]

@ljharb
Copy link
Owner

ljharb commented May 4, 2020

@nigeldunne thanks, reopening

@ljharb ljharb reopened this May 4, 2020
@smithb1994

This comment has been minimized.

@ljharb

This comment has been minimized.

@adnan-creator

This comment has been minimized.

@tzelon-cypator
Copy link

On the most recent version, an array stringified and parsed returns a string:

> qs.stringify({ foo: [ 'a', 'b' ] }, { arrayFormat: 'comma' })
'foo=a%2Cb'
> qs.parse('foo=a%2Cb', { comma: true })
{ foo: 'a,b' }

where as previously an array is returned:

> qs.parse('foo=a%2Cb', { comma: true })
{ foo: [ 'a', 'b' ] }

For now, I use this solution

qs.parse(str, {
      comma: true,
      decoder(str, defaultDecoder, charset, type) {
        const decoded = defaultDecoder(str);
        if (type === "value" && typeof decoded === "string" && decoded.includes(",")) {
          return decoded.split(",");
        }
        return decoded;
      },
    })

probably not the best solution but it is working for my case.

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

Successfully merging a pull request may close this issue.

7 participants