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

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

Closed
Om4ar opened this issue Oct 18, 2019 · 10 comments · May be fixed by #338
Closed

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

Om4ar opened this issue Oct 18, 2019 · 10 comments · May be fixed by #338

Comments

@Om4ar
Copy link
Contributor

Om4ar commented Oct 18, 2019

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

@ljharb
Copy link
Owner

ljharb commented May 3, 2020

I believe this should be fixed with #361?

@coatesap
Copy link

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?

@ljharb
Copy link
Owner

ljharb commented Nov 17, 2020

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

@andredewaard
Copy link

Is there a workaround for this?

@ljharb
Copy link
Owner

ljharb commented Dec 18, 2020

@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.

@ananiy
Copy link

ananiy commented Dec 21, 2020

using { encode: false } options to fix this

@coatesap
Copy link

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?

@ljharb
Copy link
Owner

ljharb commented Jan 13, 2021

@coatesap See #338.

@Simba14
Copy link

Simba14 commented Jan 20, 2021

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

@ljharb
Copy link
Owner

ljharb commented Jan 21, 2021

@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
Projects
None yet
6 participants