Duplicates set to last, should not apply to arrays without index

[ronnyhaase]

Hi!

Maybe I'm mistaken, but I think when duplicates is set to "last", it should not apply to arrays without indices:

qs.parse("a=1&a=2&b[]=1&b[]=2", {duplicates: "last"})
results in
{ a: '2', b: [ '2' ] }

Where a is correct, but b was not what I expected.

[ljharb]

I agree, this is a bug.

[rotdrop]

Any news on this issue? Just stumbled over it ...

[rotdrop]

For my particular case the following patch fixes the problem. However, I am not familiar with the qs package:

diff --git a/node_modules/qs/lib/parse.js b/node_modules/qs/lib/parse.js
index d25be00..5f7f322 100644
--- a/node_modules/qs/lib/parse.js
+++ b/node_modules/qs/lib/parse.js
@@ -129,7 +129,7 @@ var parseValues = function parseQueryStringValues(str, options) {
         }
 
         var existing = has.call(obj, key);
-        if (existing && options.duplicates === 'combine') {
+        if (existing && (options.duplicates === 'combine' || part.indexOf('[]=') > -1)) {
             obj[key] = utils.combine(obj[key], val);
         } else if (!existing || options.duplicates === 'last') {
             obj[key] = val;

[wdhwg001]

In general, I believe with {duplicates: "last"}:

• a[]=1&a[]=2&a=3 should produce { a: 3 }
• a[0]=1&a[1]=2&a=3 should produce { a: 3 } as well
• a=1&a[]=2&a[]=3 should produce { a: [2, 3] }
• a=1&a[0]=2&a[1]=3 should produce { a: [2, 3] } as well
• a[]=1&a=2&a[]=3 should produce { a: [3] }
• a[0]=1&a=2&a[1]=3 should produce { a: [3] }, and with allowSparse: true it'd be [,3]
• a[]=1&a=2&b=f&a[]=3 should produce { a: [3], b: "f" }
• a[0]=1&a=2&b=f&a[1]=3 should produce { a: [3], b: "f" }, and with allowSparse: true it'd be [,3]

The idea is that a continued part of an array of a until duplicates should be considered as a whole when handling duplicates.

Also we should carefully handle []= as "a[]" should produce { a: [null] } with {strictNullHandling: true}

[ljharb]

Looking at it now, if you have [] on the key then the duplicates setting indeed doesn’t apply. It’s only for the implicit repetition of unbracketed keys.

I agree with the most recent comment, and the example in the OP would yield [‘1’, ‘2’].

[wdhwg001]

Thanks for the reply.

I believe the duplicates option is meant to prevent values from composing array unexpectedly. Its three options detailed its behavior:

• "combine" means array will be composed from plain keys without [] or [{int}] suffix.
• "first" means if key duplicates, the first value will be used and we won't compose an array.
• "last" means the last value will be used.

Therefore, I come to some conclusions:

• For keys with index suffix, composing an object or array is the expected behavior.
  • After composing, the index suffix will be removed, and the value will be the final value of this "K-V pair group".
• For keys without index suffix, duplicates option controls whether it will generate an array as its final value.
• If there are duplicates occurred on the final value level, duplicates option (or perhaps another option in the future) controls whether they shell be further combined.
  • It means, a[foo]=x&a=y should be combined to { a: [{ foo: "x" }, "y"] } when "combine" is chosen, and will be { a: "y" } when "last" is chosen.

I just tried the examples above with qs 6.14.0, but it gives me unexpected results:

• a[]=1&a[]=2&a=3 should produce { a: "3" }, because there are two "final values": ["1", "2"] and "3", and we take the last one.
  • However, it gives { a: ["2", "3"] }. It means the first a[] is ignored because of the duplicates: "last", and the second a[]=2 is combined with a=3 as the result.
• a[0]=1&a[1]=2&a=3 should produce { a: "3" } as well
  • However, it produces { a: ["1", "2", "3"] }. It means a[0], a[1] and a are treated as three different keys that does not trigger the duplication logic.
• a=1&a[]=2&a[]=3 should produce { a: ["2", "3"] }, because there are two final values of a: "1" and ["2", "3"], and we choose the last final value.
  • However, it creates { a: ["1", "3"] }. It means a, a[] are treated as two different keys, and then get combined into an array.
• a=1&a[0]=2&a[1]=3 should produce { a: ["2", "3"] } as well
  • However, it now gives { a: ["1", "2", "3"] }.
• a[]=1&a=2&a[]=3 should produce { a: ["3"] }, because here we have three final values: ["1"], "2" and ["3"]. The a=2 cuts the array into two pieces.
  • However, currently it's { a: ["3", "2"] }.
• a[0]=1&a=2&a[1]=3 should produce { a: ["3"] }, and with allowSparse: true it'd be [,3]
  • However, now it's { a: ["1", "2", "3"] }.
• a[]=1&a=2&b=f&a[]=3 should produce { a: ["3"], b: "f" }
  • However, it's { a: ["3", "2"], b: "f" }.
• a[0]=1&a=2&b=f&a[1]=3 should produce { a: [3], b: "f" }, and with allowSparse: true it'd be [,3]
  • However, currently it gives { a: ["1", "2", "3"] }.

[anasdevv]

I've been looking into this and want to confirm my understanding before fixing it. The issue: The parser treats a, a[], and a[1] as separate keys. Since it parses left-to-right, it doesn't know if a key should be a single value or array until later. Proposed fix: Track when a key changes type and remove old entries when that happens. Does this sound right? Open to other approaches if you have suggestions. Thanks!

[ljharb]

As long as it passes reasonable regression tests, that solution seems as good as any other. I look forward to reviewing the PR.