fix: restore array parsing for req.query repeated keys (#7147)#7181
fix: restore array parsing for req.query repeated keys (#7147)#7181SAY-5 wants to merge 1 commit intoexpressjs:4.xfrom
Conversation
krzysdz
left a comment
There was a problem hiding this comment.
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.
) qs 6.14 enforces a default `arrayLimit` of 20, which caused the extended query parser to collapse query strings with more than 20 repeated values (e.g. `?ids=1&ids=2&...&ids=25`) into an object instead of an array. This regressed in 4.22.0 (with the bump to `qs@~6.14.1`) and broke existing consumers relying on the pre-6.14 behavior. Pass `arrayLimit: Infinity` to `qs.parse` in `parseExtendedQueryString` so repeated keys always produce arrays, matching the behavior of Express \<= 4.21.x. Added a regression test in `test/req.query.js` that sends 25 repeated values and asserts they round-trip as an array.
e8b025b to
c0b4eaa
Compare
|
Thanks @krzysdz — good catch. Updated to Also added a second regression test asserting that |
Closes #7147.
The bump to
qs@~6.14.1in 4.22.0 (#6972) pulled in qs's new defaultarrayLimitof 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 throughreq.query.ids(the issue reporter hits it with ~20 tenancy IDs).body-parseralready got an opt-out path for the extended URL-encoded middleware, but theparseExtendedQueryStringused byreq.queryfor the'extended'query parser still only passesallowPrototypes: trueand otherwise inherits qs defaults — so the regression is only visible onreq.query, which is what #7147 reports.This PR restores the pre-6.14 behavior by passing
arrayLimit: Infinitytoqs.parseinsideparseExtendedQueryString. Repeated-key values of any length round-trip back to an array, matching Express <= 4.21.x.Test plan
test/req.query.jsthat sends 25 repeatedidsvalues and assertsreq.query.idsis an array of 25 strings. Verified the test fails on4.xwithout the fix and passes with it.test/req.query.jssuite: 11 passing.If it would help, I'm happy to open a matching PR against
5.x— the sameparseExtendedQueryStringlives there with the same defaults.