[Fix] arrayLimit means max count, not max index, in combine/merge/parseArrayValue

Author: ljharb

lib/parse.js

@@ -40,8 +40,8 @@ var parseArrayValue = function (val, options, currentArrayLength) {
         return val.split(',');
     }
 
-    if (options.throwOnLimitExceeded && currentArrayLength > options.arrayLimit) {
-        throw new RangeError('Array limit exceeded. Only ' + (options.arrayLimit + 1) + ' element' + (options.arrayLimit === 0 ? '' : 's') + ' allowed in an array.');
+    if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) {
+        throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.');
     }
 
     return val;
@@ -194,7 +194,7 @@ var parseObject = function (chain, val, options, valuesParsed) {
                 && options.parseArrays;
             if (!options.parseArrays && decodedRoot === '') {
                 obj = { 0: leaf };
-            } else if (isValidArrayIndex && index <= options.arrayLimit) {
+            } else if (isValidArrayIndex && index < options.arrayLimit) {
                 obj = [];
                 obj[index] = leaf;
             } else if (isValidArrayIndex && options.throwOnLimitExceeded) {

lib/utils.js

@@ -112,7 +112,7 @@ var merge = function merge(target, source, options) {
             return markOverflow(result, getMaxIndex(source) + 1);
         }
         var combined = [target].concat(source);
-        if (options && typeof options.arrayLimit === 'number' && combined.length - 1 > options.arrayLimit) {
+        if (options && typeof options.arrayLimit === 'number' && combined.length > options.arrayLimit) {
             return markOverflow(arrayToObject(combined, options), combined.length - 1);
         }
         return combined;
@@ -296,7 +296,7 @@ var combine = function combine(a, b, arrayLimit, plainObjects) {
     }
 
     var result = [].concat(a, b);
-    if (result.length - 1 > arrayLimit) {
+    if (result.length > arrayLimit) {
         return markOverflow(arrayToObject(result, { plainObjects: plainObjects }), result.length - 1);
     }
     return result;

test/parse.js

@@ -261,11 +261,11 @@ test('parse()', function (t) {
     });
 
     t.test('limits specific array indices to arrayLimit', function (st) {
-        st.deepEqual(qs.parse('a[20]=a', { arrayLimit: 20 }), { a: ['a'] });
-        st.deepEqual(qs.parse('a[21]=a', { arrayLimit: 20 }), { a: { 21: 'a' } });
+        st.deepEqual(qs.parse('a[19]=a', { arrayLimit: 20 }), { a: ['a'] });
+        st.deepEqual(qs.parse('a[20]=a', { arrayLimit: 20 }), { a: { 20: 'a' } });
 
-        st.deepEqual(qs.parse('a[20]=a'), { a: ['a'] });
-        st.deepEqual(qs.parse('a[21]=a'), { a: { 21: 'a' } });
+        st.deepEqual(qs.parse('a[19]=a'), { a: ['a'] });
+        st.deepEqual(qs.parse('a[20]=a'), { a: { 20: 'a' } });
         st.end();
     });
 
@@ -483,7 +483,7 @@ test('parse()', function (t) {
 
     t.test('allows overriding array limit', function (st) {
         st.deepEqual(qs.parse('a[0]=b', { arrayLimit: -1 }), { a: { 0: 'b' } });
-        st.deepEqual(qs.parse('a[0]=b', { arrayLimit: 0 }), { a: ['b'] });
+        st.deepEqual(qs.parse('a[0]=b', { arrayLimit: 0 }), { a: { 0: 'b' } });
 
         st.deepEqual(qs.parse('a[-1]=b', { arrayLimit: -1 }), { a: { '-1': 'b' } });
         st.deepEqual(qs.parse('a[-1]=b', { arrayLimit: 0 }), { a: { '-1': 'b' } });
@@ -1118,22 +1118,22 @@ test('parse()', function (t) {
         });
 
         st.test('throws error when array limit exceeded', function (sst) {
-            // 5 elements (indices 0-4), max index 4 > limit 3
+            // 4 elements exceeds limit of 3
             sst['throws'](
                 function () {
-                    qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5', { arrayLimit: 3, throwOnLimitExceeded: true });
+                    qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3, throwOnLimitExceeded: true });
                 },
-                new RangeError('Array limit exceeded. Only 4 elements allowed in an array.'),
+                new RangeError('Array limit exceeded. Only 3 elements allowed in an array.'),
                 'throws error when array limit is exceeded'
             );
             sst.end();
         });
 
         st.test('does not throw when at limit', function (sst) {
-            // 4 elements (indices 0-3), max index 3 = limit 3, should not throw
-            var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3, throwOnLimitExceeded: true });
+            // 3 elements = limit of 3, should not throw
+            var result = qs.parse('a[]=1&a[]=2&a[]=3', { arrayLimit: 3, throwOnLimitExceeded: true });
             sst.ok(Array.isArray(result.a), 'result is an array');
-            sst.deepEqual(result.a, ['1', '2', '3', '4'], 'all values present');
+            sst.deepEqual(result.a, ['1', '2', '3'], 'all values present');
             sst.end();
         });
 
@@ -1347,36 +1347,36 @@ test('DOS', function (t) {
 });
 
 test('arrayLimit boundary conditions', function (t) {
-    // arrayLimit is about max index, not length. With arrayLimit: 3, indices 0-3 are allowed (4 elements)
+    // arrayLimit is the max number of elements allowed in an array
     t.test('exactly at the limit stays as array', function (st) {
-        // 4 elements (indices 0-3), max index 3 = limit 3
-        var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3 });
-        st.ok(Array.isArray(result.a), 'result is an array when max index equals limit');
-        st.deepEqual(result.a, ['1', '2', '3', '4'], 'all values present');
+        // 3 elements = limit of 3
+        var result = qs.parse('a[]=1&a[]=2&a[]=3', { arrayLimit: 3 });
+        st.ok(Array.isArray(result.a), 'result is an array when count equals limit');
+        st.deepEqual(result.a, ['1', '2', '3'], 'all values present');
         st.end();
     });
 
     t.test('one over the limit converts to object', function (st) {
-        // 5 elements (indices 0-4), max index 4 > limit 3
-        var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5', { arrayLimit: 3 });
+        // 4 elements exceeds limit of 3
+        var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3 });
         st.notOk(Array.isArray(result.a), 'result is not an array when over limit');
-        st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3', 3: '4', 4: '5' }, 'all values preserved as object');
+        st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3', 3: '4' }, 'all values preserved as object');
         st.end();
     });
 
-    t.test('arrayLimit 1 with two values', function (st) {
-        // 2 elements (indices 0-1), max index 1 = limit 1, should be array
-        var result = qs.parse('a[]=1&a[]=2', { arrayLimit: 1 });
-        st.ok(Array.isArray(result.a), 'result is an array when max index equals limit');
-        st.deepEqual(result.a, ['1', '2'], 'both values preserved as array');
+    t.test('arrayLimit 1 with one value', function (st) {
+        // 1 element = limit of 1
+        var result = qs.parse('a[]=1', { arrayLimit: 1 });
+        st.ok(Array.isArray(result.a), 'result is an array when count equals limit');
+        st.deepEqual(result.a, ['1'], 'value preserved as array');
         st.end();
     });
 
-    t.test('arrayLimit 1 with three values converts to object', function (st) {
-        // 3 elements (indices 0-2), max index 2 > limit 1
-        var result = qs.parse('a[]=1&a[]=2&a[]=3', { arrayLimit: 1 });
+    t.test('arrayLimit 1 with two values converts to object', function (st) {
+        // 2 elements exceeds limit of 1
+        var result = qs.parse('a[]=1&a[]=2', { arrayLimit: 1 });
         st.notOk(Array.isArray(result.a), 'result is not an array');
-        st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3' }, 'all values preserved as object');
+        st.deepEqual(result.a, { 0: '1', 1: '2' }, 'all values preserved as object');
         st.end();
     });
 

test/utils.js

@@ -196,41 +196,42 @@ test('combine()', function (t) {
             s2t.end();
         });
 
-        // arrayLimit is max index allowed, so with limit 3, indices 0-3 (4 elements) are allowed
         st.test('exactly at the limit stays as array', function (s2t) {
-            // 4 elements (indices 0-3), max index 3 = limit 3
-            var combined = utils.combine(['a', 'b', 'c'], 'd', 3, false);
-            s2t.deepEqual(combined, ['a', 'b', 'c', 'd'], 'stays as array when max index equals limit');
+            var combined = utils.combine(['a', 'b'], 'c', 3, false);
+            s2t.deepEqual(combined, ['a', 'b', 'c'], 'stays as array when count equals limit');
             s2t.ok(Array.isArray(combined), 'result is an array');
             s2t.end();
         });
 
         st.test('over the limit', function (s2t) {
-            // 5 elements (indices 0-4), max index 4 > limit 3
-            var combined = utils.combine(['a', 'b', 'c', 'd'], 'e', 3, false);
-            s2t.deepEqual(combined, { 0: 'a', 1: 'b', 2: 'c', 3: 'd', 4: 'e' }, 'converts to object when over limit');
+            var combined = utils.combine(['a', 'b', 'c'], 'd', 3, false);
+            s2t.deepEqual(combined, { 0: 'a', 1: 'b', 2: 'c', 3: 'd' }, 'converts to object when over limit');
             s2t.notOk(Array.isArray(combined), 'result is not an array');
             s2t.end();
         });
 
-        st.test('with arrayLimit 0', function (s2t) {
-            // 1 element (index 0), max index 0 = limit 0, should stay as array
-            var combined = utils.combine([], 'a', 0, false);
-            s2t.deepEqual(combined, ['a'], 'stays as array with arrayLimit 0 and single element');
+        st.test('with arrayLimit 1', function (s2t) {
+            var combined = utils.combine([], 'a', 1, false);
+            s2t.deepEqual(combined, ['a'], 'stays as array when count equals limit');
             s2t.ok(Array.isArray(combined), 'result is an array');
             s2t.end();
         });
 
+        st.test('with arrayLimit 0 converts single element to object', function (s2t) {
+            var combined = utils.combine([], 'a', 0, false);
+            s2t.deepEqual(combined, { 0: 'a' }, 'converts to object when count exceeds limit');
+            s2t.notOk(Array.isArray(combined), 'result is not an array');
+            s2t.end();
+        });
+
         st.test('with arrayLimit 0 and two elements converts to object', function (s2t) {
-            // 2 elements (indices 0-1), max index 1 > limit 0
             var combined = utils.combine(['a'], 'b', 0, false);
-            s2t.deepEqual(combined, { 0: 'a', 1: 'b' }, 'converts to object when max index exceeds limit');
+            s2t.deepEqual(combined, { 0: 'a', 1: 'b' }, 'converts to object when count exceeds limit');
             s2t.notOk(Array.isArray(combined), 'result is not an array');
             s2t.end();
         });
 
         st.test('with plainObjects option', function (s2t) {
-            // 3 elements (indices 0-2), max index 2 > limit 1
             var combined = utils.combine(['a', 'b'], 'c', 1, true);
             var expected = { __proto__: null, 0: 'a', 1: 'b', 2: 'c' };
             s2t.deepEqual(combined, expected, 'converts to object with null prototype');