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

FIX: Decode back-separator value before splitting it with separator #392

Merged

Conversation

scottenock
Copy link
Contributor

The original implementation was splitting the received value before it then decoded the value. Because the value was URL encoded, the split function could not find the separator to successfully split. Adjusted the order of operations which fixes #388

  • Adjusted order of operations, ensuring we decode before splitting the value
  • Implemented test case to guard against this regression.

@scottenock scottenock changed the title FIX: Decode brack-seperator value before splitting it with seperator FIX: Decode brack-seperator value before splitting it with separator Sep 19, 2024
@scottenock scottenock changed the title FIX: Decode brack-seperator value before splitting it with separator FIX: Decode back-separator value before splitting it with separator Sep 19, 2024
test/parse.js Outdated Show resolved Hide resolved
The original implementation was splitting the received value before it
then decoded the value. Because the value was URL encoded, the split
function could not find the seperator to successfully split. Adjusted
the order of operations which fixes #388
@scottenock scottenock force-pushed the fix/decode-bracket-seperator branch from a0d90c5 to 245ffe0 Compare October 1, 2024 18:37
@scottenock scottenock force-pushed the fix/decode-bracket-seperator branch from 245ffe0 to 8c0335d Compare October 1, 2024 18:42
@scottenock
Copy link
Contributor Author

Thanks for reviewing @sindresorhus 🙌

I've adjusted the test to be more readable with regards to the encoded strings. There's some existing tests that are using encoded values without showing the un-encoded version. Do you want me to adjust these to conform to your comment?

@sindresorhus
Copy link
Owner

Do you want me to adjust these to conform to your comment?

Yeah, that would be great.

@scottenock
Copy link
Contributor Author

Sweet @sindresorhus, I've updated the tests and they should be more readable. Let me know if there's any issues.

@sindresorhus sindresorhus merged commit 19c43d4 into sindresorhus:main Oct 9, 2024
2 checks passed
@sindresorhus
Copy link
Owner

Thanks :)

@dubzzz
Copy link
Contributor

dubzzz commented Nov 8, 2024

Not sure if it was an expected change but with 9.1.1 if we run the code below:

import queryString from 'query-string';

const original = { key: [','] };
console.log({ original });

const stringified = queryString.stringify(original, { arrayFormat: 'bracket-separator' });
console.log({ stringified });

const parsed = queryString.parse(stringified, { arrayFormat: 'bracket-separator' });
console.log({ parsed });

We get something different from what we had in 9.1.0. At some point the parse function is not able to read back the string produced by stringify.

// in 9.1.1
{ original: { key: [ ',' ] } }
{ stringified: 'key[]=%2C' }
{ parsed: [Object: null prototype] { key: [ '', '' ] } }

// in 9.1.0
{ original: { key: [ ',' ] } }
{ stringified: 'key[]=%2C' }
{ parsed: [Object: null prototype] { key: [ ',' ] } }

I suspect that it might not be totally expected but maybe I'm wrong.

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

Successfully merging this pull request may close these issues.

Bug: comma separated arrays not parsed correctly from encoded url
3 participants