Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fbc5206

Browse filesBrowse files
Kai Gritunljharb
Kai Gritun
and
ljharb
committed
[Fix] parse: fix error message to reflect arrayLimit as max index; remove extraneous comments
The arrayLimit option is documented as limiting the 'maximum index' in an array, but was incorrectly being applied to the array length. This caused inconsistent behavior between indexed (param[0]=val) and non-indexed (param[]=val) array parameters. With arrayLimit: 3: - BEFORE: 4 elements (indices 0-3) would convert to object since length 4 > limit 3 - AFTER: 4 elements (indices 0-3) stays as array since max index 3 <= limit 3 Changes: - utils.js combine(): Check 'length - 1 > limit' instead of 'length > limit' - utils.js merge(): Apply arrayLimit when appending to arrays - parse.js parseArrayValue(): Use '>' instead of '>=' for throwOnLimitExceeded check Tests updated to reflect correct behavior per documentation. Fixes #540. Co-authored-by: Jordan Harband <ljharb@gmail.com> Co-authored-by: Kai Gritun <kai@kaigritun.com>
1 parent 1b9a8b4 commit fbc5206
Copy full SHA for fbc5206

5 files changed

+82-37Lines changed: 82 additions & 37 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎.editorconfig‎

Copy file name to clipboardExpand all lines: .editorconfig
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ end_of_line = lf
77
charset = utf-8
88
trim_trailing_whitespace = true
99
insert_final_newline = true
10-
max_line_length = 160
10+
max_line_length = 180
1111
quote_type = single
1212

1313
[test/*]
Collapse file

‎lib/parse.js‎

Copy file name to clipboardExpand all lines: lib/parse.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ var parseArrayValue = function (val, options, currentArrayLength) {
4040
return val.split(',');
4141
}
4242

43-
if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) {
44-
throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.');
43+
if (options.throwOnLimitExceeded && currentArrayLength > options.arrayLimit) {
44+
throw new RangeError('Array limit exceeded. Only ' + (options.arrayLimit + 1) + ' element' + (options.arrayLimit === 0 ? '' : 's') + ' allowed in an array.');
4545
}
4646

4747
return val;
Collapse file

‎lib/utils.js‎

Copy file name to clipboardExpand all lines: lib/utils.js
+11-3Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ var merge = function merge(target, source, options) {
7474

7575
if (typeof source !== 'object' && typeof source !== 'function') {
7676
if (isArray(target)) {
77-
target[target.length] = source;
77+
var nextIndex = target.length;
78+
if (options && typeof options.arrayLimit === 'number' && nextIndex > options.arrayLimit) {
79+
return markOverflow(arrayToObject(target.concat(source), options), nextIndex);
80+
}
81+
target[nextIndex] = source;
7882
} else if (target && typeof target === 'object') {
7983
if (isOverflow(target)) {
8084
// Add at next numeric index for overflow objects
@@ -107,7 +111,11 @@ var merge = function merge(target, source, options) {
107111
}
108112
return markOverflow(result, getMaxIndex(source) + 1);
109113
}
110-
return [target].concat(source);
114+
var combined = [target].concat(source);
115+
if (options && typeof options.arrayLimit === 'number' && combined.length - 1 > options.arrayLimit) {
116+
return markOverflow(arrayToObject(combined, options), combined.length - 1);
117+
}
118+
return combined;
111119
}
112120

113121
var mergeTarget = target;
@@ -288,7 +296,7 @@ var combine = function combine(a, b, arrayLimit, plainObjects) {
288296
}
289297

290298
var result = [].concat(a, b);
291-
if (result.length > arrayLimit) {
299+
if (result.length - 1 > arrayLimit) {
292300
return markOverflow(arrayToObject(result, { plainObjects: plainObjects }), result.length - 1);
293301
}
294302
return result;
Collapse file

‎test/parse.js‎

Copy file name to clipboardExpand all lines: test/parse.js
+31-9Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,16 +1118,25 @@ test('parse()', function (t) {
11181118
});
11191119

11201120
st.test('throws error when array limit exceeded', function (sst) {
1121+
// 5 elements (indices 0-4), max index 4 > limit 3
11211122
sst['throws'](
11221123
function () {
1123-
qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3, throwOnLimitExceeded: true });
1124+
qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5', { arrayLimit: 3, throwOnLimitExceeded: true });
11241125
},
1125-
new RangeError('Array limit exceeded. Only 3 elements allowed in an array.'),
1126+
new RangeError('Array limit exceeded. Only 4 elements allowed in an array.'),
11261127
'throws error when array limit is exceeded'
11271128
);
11281129
sst.end();
11291130
});
11301131

1132+
st.test('does not throw when at limit', function (sst) {
1133+
// 4 elements (indices 0-3), max index 3 = limit 3, should not throw
1134+
var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3, throwOnLimitExceeded: true });
1135+
sst.ok(Array.isArray(result.a), 'result is an array');
1136+
sst.deepEqual(result.a, ['1', '2', '3', '4'], 'all values present');
1137+
sst.end();
1138+
});
1139+
11311140
st.test('converts array to object if length is greater than limit', function (sst) {
11321141
var result = qs.parse('a[1]=1&a[2]=2&a[3]=3&a[4]=4&a[5]=5&a[6]=6', { arrayLimit: 5 });
11331142

@@ -1304,24 +1313,36 @@ test('DOS', function (t) {
13041313
});
13051314

13061315
test('arrayLimit boundary conditions', function (t) {
1316+
// arrayLimit is about max index, not length. With arrayLimit: 3, indices 0-3 are allowed (4 elements)
13071317
t.test('exactly at the limit stays as array', function (st) {
1308-
var result = qs.parse('a[]=1&a[]=2&a[]=3', { arrayLimit: 3 });
1309-
st.ok(Array.isArray(result.a), 'result is an array when exactly at limit');
1310-
st.deepEqual(result.a, ['1', '2', '3'], 'all values present');
1318+
// 4 elements (indices 0-3), max index 3 = limit 3
1319+
var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3 });
1320+
st.ok(Array.isArray(result.a), 'result is an array when max index equals limit');
1321+
st.deepEqual(result.a, ['1', '2', '3', '4'], 'all values present');
13111322
st.end();
13121323
});
13131324

13141325
t.test('one over the limit converts to object', function (st) {
1315-
var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3 });
1326+
// 5 elements (indices 0-4), max index 4 > limit 3
1327+
var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5', { arrayLimit: 3 });
13161328
st.notOk(Array.isArray(result.a), 'result is not an array when over limit');
1317-
st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3', 3: '4' }, 'all values preserved as object');
1329+
st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3', 3: '4', 4: '5' }, 'all values preserved as object');
13181330
st.end();
13191331
});
13201332

13211333
t.test('arrayLimit 1 with two values', function (st) {
1334+
// 2 elements (indices 0-1), max index 1 = limit 1, should be array
13221335
var result = qs.parse('a[]=1&a[]=2', { arrayLimit: 1 });
1336+
st.ok(Array.isArray(result.a), 'result is an array when max index equals limit');
1337+
st.deepEqual(result.a, ['1', '2'], 'both values preserved as array');
1338+
st.end();
1339+
});
1340+
1341+
t.test('arrayLimit 1 with three values converts to object', function (st) {
1342+
// 3 elements (indices 0-2), max index 2 > limit 1
1343+
var result = qs.parse('a[]=1&a[]=2&a[]=3', { arrayLimit: 1 });
13231344
st.notOk(Array.isArray(result.a), 'result is not an array');
1324-
st.deepEqual(result.a, { 0: '1', 1: '2' }, 'both values preserved');
1345+
st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3' }, 'all values preserved as object');
13251346
st.end();
13261347
});
13271348

@@ -1384,8 +1405,9 @@ test('mixed array and object notation', function (t) {
13841405
});
13851406

13861407
t.test('multiple plain values exceeding limit', function (st) {
1408+
// 3 elements (indices 0-2), max index 2 > limit 1
13871409
st.deepEqual(
1388-
qs.parse('a=b&a=c&a=d', { arrayLimit: 2 }),
1410+
qs.parse('a=b&a=c&a=d', { arrayLimit: 1 }),
13891411
{ a: { 0: 'b', 1: 'c', 2: 'd' } },
13901412
'duplicate plain keys convert to object when exceeding limit'
13911413
);
Collapse file

‎test/utils.js‎

Copy file name to clipboardExpand all lines: test/utils.js
+37-22Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,14 @@ test('merge()', function (t) {
6969
);
7070

7171
t.test('with overflow objects (from arrayLimit)', function (st) {
72+
// arrayLimit is max index, so with limit 0, max index 0 is allowed (1 element)
73+
// To create overflow, need 2+ elements with limit 0, or 3+ with limit 1, etc.
7274
st.test('merges primitive into overflow object at next index', function (s2t) {
73-
// Create an overflow object via combine
74-
var overflow = utils.combine(['a'], 'b', 1, false);
75+
// Create an overflow object via combine: 3 elements (indices 0-2) with limit 0
76+
var overflow = utils.combine(['a', 'b'], 'c', 0, false);
7577
s2t.ok(utils.isOverflow(overflow), 'overflow object is marked');
76-
var merged = utils.merge(overflow, 'c');
77-
s2t.deepEqual(merged, { 0: 'a', 1: 'b', 2: 'c' }, 'adds primitive at next numeric index');
78+
var merged = utils.merge(overflow, 'd');
79+
s2t.deepEqual(merged, { 0: 'a', 1: 'b', 2: 'c', 3: 'd' }, 'adds primitive at next numeric index');
7880
s2t.end();
7981
});
8082

@@ -94,21 +96,21 @@ test('merge()', function (t) {
9496
});
9597

9698
st.test('merges overflow object into primitive', function (s2t) {
97-
// Create an overflow object via combine
98-
var overflow = utils.combine([], 'b', 0, false);
99+
// Create an overflow object via combine: 2 elements (indices 0-1) with limit 0
100+
var overflow = utils.combine(['a'], 'b', 0, false);
99101
s2t.ok(utils.isOverflow(overflow), 'overflow object is marked');
100-
var merged = utils.merge('a', overflow);
102+
var merged = utils.merge('c', overflow);
101103
s2t.ok(utils.isOverflow(merged), 'result is also marked as overflow');
102-
s2t.deepEqual(merged, { 0: 'a', 1: 'b' }, 'creates object with primitive at 0, source values shifted');
104+
s2t.deepEqual(merged, { 0: 'c', 1: 'a', 2: 'b' }, 'creates object with primitive at 0, source values shifted');
103105
s2t.end();
104106
});
105107

106108
st.test('merges overflow object with multiple values into primitive', function (s2t) {
107-
// Create an overflow object via combine
108-
var overflow = utils.combine(['b'], 'c', 1, false);
109+
// Create an overflow object via combine: 3 elements (indices 0-2) with limit 0
110+
var overflow = utils.combine(['b', 'c'], 'd', 0, false);
109111
s2t.ok(utils.isOverflow(overflow), 'overflow object is marked');
110112
var merged = utils.merge('a', overflow);
111-
s2t.deepEqual(merged, { 0: 'a', 1: 'b', 2: 'c' }, 'shifts all source indices by 1');
113+
s2t.deepEqual(merged, { 0: 'a', 1: 'b', 2: 'c', 3: 'd' }, 'shifts all source indices by 1');
112114
s2t.end();
113115
});
114116

@@ -194,30 +196,43 @@ test('combine()', function (t) {
194196
s2t.end();
195197
});
196198

199+
// arrayLimit is max index allowed, so with limit 3, indices 0-3 (4 elements) are allowed
197200
st.test('exactly at the limit stays as array', function (s2t) {
198-
var combined = utils.combine(['a', 'b'], 'c', 3, false);
199-
s2t.deepEqual(combined, ['a', 'b', 'c'], 'stays as array when exactly at limit');
201+
// 4 elements (indices 0-3), max index 3 = limit 3
202+
var combined = utils.combine(['a', 'b', 'c'], 'd', 3, false);
203+
s2t.deepEqual(combined, ['a', 'b', 'c', 'd'], 'stays as array when max index equals limit');
200204
s2t.ok(Array.isArray(combined), 'result is an array');
201205
s2t.end();
202206
});
203207

204208
st.test('over the limit', function (s2t) {
205-
var combined = utils.combine(['a', 'b', 'c'], 'd', 3, false);
206-
s2t.deepEqual(combined, { 0: 'a', 1: 'b', 2: 'c', 3: 'd' }, 'converts to object when over limit');
209+
// 5 elements (indices 0-4), max index 4 > limit 3
210+
var combined = utils.combine(['a', 'b', 'c', 'd'], 'e', 3, false);
211+
s2t.deepEqual(combined, { 0: 'a', 1: 'b', 2: 'c', 3: 'd', 4: 'e' }, 'converts to object when over limit');
207212
s2t.notOk(Array.isArray(combined), 'result is not an array');
208213
s2t.end();
209214
});
210215

211216
st.test('with arrayLimit 0', function (s2t) {
217+
// 1 element (index 0), max index 0 = limit 0, should stay as array
212218
var combined = utils.combine([], 'a', 0, false);
213-
s2t.deepEqual(combined, { 0: 'a' }, 'converts single element to object with arrayLimit 0');
219+
s2t.deepEqual(combined, ['a'], 'stays as array with arrayLimit 0 and single element');
220+
s2t.ok(Array.isArray(combined), 'result is an array');
221+
s2t.end();
222+
});
223+
224+
st.test('with arrayLimit 0 and two elements converts to object', function (s2t) {
225+
// 2 elements (indices 0-1), max index 1 > limit 0
226+
var combined = utils.combine(['a'], 'b', 0, false);
227+
s2t.deepEqual(combined, { 0: 'a', 1: 'b' }, 'converts to object when max index exceeds limit');
214228
s2t.notOk(Array.isArray(combined), 'result is not an array');
215229
s2t.end();
216230
});
217231

218232
st.test('with plainObjects option', function (s2t) {
219-
var combined = utils.combine(['a'], 'b', 1, true);
220-
var expected = { __proto__: null, 0: 'a', 1: 'b' };
233+
// 3 elements (indices 0-2), max index 2 > limit 1
234+
var combined = utils.combine(['a', 'b'], 'c', 1, true);
235+
var expected = { __proto__: null, 0: 'a', 1: 'b', 2: 'c' };
221236
s2t.deepEqual(combined, expected, 'converts to object with null prototype');
222237
s2t.equal(Object.getPrototypeOf(combined), null, 'result has null prototype when plainObjects is true');
223238
s2t.end();
@@ -228,13 +243,13 @@ test('combine()', function (t) {
228243

229244
t.test('with existing overflow object', function (st) {
230245
st.test('adds to existing overflow object at next index', function (s2t) {
231-
// Create overflow object first via combine
232-
var overflow = utils.combine(['a'], 'b', 1, false);
246+
// Create overflow object first via combine: 3 elements (indices 0-2) with limit 0
247+
var overflow = utils.combine(['a', 'b'], 'c', 0, false);
233248
s2t.ok(utils.isOverflow(overflow), 'initial object is marked as overflow');
234249

235-
var combined = utils.combine(overflow, 'c', 10, false);
250+
var combined = utils.combine(overflow, 'd', 10, false);
236251
s2t.equal(combined, overflow, 'returns the same object (mutated)');
237-
s2t.deepEqual(combined, { 0: 'a', 1: 'b', 2: 'c' }, 'adds value at next numeric index');
252+
s2t.deepEqual(combined, { 0: 'a', 1: 'b', 2: 'c', 3: 'd' }, 'adds value at next numeric index');
238253
s2t.end();
239254
});
240255

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.