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

Regression introduced in 9.1.1: parse cannot read back stringify when using bracket-seperator mode #398

Open
dubzzz opened this issue Nov 13, 2024 · 7 comments

Comments

@dubzzz
Copy link
Contributor

dubzzz commented Nov 13, 2024

Not sure if it was an expected impact of #392, but it seems it introduced a regression in 9.1.1.

The following piece of code does not behave the same in 9.1.0 and 9.1.1. In the later the parse function cannot read back itself.

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 });

Here are the output we get for each version:

// 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: [ ',' ] } }
@sindresorhus
Copy link
Owner

// @scottenock

@scottenock
Copy link
Contributor

Thanks @sindresorhus - taking a look 💪

@scottenock
Copy link
Contributor

Hi @sindresorhus + @dubzzz - I managed to take a look at this and have determined that 9.1.1 is behaving as expected.

@dubzzz , in your example you are using bracket-separator and not passing in an arrayFormatSeparator option, therefore the default separator of , will be used, resulting in the comma being split:

queryString.parse('foo=,', { arrayFormat: 'bracket-separator' }) // => { foo : ['', ''] }

Note that your example will still work and be Idempotent if you specify an arrayFormatSeparator that isn't the character you are hoping to preserve:

queryString.parse('foo=,', { arrayFormat: 'bracket-separator',  arrayFormatSeparator: '|' }) // => { foo : [','] }

Also note that in both 9.1.1 and 9.1.0 that this is true for the arrayFormat separator:

queryString.parse('foo=,', { arrayFormat: 'separator' }) // => { foo : ['', ''] }

If you're happy with my findings @sindresorhus I think we can close this out 💪

@dubzzz
Copy link
Contributor Author

dubzzz commented Dec 31, 2024

Well in such a case you have an issue for an input being:

const original = { key: ['|'] };

More precisely how to handle something like:

const original = { key: [[...Array(256)].map((_, i) => String.fromCodePoint(i)).join('')] };

@scottenock
Copy link
Contributor

Happy new year everyone! 🥳

@dubzzz I'm not sure what you mean with that example?

@dubzzz
Copy link
Contributor Author

dubzzz commented Jan 22, 2025

Let suppose I don't control the input of my user. My user could pass any string called input. If I get your point, executing the following should give me back the original input whatever the source input:

const inputStringified = queryString.stringify(input, {
  arrayFormat: "bracket-separator",
  arrayFormatSeparator: "|",
});
const output = queryString.parse(inputStringified, {
  arrayFormat: "bracket-separator",
  arrayFormatSeparator: "|",
});

Well, it's not the case following your change. If you take input = { key: ["|"] } you get output = { key: [ '', '' ] }.

Why is it an issue? Because it means that as a end user of the library we have to tweak the input to make it reversible. Before the change introduced in 9.1.1 it used to be reversible no matter the input.

Said differently:

Let say I want to use the arrayFormat bracket-separator with:

const input = {
  key: [
    [...Array(1_048_576)]
      .map((_, i) => String.fromCodePoint(i))
      .filter((_, i) => i < 0xd800 || i > 0xdfff)
      .join(""),
  ],
};

How can I do to make sure parse can reverse stringify? At the moment neither: queryString.parse(queryString.stringify(input, {arrayFormat: "bracket-separator"}), {arrayFormat: "bracket-separator"}) nor queryString.parse(queryString.stringify(input, {arrayFormat: "bracket-separator",arrayFormatSeparator: "|"}), {arrayFormat: "bracket-separator",arrayFormatSeparator: "|"}) will give me back my original value while it used to be the case in the previous version.

@scottenock
Copy link
Contributor

scottenock commented Jan 22, 2025

Thanks for the details @dubzzz , I think we need @sindresorhus's input on this.

As I have mentioned the behaviour outlined in the suspected regression for the bracket-separator is the standard behaviour that the separator has always provided.

So, the change in 9.1.1 fixed #388 and brought the behaviour of bracket-separator inline with that of the separator, making them both consistent.

Here are the outputs we get for each version when using separator:

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

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

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

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

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

As the above shows, this behaviour goes back to version 7.1.3 (I haven't checked past that, but I'm pretty sure it would be same behaviour)

So my take is that this isn't a regression and is fixing the inconsistent behaviour of the separator-bracket. The behaviour that you want @dubzzz was a result of parse not correctly parsing encoded values, e.g. 'key=%2C'

Keen to get @sindresorhus thoughts on this, as if this is truly regression then this bug has existed since version 7.1.3

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

No branches or pull requests

3 participants