arrayLimit does not throw error when throwOnLimitExceeded flag is true

[radualex]

Hello,

I have used the throwOnLimitExceeded flag to throw when limits are exceeded. However, it seems that arrayLimit does not throw as expected, and it returns the object format when limit is exceeded, which I think is the default behavior.

This is the configuration:

Based on this configuration, I would expect the following test to pass:

Instead I received the object format:

    - Expected  - 1
    + Received  + 5

    - Object {}
    + Object {
    +   "a": Object {
    +     "1001": "b",
    +   },
    + }

[ljharb]

What version of qs are you using?

Looking at #517, it seems like perhaps the issue might be because you're making a sparse array and aren't setting allowSparse to true. What happens if you do that?

[radualex]

@ljharb Thanks for the reply! I am using qs@6.14.0. I've tried allowSparse set to true and I get the same result.

[ljharb]

Interesting. That does feel like a bug.

One more thing to try - if you actually have N + 1 items in the array (as opposed to just setting the N+1th index), what happens?

[radualex]

@ljharb I've changed the arrayLimit to 10 and tested with this input: a[0]=b&a[1]=b&a[2]=b&a[3]=b&a[4]=b&a[5]=b&a[6]=b&a[7]=b&a[8]=b&a[9]=b&a[10]=b&a[11]=b. I get the same object format response instead of the error throw:

 - Expected  -  1
    + Received  + 16

    - Object {}
    + Object {
    +   "a": Object {
    +     "0": "b",
    +     "1": "b",
    +     "10": "b",
    +     "11": "b",
    +     "2": "b",
    +     "3": "b",
    +     "4": "b",
    +     "5": "b",
    +     "6": "b",
    +     "7": "b",
    +     "8": "b",
    +     "9": "b",
    +   },
    + }

[ljharb]

Thanks, then either the docs are wrong or there's a bug. Will look into it.

[Devanshshukla08]

Since the index "1001" is greater than the "arrayLimit" of 1000, "qs" parses a as an object, not an array. It creates an object with a key named 'a' which in turn has a property named '1001'.
So because of this - the output is { a: { '1001': 'b' } }. this is the intended behavior from the library's perspective, so no error is thrown, and your catch block is never executed.

To test if your "try...catch" block and "throwOnLimitExceeded" flags are working, you should exceed a limit that actually throws an error, such as "depth". Your configuration has "depth: 10"

[radualex]

@Devanshshukla08 As I understand, parsing arrays as objects when the limit is exceeded is the default behavior when throwOnLimitExceeded is false, but as soon as the flag is true, then the expected behavior changes, and an error should be thrown when the array limit is exceeded.

The depth and parameterLimit are working as expected for throwOnLimitExceeded: true.

[Jayesh-11]

Hi @ljharb
RCA - the issue was that the arrayLimit with throwOnLimitExceeded should work with allowSparse but it does not work
since the index is greater than the array limit it becomes an object without throwing error, hence creating objects where it should be throwing error instead of silently converting to object

Plus, the issue example should be used with allowSprase based on this guide in docs
since in that case only the length of the array would exceed the actual max

// parse.js

    } else if (
                !isNaN(index)
                && root !== decodedRoot
                && String(index) === decodedRoot
                && index >= 0
                && (options.parseArrays && index <= options.arrayLimit)   --> this is the part that fails i.e false
    ) {

consider following example

const a = parse("a[99]", {
  ignoreQueryPrefix: true,
  arrayLimit: 10,
  parameterLimit: 10,
  depth: 10,
  strictDepth: true,
  throwOnLimitExceeded: true,
  allowSparse: true,
});
RESULT : { a: { '99': '' } } // which seems incorrect even though we allow sprase here and should be throwing error instead



const a = parse("a[0]=1&a[1]=2&a[2]=3&a[10]=4", {
  ignoreQueryPrefix: true,
  arrayLimit: 6,
  parameterLimit: 10,
  depth: 10,
  strictDepth: true,
  throwOnLimitExceeded: true,
  allowSparse: true,
});
RESULT : { a: { '0': '1', '1': '2', '2': '3', '10': '4' } } // which seems incorrect even though we allow sprase here and should be throwing error instead

the proposed solution is throw early error based on index check attached in #532
which throws error in above example

please share any feedback or insight on above
Thanks!