qs.parse doesn't handle encoded comma when parsing array with comma option

[thangpham7793]

Hello this is my first time opening an issue here! First of all thank you for this amazing library.

Basically my company is following jsonapi's spec for formatting filter in URL

GET /comments?filter[post]=1,2 HTTP/1.1

This works fine if the comma is not encoded

it('should parse post into an array', () => {
    expect(
      qs.parse('filter[post]=1,2', {
        comma: true,
      })
    ).toStrictEqual({ filter: { post: ['1', '2'] } })
  })

Atm our http library automatically encodes comma, which I suspect makes parse treat the comma-separated list as just a string.

it('should parse post into an array', () => {
    expect(
      qs.parse('filter[post]=1%2C2', {
        comma: true,
      })
    ).toStrictEqual({ filter: { post: ['1', '2'] } })
})

This test fails and the returned array is a string, "1,2".

Please let me know if there's anything that I miss that would help.

P/S: After reading the source code and the tests I think maybe this feature is not supported.

t.test('arrayFormat: comma allows only comma-separated arrays', function (st) {
        st.deepEqual(qs.parse('a[]=b&a[]=c', { arrayFormat: 'comma' }), { a: ['b', 'c'] });
        st.deepEqual(qs.parse('a[0]=b&a[1]=c', { arrayFormat: 'comma' }), { a: ['b', 'c'] });
        st.deepEqual(qs.parse('a=b,c', { arrayFormat: 'comma' }), { a: 'b,c' });
        st.deepEqual(qs.parse('a=b&a=c', { arrayFormat: 'comma' }), { a: ['b', 'c'] });
        st.end();
    });

If you think this is not a bug but rather a missing feature, do you think it should be included in the lib?

[ljharb]

The arrayFormat comma feature treats unencoded commas differently from encoded ones. Encoded commas are not special delimiters, they're just text - the , character. Unencoded commas are special delimiters.

Thus, if your http library always unconditionally encodes the comma, and you can't fix that, then your options are:

1. use a different arrayFormat ("brackets" is the most common, and will be the default in the next semver-major)
2. use a custom encoder/decoder with qs. I'm not certain how/if this will work, but i'm happy to explore a way to make this work for you.

[Ugzuzg]

Relevant commit with the change (qs@6.8.2): https://github.com/ljharb/qs/commit/0ece6d886a6e59589f0b5e753e1d0a3c98bc1b3c

We're also using qs to stringify the query with arrayFormat: 'comma', and it encodes the commas. This means qs isn't able to parse its own output.

qs.parse(qs.stringify({ filters: { status: ['a', 'b'] } }, { arrayFormat: 'comma' }), { comma: true });
/*
Outputs
{
  "filters": {
    "status": "a,b"
  }
}
*/

[ljharb]

I do see that qs.stringify({ filters: { status: ['a', 'b'] } }, { arrayFormat: 'comma', encodeValuesOnly: true }) produces filters[status]=a%2Cb instead of filters[status]=a,b as i'd expect.

qs.parse('filters[status]=a,b', { comma: true }) produces { filters: { status: [ 'a', 'b' ] } } so this seems like a stringify bug more than anything else?

[ljharb]

aha, yes - duplicate of #387, related to #237?

I'm not sure if this can be fixed without it being a breaking change; i'll look into it.

[Ugzuzg]

I don't think #237 is the solution to the problem above. I agree, that my comment is a duplicate of #387. I only commented in this issue, because it seemed relevant to me at the time.

[ljharb]

I've made a branch with 5 failing tests - 2 of which I don't actually know how it would be possible to represent (nested arrays inside an object using the comma format).

A PR into that branch that improves the tests and/or provides a solution would be much appreciated.

[tomasztomys]

+1

[adnan-creator]

Please assign this issue to me, I am working on a PR.