Skip to content

Stringify with arrayFormat: comma, wrongfully encode comma in return #337

Description

@Om4ar

wrong result with stringifying

qs.stringify({ a: ['b', 'c'] }, { arrayFormat: 'comma' })
return a=b%2Cc

when it should return a=b,c

because parsing , as reserved character per RFC 3986 should split the array if we used a parser withcomma: true option
and %2C (percent encoded comma) should be treated normally as text character

related to : #336 pr

Activity

  1. ljharb commented on May 3, 2020

    @ljharb
    Owner

    I believe this should be fixed with #361?

  2. coatesap commented on Nov 17, 2020

    @coatesap

    This doesn't actually appear to have been fixed. The latest version still outputs a=b%2Cc. In fact the unit test also confirms that is the current behaviour:

    st.equal(
      qs.stringify({ a: ['b', 'c', 'd'] }, { arrayFormat: 'comma' }),
      'a=b%2Cc%2Cd',
      'comma => comma'
    );

    Surely this is wrong though, as it means you can't encode values that have commas in themselves?

  3. ljharb commented on Nov 17, 2020

    @ljharb
    Owner

    You can, it will double-encode them i believe. A PR adding those as test cases would be appreciated.

  4. andredewaard commented on Dec 18, 2020

    @andredewaard

    Is there a workaround for this?

  5. ljharb commented on Dec 18, 2020

    @ljharb
    Owner

    @andredewaard it's not clear there's anything to be worked around. A PR with failing test cases would help to quickly result in an answer or a fix.

  6. ananiy commented on Dec 21, 2020

    @ananiy

    using { encode: false } options to fix this

  7. coatesap commented on Jan 13, 2021

    @coatesap

    I've just tested this, and it doesn't double encode them. Which means that currently you can't send value that contain commas when using commas for the arrayFormat.

    qs.stringify({ a: ['b,c', 'd,e'] }, { arrayFormat: 'comma', encode: true }) 
    // output: a=b%2Cc%2Cd%2Ce
    // decoded: a=b,c,d,e

    Should this issue be re-opened or a new more-specific one started?

  8. ljharb commented on Jan 13, 2021

    @ljharb
    Owner
  9. Simba14 commented on Jan 20, 2021

    @Simba14

    Hey @ljharb, what's the status of the draft #338? Would be great to have it merged in 👌

  10. ljharb commented on Jan 21, 2021

    @ljharb
    Owner

    @Simba14 i left a review comment in november 2019, and the OP hasn't responded since. If you'd like to help, please feel free to comment on that PR with a link to an updated branch/commit (please do not open a new PR).

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions