fix: keep repeated extended query params as arrays beyond 20 values

[wwenrr]

Summary

• fix extended query parser behavior for repeated keys with >20 values
• set arrayLimit: 1000 in parseExtendedQueryString so repeated key form (a=1&a=2...) keeps returning arrays
• add regression test for 21 repeated tenancyIds values

Why

Issue #7147 reports that in 4.22.x repeated query keys switch from array to object once count exceeds 20, which is a behavior break for existing apps.

Testing

• ran test suite in Docker (node:22)
• validated req.query extended parser test including new regression case

[krzysdz]

As I wrote in #7147 (comment), this is something that, to my knowledge, has not been decided on. There are 3 potential solutions:

1. Do not fix it and guide users to configure their own "query parser" with appropriate options.
2. Follow body-parser and introduce new logic that sets arrayLimit to the number of parameters (expressjs/body-parser#689)
3. Set arrayLimit to an arbitrary higher value <= 1000. 1000 is the parameterLimit and highest value that makes sense (since qs 6.14.2 it restricts the array length to at most 1000 entries, in earlier versions (<6.14.1) it restricted the index to 1000, so 1001 elements). This, however, also significantly raises the default limit allowing much larger sparse arrays using array[idx] notation.

This PR and #7181 (for 4.x) choose the 3rd option, but the opinions in Slack thread from January were in favour of the 1st. There was no final decision on this, as @jonchurch was asked for his opinion on this and then everyone forgot about it.

This approach probably does not open any app for DoS, because creating arrays up to 1000 elements has been possible before, just using repetitions. qs has quadratic time complexity when parsing arrays (both indexed and using repetitions), but for 1000 elements it's not a problem. The one potentially affected pattern that I can think of right now is

for (const [k, v] of Object.entries(req.query)) {
    if (Array.isArray(v)) {
        for (let i = 0; i < v.length; ++i) {
            // ...
        }
    }
}

This (with these changes) would in total run 1000 (parameterLimit) * 1000 (arrayLimit) inner loop iterations, compared to 1000 * 21 (with qs <= 6.14.1) or 1000 * 20 (qs >= 6.14.2) - a 50x increase.

[bjohansebas]

My two cents (I don’t have time to go deep into this right now (my plate is full)), but hopefully @expressjs/triagers @expressjs/express-collaborators can help here, is that we shouldn’t leave it as is. We should come up with a solution that isn’t a breaking change, because this issue is an unintentional breaking change and it can break applications without people realizing it.