`stringify` with `arrayFormat: 'comma'` does not encode commas in strings.

[dobromir-hristov]

Hello,

I noticed that calling qs.stringify({ query: 'some, query' }, { arrayFormat: 'comma', } would output ?query=some,query which, if parsed back, would generate an array { query: ['some', 'query'] }.

I looked at the code for stringify and I noticed this - https://github.com/ljharb/qs/blob/main/lib/stringify.js#L125

This will essentially split the string by the comma and then re-stitch it. I think this is used for handling nested objects with arrays in them? Not sure what would be the best way to fix this...

Edit: This would also leave commas in deep object values as well, which when parsed back would generate arrays again.

[ljharb]

Comma arrayFormat is super weird, and it's really unclear how it's expected to work.

That said, qs.parse('query=some%2C%20query', { arrayFormat: 'comma' }) produces { query: 'some, query' } so i think it's working correctly?

[dobromir-hristov]

Right. If stringify would encode the comma in strings, instead of treating as divider, it would be perfect.

[ljharb]

I'm not sure what you mean. It already round-trips correctly when you use the same stringify and parsing options.

What output would you expect?

[dobromir-hristov]

For me it outputs query=some,query instead of query=some%2C%20query when I stringify.

[ljharb]

What version of qs are you using?

[dobromir-hristov]

Latest one. Installed it 2 days ago.

[ljharb]

hmm, qs.stringify({ query: 'some, query' }, { arrayFormat: 'comma' }) === 'query=some%2C%20query' returns true for me in the repl for v6.11.0, so I'm not sure what to tell you there :-/

make sure require('qs/package.json').version is 6.11.0 from the same place you're requiring qs.

[dobromir-hristov]

Hey, sorry about not specifying, but I just noticed I also had encodeValuesOnly: true.

If I remove it, it properly encodes the for strings, but it also completely breaks the arrays:

qs.stringify({ query: 'foo,bar', queryArray: ['foo', 'bar'] }, { arrayFormat: 'comma' }) === 'query=foo%2Cbar&queryArray=foo%2Cbar';

qs.parse('query=foo%2Cbar&queryArray=foo%2Cbar', { comma: true }) ===  {query: 'foo,bar', queryArray: 'foo,bar'}

VS

qs.stringify({ query: 'foo,bar', queryArray: ['foo', 'bar'] }, { arrayFormat: 'comma', encodeValuesOnly: true }) === 'query=foo,bar&queryArray=foo,bar';

qs.parse('query=foo,bar&queryArray=foo,bar', { comma: true }) === {query: Array(2), queryArray: Array(2)}

[ljharb]

indeed, in the second example i get { query: [ 'foo', 'bar' ], queryArray: [ 'foo', 'bar' ] }.

There's no parse setting to invert encodeValuesOnly though.

What exactly is the behavior you want? If stringify is producing foo,bar for both values, then it would be very weird to produce a different JS representation of both values.

(this is why "comma" is a weird format; tbh "brackets" is the only format that actually makes sense to me)

[dobromir-hristov]

Hm. I thought "sanitize values" meant the values of the query parameters. In any case, the comma's in the arrays are delimiters, that should be left as is. Comma's in strings are to be sanitized.

If I have an array of strings and one has a comma, it will probably break the strings into multiple items as it is now.

The brackets format is good but is extremely verbose and long, compared to the comma arrays, especially for urls that are meant to be shared.

[ljharb]

Shorteners are typically what's used for URLs that are meant to be shared :-)

I do see a clear bug tho, as you've mentioned - qs.stringify({ query: 'foo,bar', queryArray: ['foo', 'bar,baz'] }, { arrayFormat: 'comma', encodeValuesOnly: true }) produces 'query=foo,bar&queryArray=foo,bar,baz', which strips the info that there's 2 items instead of 3.

[ljharb]

With encodeValuesOnly, i would expect a delimiter comma to be left alone, but an item comma to be encoded. What would you expect for the item comma when the delimiter commas are encoded?

[coatesap]

@ljharb - would you be able to review PR #463 as it would allow people to start using arrayFormat more widely, without worrying about commas in values?