fix: restore array parsing for req.query repeated keys (#7147)

[SAY-5]

Closes #7147.

The bump to qs@~6.14.1 in 4.22.0 (#6972) pulled in qs's new default arrayLimit of 20, which causes the extended query parser to silently collapse repeated-key query strings with more than 20 values into an object (e.g. { "0": "a", "1": "b", ... }) instead of producing an array. This broke consumers that had been passing larger batches of repeated parameters through req.query.ids (the issue reporter hits it with ~20 tenancy IDs).

body-parser already got an opt-out path for the extended URL-encoded middleware, but the parseExtendedQueryString used by req.query for the 'extended' query parser still only passes allowPrototypes: true and otherwise inherits qs defaults — so the regression is only visible on req.query, which is what #7147 reports.

This PR restores the pre-6.14 behavior by passing arrayLimit: Infinity arrayLimit: 1000 to qs.parse inside parseExtendedQueryString. Repeated-key values of any length round-trip back to an array, matching Express <= 4.21.x.

Test plan

• Added a regression test in test/req.query.js that sends 25 repeated ids values and asserts req.query.ids is an array of 25 strings. Verified the test fails on 4.x without the fix and passes with it.
• Full test/req.query.js suite: 11 passing.

If it would help, I'm happy to open a matching PR against 5.x — the same parseExtendedQueryString lives there with the same defaults.

[krzysdz]

Infinity completely removes protection from sending something like arr[999999]=x and allows creating large arrays by sending just 1 parameter with an index.

PR #7151 (to master) uses a value of 1000 (same as the parameter limit), which is a better idea, but it still relaxes the limit compared to qs 6.14.0.

[SAY-5]

Thanks @krzysdz — good catch. Updated to arrayLimit: 1000 to match PR #7151 and the parameter limit used elsewhere. That still lets apps pass repeated-key arrays up to 1000 elements (which covers the failing real-world case in the issue) while bounding single-parameter expansions like arr[999999]=x.

Also added a second regression test asserting that arr[9999]=x now collapses to an object instead of allocating a sparse array, so the arrayLimit bound stays covered.

[jonchurch]

I pushed an update to drop refs to the specific issue (prefer commit history for that) and to drop the sparse array test which is not related to the regression.

[jonchurch]

For clarity, the specific root cause and knock on effect is:

qs's arrayLimit default of 20 was historically not enforced on repeated-key parsing.

qs 6.14.1 closed that gap, and Express 4.22.0 pulled the fix in via the qs@~6.14.1 bump (#6972) — so ?ids=1&ids=2&... past 20 values silently flips req.query.ids from array to object on default config.

This PR sets arrayLimit: 1000 in parseExtendedQueryString (matching parameterLimit) to restore the repeated-key behavior Express 4.x has historically supported.

Note on indexed-bracket notation: qs uses arrayLimit as a single knob across three parsing paths (repeated keys, empty-bracket arr[]=, and indexed-bracket arr[N]=). For indexed-bracket queries at indices 21..999, this PR widens the threshold past 4.21's effective limit of 20 — ?arr[500]=v will now parse as an array ["v"] rather than an object {"500":"v"}.

EDIT: see update and change suggestion #7181 (comment)

This is, to me, a surprising side effect. But I do think it is reasonable as my own experience expects that the repeated key form is more common. Open to input here though

[ljharb]

it'd actually parse as an array where index 500 will be v, not index 0, so virtually all usage patterns will work the same.

[jonchurch]

@ljharb this is how Im testing it:

// express 4.21 baseline (qs 6.13.0, default arrayLimit:20)
qs.parse('arr[500]=v')
// { arr: { '500': 'v' } }

// Proposed fix (qs 6.14.2, arrayLimit:1000)
qs.parse('arr[500]=v', { arrayLimit: 1000 })
// { arr: [ 'v' ] }

arr[500]=v flips from object (key "500") to an array (value at index 0)

[ljharb]

ah right, you'd need the allowSparse: true option for the index to remain correct.

[jonchurch]

with allowSparse: true we get a sparse array, but it doesnt restore the previous behavior of arr[500]=v coming back as an object.

[ljharb]

Definitely. But if you treat it like an object with 500 as a key, as someone's current code would, it would continue to work the same.

[SAY-5]

Right — that's the deciding factor. If the keyed access status[500] would still work after the change, it stays backward-compatible for everyone who's been treating it as a plain object. Should I rework the patch to keep the numeric-key access path intact (e.g. via a Proxy or a hybrid getter), or is the current shape OK?

[SAY-5]

Closing — using Infinity does remove the arr[999999] protection that #7151 was discussing. Wrong tradeoff on my side; thanks for catching it.

[jonchurch]

Reopened this, it is my preferred fix for the 4.x line, it is neat and tidy and fixes a regression in default settings that has been open too long.

PR restores the default extended req.query behavior 4.x had before, allowing up to 1000 array elements, by passing arrayLimit: 1000 when constructing the extended query parser.

[krzysdz]

Objects suddenly becoming arrays should not be a big problem (arrays can be used as objects, JS being JS...).

The thing that concerns me is that someone may have relied on e.g. ?filter[1001]=abc being accessible as req.query.filter[1001]. While this has never worked for ?filter[5]=abc (req.query.filter would be ['abc']), I wouldn’t be surprised if someone has used hardcoded numbers > 20 as keys.

I think we can try releasing this as is and see if anyone reports new issues. With the allowSparse: true alternative, there would be a difference for indices <20 (small arrays suddenly are sparse and have empty elements) and surely someone would report this as a security issue (DoS).

EDIT: If this becomes an issue again, here's a small demonstration of the old behaviour and possible changes to arrayLimit and allowSparse: https://gist.github.com/krzysdz/35d7ce9962a08534226ba91d51530d7c#file-results-md