diff --git a/lib/src/extensions/decode.dart b/lib/src/extensions/decode.dart index c420c7b..35d42f6 100644 --- a/lib/src/extensions/decode.dart +++ b/lib/src/extensions/decode.dart @@ -192,7 +192,7 @@ extension _$Decode on QS { // Duplicate key policy: combine/first/last (default: combine). final bool existing = obj.containsKey(key); if (existing && options.duplicates == Duplicates.combine) { - obj[key] = Utils.combine(obj[key], val); + obj[key] = Utils.combine(obj[key], val, listLimit: options.listLimit); } else if (!existing || options.duplicates == Duplicates.last) { obj[key] = val; } @@ -209,7 +209,8 @@ extension _$Decode on QS { /// - When `parseLists` is false, numeric segments are treated as string keys. /// - When `allowEmptyLists` is true, an empty string (or `null` under /// `strictNullHandling`) under a `[]` segment yields an empty list. - /// - `listLimit` applies to explicit numeric indices as an upper bound. + /// - `listLimit` applies to explicit numeric indices and list growth via `[]`; + /// when exceeded, lists are converted into maps with string indices. /// - A negative `listLimit` disables numeric‑index parsing (bracketed numbers become map keys). /// Empty‑bracket pushes (`[]`) still create lists here; this method does not enforce /// `throwOnLimitExceeded` for that path. Comma‑split growth (if any) has already been @@ -263,10 +264,16 @@ extension _$Decode on QS { // Anonymous list segment `[]` — either an empty list (when allowed) or a // single-element list with the leaf combined in. if (root == '[]' && options.parseLists) { - obj = options.allowEmptyLists && - (leaf == '' || (options.strictNullHandling && leaf == null)) - ? List.empty(growable: true) - : Utils.combine([], leaf); + if (Utils.isOverflow(leaf)) { + // leaf can already be overflow (e.g. duplicates combine/listLimit), + // so preserve it instead of re-wrapping into a list. + obj = leaf; + } else { + obj = options.allowEmptyLists && + (leaf == '' || (options.strictNullHandling && leaf == null)) + ? List.empty(growable: true) + : Utils.combine([], leaf, listLimit: options.listLimit); + } } else { obj = {}; // Normalize bracketed segments ("[k]"). Note: depending on how key decoding is configured, diff --git a/lib/src/models/decode_options.dart b/lib/src/models/decode_options.dart index c7dcbf4..e9bb8b7 100644 --- a/lib/src/models/decode_options.dart +++ b/lib/src/models/decode_options.dart @@ -98,10 +98,13 @@ final class DecodeOptions with EquatableMixin { /// `a[]=` without coercing or discarding them. final bool allowEmptyLists; - /// Maximum list index that will be honored when decoding bracket indices. + /// Maximum list size/index that will be honored when decoding bracket lists. /// /// Keys like `a[9999999]` can cause excessively large sparse lists; above - /// this limit, indices are treated as string map keys instead. + /// this limit, indices are treated as string map keys instead. The same + /// limit also applies to empty-bracket pushes (`a[]`) and duplicate combines: + /// once growth exceeds the limit, the list is converted to a map with string + /// indices to preserve values (matching Node `qs` arrayLimit semantics). /// /// **Negative values:** passing a negative `listLimit` (e.g. `-1`) disables /// numeric‑index parsing entirely — any bracketed number like `a[0]` or diff --git a/lib/src/qs.dart b/lib/src/qs.dart index 9bbf35b..4f79f4e 100644 --- a/lib/src/qs.dart +++ b/lib/src/qs.dart @@ -37,9 +37,6 @@ final class QS { /// * a query [String] (e.g. `"a=1&b[c]=2"`), or /// * a pre-tokenized `Map` produced by a custom tokenizer. /// - When `input` is `null` or the empty string, `{}` is returned. - /// - If [DecodeOptions.parseLists] is `true` and the number of top‑level - /// parameters exceeds [DecodeOptions.listLimit], list parsing is - /// temporarily disabled for this call to bound memory (mirrors Node `qs`). /// - Throws [ArgumentError] if `input` is neither a `String` nor a /// `Map`. /// @@ -67,16 +64,6 @@ final class QS { ? _$Decode._parseQueryStringValues(input, options) : input; - // Guardrail: if the top-level parameter count is large, temporarily disable - // list parsing to keep memory bounded (matches Node `qs`). Only apply for - // raw string inputs, not for pre-tokenized maps. - if (input is String && - options.parseLists && - options.listLimit > 0 && - (tempObj?.length ?? 0) > options.listLimit) { - options = options.copyWith(parseLists: false); - } - Map obj = {}; // Merge each parsed key into the accumulator using the same rules as Node `qs`. diff --git a/lib/src/utils.dart b/lib/src/utils.dart index ae98b7c..8917815 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -28,6 +28,42 @@ part 'constants/hex_table.dart'; final class Utils { static const int _segmentLimit = 1024; + /// Tracks array-overflow objects (from listLimit) without mutating user data. + static final Expando _overflowIndex = Expando('qsOverflowIndex'); + + /// Marks a map as an overflow container with the given max index. + @internal + @visibleForTesting + static Map markOverflow( + Map obj, + int maxIndex, + ) { + _overflowIndex[obj] = maxIndex; + return obj; + } + + /// Returns `true` if the given object is marked as an overflow container. + @internal + static bool isOverflow(dynamic obj) => + obj is Map && _overflowIndex[obj] != null; + + /// Returns the tracked max numeric index for an overflow map, or -1 if absent. + static int _getOverflowIndex(Map obj) => _overflowIndex[obj] ?? -1; + + /// Updates the tracked max numeric index for an overflow map. + static void _setOverflowIndex(Map obj, int maxIndex) { + _overflowIndex[obj] = maxIndex; + } + + /// Returns the larger of the current max and the parsed numeric key (if any). + static int _updateOverflowMax(int current, String key) { + final int? parsed = int.tryParse(key); + if (parsed == null || parsed < 0) { + return current; + } + return parsed > current ? parsed : current; + } + /// Deeply merges `source` into `target` while preserving insertion order /// and list semantics used by `qs`. /// @@ -127,12 +163,17 @@ final class Utils { } } else if (target is Map) { if (source is Iterable) { - target = { - for (final MapEntry entry in target.entries) - entry.key.toString(): entry.value, + final Map sourceMap = { for (final (int i, dynamic item) in source.indexed) if (item is! Undefined) i.toString(): item }; + return merge(target, sourceMap, options); + } + if (isOverflow(target)) { + final int newIndex = _getOverflowIndex(target) + 1; + target[newIndex.toString()] = source; + _setOverflowIndex(target, newIndex); + return target; } } else if (source != null) { if (target is! Iterable && source is Iterable) { @@ -146,11 +187,40 @@ final class Utils { if (target == null || target is! Map) { if (target is Iterable) { - return Map.of({ + final Map mergeTarget = { for (final (int i, dynamic item) in target.indexed) if (item is! Undefined) i.toString(): item, - ...source, - }); + }; + for (final MapEntry entry in source.entries) { + final String key = entry.key.toString(); + if (mergeTarget.containsKey(key)) { + mergeTarget[key] = merge( + mergeTarget[key], + entry.value, + options, + ); + } else { + mergeTarget[key] = entry.value; + } + } + return mergeTarget; + } + + if (isOverflow(source)) { + final int sourceMax = _getOverflowIndex(source); + final Map result = { + if (target != null) '0': target, + }; + for (final MapEntry entry in source.entries) { + final String key = entry.key.toString(); + final int? oldIndex = int.tryParse(key); + if (oldIndex == null) { + result[key] = entry.value; + } else { + result[(oldIndex + 1).toString()] = entry.value; + } + } + return markOverflow(result, sourceMax + 1); } return [ @@ -165,6 +235,9 @@ final class Utils { ]; } + final bool targetOverflow = isOverflow(target); + int? overflowMax = targetOverflow ? _getOverflowIndex(target) : null; + Map mergeTarget = target is Iterable && source is! Iterable ? { for (final (int i, dynamic item) in (target as Iterable).indexed) @@ -176,6 +249,9 @@ final class Utils { }; for (final MapEntry entry in source.entries) { + if (overflowMax != null) { + overflowMax = _updateOverflowMax(overflowMax, entry.key.toString()); + } mergeTarget.update( entry.key.toString(), (value) => merge( @@ -186,6 +262,9 @@ final class Utils { ifAbsent: () => entry.value, ); } + if (overflowMax != null) { + markOverflow(mergeTarget, overflowMax); + } return mergeTarget; } @@ -577,17 +656,47 @@ final class Utils { return root; } - /// Concatenates two values as a typed `List`, spreading iterables. + /// Concatenates two values, spreading iterables. + /// + /// When [listLimit] is provided and exceeded, returns a map with string keys. + /// Any throwing behavior is enforced earlier during parsing, matching Node `qs`. + /// + /// **Note:** If [a] is already an overflow object, this method mutates [a] + /// in place by appending entries from [b]. /// /// Examples: /// ```dart - /// combine<int>([1,2], 3); // [1,2,3] - /// combine<String>('a', ['b','c']); // ['a','b','c'] + /// combine([1,2], 3); // [1,2,3] + /// combine('a', ['b','c']); // ['a','b','c'] /// ``` - static List combine(dynamic a, dynamic b) => [ - if (a is Iterable) ...a else a, - if (b is Iterable) ...b else b, - ]; + static dynamic combine(dynamic a, dynamic b, {int? listLimit}) { + if (isOverflow(a)) { + int newIndex = _getOverflowIndex(a); + if (b is Iterable) { + for (final item in b) { + newIndex++; + a[newIndex.toString()] = item; + } + } else { + newIndex++; + a[newIndex.toString()] = b; + } + _setOverflowIndex(a, newIndex); + return a; + } + + final List result = [ + if (a is Iterable) ...a else a, + if (b is Iterable) ...b else b, + ]; + + if (listLimit != null && listLimit >= 0 && result.length > listLimit) { + final Map overflow = createIndexMap(result); + return markOverflow(overflow, result.length - 1); + } + + return result; + } /// Applies `fn` to a scalar or maps it over an iterable, returning the result. /// diff --git a/test/comparison/package.json b/test/comparison/package.json index 3da9b28..43215c2 100644 --- a/test/comparison/package.json +++ b/test/comparison/package.json @@ -5,6 +5,6 @@ "author": "Klemen Tusar", "license": "BSD-3-Clause", "dependencies": { - "qs": "^6.12.1" + "qs": "^6.14.1" } } diff --git a/test/unit/decode_test.dart b/test/unit/decode_test.dart index 454eae0..bf4e860 100644 --- a/test/unit/decode_test.dart +++ b/test/unit/decode_test.dart @@ -517,7 +517,7 @@ void main() { expect( QS.decode('a[]=b&a=c', const DecodeOptions(listLimit: 0)), equals({ - 'a': ['b', 'c'] + 'a': {'0': 'b', '1': 'c'} }), ); expect( @@ -536,7 +536,7 @@ void main() { expect( QS.decode('a=b&a[]=c', const DecodeOptions(listLimit: 0)), equals({ - 'a': ['b', 'c'] + 'a': {'0': 'b', '1': 'c'} }), ); expect( @@ -916,7 +916,7 @@ void main() { const DecodeOptions(strictNullHandling: true, listLimit: 0), ), equals({ - 'a': ['b', null, 'c', ''] + 'a': {'0': 'b', '1': null, '2': 'c', '3': ''} }), ); @@ -936,7 +936,7 @@ void main() { const DecodeOptions(strictNullHandling: true, listLimit: 0), ), equals({ - 'a': ['b', '', 'c', null] + 'a': {'0': 'b', '1': '', '2': 'c', '3': null} }), ); @@ -1998,7 +1998,7 @@ void main() { const DecodeOptions(listLimit: 0), ), equals({ - 'a': ['1', '2'] + 'a': {'0': '1', '1': '2'} }), ); }); @@ -2024,6 +2024,126 @@ void main() { }); }); + group('array limit parity', () { + test('prevents list DOS with [] notation', () { + final List values = List.filled(105, 'x'); + final String attack = 'a[]=${values.join('&a[]=')}'; + final result = QS.decode(attack, const DecodeOptions(listLimit: 100)); + final aValue = result['a']; + expect(aValue, isA>()); + expect((aValue as Map).length, 105); + expect(Utils.isOverflow(aValue), isTrue); + }); + + test('listLimit boundary conditions', () { + final resultAtLimit = + QS.decode('a[]=1&a[]=2&a[]=3', const DecodeOptions(listLimit: 3)); + expect(resultAtLimit['a'], isA()); + expect(resultAtLimit['a'], ['1', '2', '3']); + + final resultOverLimit = QS.decode( + 'a[]=1&a[]=2&a[]=3&a[]=4', + const DecodeOptions(listLimit: 3), + ); + expect(resultOverLimit['a'], isA>()); + expect(resultOverLimit['a'], { + '0': '1', + '1': '2', + '2': '3', + '3': '4', + }); + expect(Utils.isOverflow(resultOverLimit['a']), isTrue); + + final resultLimitOne = + QS.decode('a[]=1&a[]=2', const DecodeOptions(listLimit: 1)); + expect(resultLimitOne['a'], {'0': '1', '1': '2'}); + expect(Utils.isOverflow(resultLimitOne['a']), isTrue); + }); + + test('mixed array and object notation', () { + expect( + QS.decode('a[]=b&a[c]=d'), + equals({ + 'a': {'0': 'b', 'c': 'd'} + }), + ); + + expect( + QS.decode('a[0]=b&a[c]=d'), + equals({ + 'a': {'0': 'b', 'c': 'd'} + }), + ); + + expect( + QS.decode('a=b&a[]=c', const DecodeOptions(listLimit: 20)), + equals({ + 'a': ['b', 'c'] + }), + ); + + expect( + QS.decode('a[]=b&a=c', const DecodeOptions(listLimit: 20)), + equals({ + 'a': ['b', 'c'] + }), + ); + + expect( + QS.decode('a=b&a[0]=c', const DecodeOptions(listLimit: 20)), + equals({ + 'a': ['b', 'c'] + }), + ); + + expect( + QS.decode('a=b&a=c&a=d', const DecodeOptions(listLimit: 20)), + equals({ + 'a': ['b', 'c', 'd'] + }), + ); + + expect( + QS.decode('a=b&a=c&a=d', const DecodeOptions(listLimit: 2)), + equals({ + 'a': {'0': 'b', '1': 'c', '2': 'd'} + }), + ); + }); + + test('mixed [] and [0] under tight listLimit', () { + // Note: overflow tagging is order-dependent in Node qs; we mirror that. + final resultZero = + QS.decode('a[]=b&a[0]=c', const DecodeOptions(listLimit: 0)); + expect(resultZero['a'], isA>()); + expect(resultZero['a'], { + '0': ['b', 'c'] + }); + expect(Utils.isOverflow(resultZero['a']), isTrue); + + final resultOne = + QS.decode('a[]=b&a[0]=c', const DecodeOptions(listLimit: 1)); + expect(resultOne['a'], ['b', 'c']); + expect(Utils.isOverflow(resultOne['a']), isFalse); + }); + + test('mixed [0] and [] under tight listLimit', () { + // Same structure as above but overflow is not tagged when order flips. + final resultZero = + QS.decode('a[0]=b&a[]=c', const DecodeOptions(listLimit: 0)); + expect(resultZero['a'], isA>()); + expect(resultZero['a'], { + '0': ['b', 'c'] + }); + expect(Utils.isOverflow(resultZero['a']), isFalse); + + final resultOne = + QS.decode('a[0]=b&a[]=c', const DecodeOptions(listLimit: 1)); + expect(resultOne['a'], ['b', 'c']); + expect(Utils.isOverflow(resultOne['a']), isFalse); + }); + }); + group('key-aware decoder + options isolation', () { test('custom decoder receives kind for keys and values', () { final kinds = []; diff --git a/test/unit/uri_extension_test.dart b/test/unit/uri_extension_test.dart index 5a994ea..5289a71 100644 --- a/test/unit/uri_extension_test.dart +++ b/test/unit/uri_extension_test.dart @@ -447,7 +447,7 @@ void main() { Uri.parse('$testUrl?a[]=b&a=c') .queryParametersQs(const DecodeOptions(listLimit: 0)), equals({ - 'a': ['b', 'c'] + 'a': {'0': 'b', '1': 'c'} }), ); expect( @@ -468,7 +468,7 @@ void main() { Uri.parse('$testUrl?a=b&a[]=c') .queryParametersQs(const DecodeOptions(listLimit: 0)), equals({ - 'a': ['b', 'c'] + 'a': {'0': 'b', '1': 'c'} }), ); expect( @@ -830,7 +830,7 @@ void main() { const DecodeOptions(strictNullHandling: true, listLimit: 0), ), equals({ - 'a': ['b', null, 'c', ''] + 'a': {'0': 'b', '1': null, '2': 'c', '3': ''} }), ); @@ -848,7 +848,7 @@ void main() { const DecodeOptions(strictNullHandling: true, listLimit: 0), ), equals({ - 'a': ['b', '', 'c', null] + 'a': {'0': 'b', '1': '', '2': 'c', '3': null} }), ); diff --git a/test/unit/utils_test.dart b/test/unit/utils_test.dart index 71a3e00..6af029d 100644 --- a/test/unit/utils_test.dart +++ b/test/unit/utils_test.dart @@ -931,13 +931,64 @@ void main() { isA(), ); }); + + group('with overflow objects (from listLimit)', () { + test('merges primitive into overflow object at next index', () { + final overflow = + Utils.combine(['a'], 'b', listLimit: 1) as Map; + expect(Utils.isOverflow(overflow), isTrue); + + final merged = Utils.merge(overflow, 'c') as Map; + expect(merged, {'0': 'a', '1': 'b', '2': 'c'}); + }); + + test('merges overflow object into primitive', () { + final overflow = + Utils.combine([], 'b', listLimit: 0) as Map; + expect(Utils.isOverflow(overflow), isTrue); + + final merged = Utils.merge('a', overflow) as Map; + expect(Utils.isOverflow(merged), isTrue); + expect(merged, {'0': 'a', '1': 'b'}); + }); + + test('merges overflow object with multiple values into primitive', () { + final overflow = + Utils.combine(['b'], 'c', listLimit: 1) as Map; + final merged = Utils.merge('a', overflow) as Map; + expect(merged, {'0': 'a', '1': 'b', '2': 'c'}); + }); + + test('merges overflow object with list values by key', () { + final overflow = + Utils.combine(['a'], 'b', listLimit: 1) as Map; + final merged = + Utils.merge(overflow, ['x', 'y']) as Map; + expect(merged, { + '0': ['a', 'x'], + '1': ['b', 'y'], + }); + expect(Utils.isOverflow(merged), isTrue); + }); + + test('merges list with overflow object into index map', () { + final overflow = + Utils.combine(['a'], 'b', listLimit: 1) as Map; + final merged = Utils.merge(['x'], overflow) as Map; + expect(merged, { + '0': ['x', 'a'], + '1': 'b', + }); + expect(Utils.isOverflow(merged), isFalse); + }); + }); }); group('combine', () { test('both lists', () { const List a = [1]; const List b = [2]; - final List combined = Utils.combine(a, b); + final combined = Utils.combine(a, b); expect(a, equals([1])); expect(b, equals([2])); @@ -952,7 +1003,7 @@ void main() { const int bN = 2; const List b = [bN]; - final List combinedAnB = Utils.combine(aN, b); + final combinedAnB = Utils.combine(aN, b); expect(b, equals([bN])); expect(aN, isNot(same(combinedAnB))); expect(a, isNot(same(combinedAnB))); @@ -960,7 +1011,7 @@ void main() { expect(b, isNot(same(combinedAnB))); expect(combinedAnB, equals([1, 2])); - final List combinedABn = Utils.combine(a, bN); + final combinedABn = Utils.combine(a, bN); expect(a, equals([aN])); expect(aN, isNot(same(combinedABn))); expect(a, isNot(same(combinedABn))); @@ -972,12 +1023,72 @@ void main() { test('neither is a list', () { const int a = 1; const int b = 2; - final List combined = Utils.combine(a, b); + final combined = Utils.combine(a, b); expect(a, isNot(same(combined))); expect(b, isNot(same(combined))); expect(combined, equals([1, 2])); }); + + group('with listLimit', () { + test('under the limit returns a list', () { + final combined = Utils.combine(['a', 'b'], 'c', listLimit: 10); + expect(combined, ['a', 'b', 'c']); + expect(combined, isA()); + }); + + test('exactly at the limit stays a list', () { + final combined = Utils.combine(['a', 'b'], 'c', listLimit: 3); + expect(combined, ['a', 'b', 'c']); + expect(combined, isA()); + }); + + test('over the limit converts to a map', () { + final combined = Utils.combine(['a', 'b', 'c'], 'd', listLimit: 3); + expect(combined, {'0': 'a', '1': 'b', '2': 'c', '3': 'd'}); + expect(combined, isA>()); + }); + + test('listLimit 0 converts to a map', () { + final combined = Utils.combine([], 'a', listLimit: 0); + expect(combined, {'0': 'a'}); + expect(combined, isA>()); + }); + }); + + group('with existing overflow object', () { + test('adds to existing overflow object at next index', () { + final overflow = + Utils.combine(['a'], 'b', listLimit: 1) as Map; + expect(Utils.isOverflow(overflow), isTrue); + + final combined = Utils.combine(overflow, 'c', listLimit: 10) + as Map; + expect(combined, same(overflow)); + expect(combined, {'0': 'a', '1': 'b', '2': 'c'}); + }); + + test('spreads iterable into existing overflow object', () { + final overflow = + Utils.combine(['a'], 'b', listLimit: 1) as Map; + final combined = Utils.combine(overflow, ['c', 'd'], listLimit: 10) + as Map; + + expect(combined, same(overflow)); + expect(combined, {'0': 'a', '1': 'b', '2': 'c', '3': 'd'}); + }); + + test('does not treat plain object with numeric keys as overflow', () { + final plainObj = {'0': 'a', '1': 'b'}; + expect(Utils.isOverflow(plainObj), isFalse); + + final combined = Utils.combine(plainObj, 'c', listLimit: 10); + expect(combined, [ + {'0': 'a', '1': 'b'}, + 'c' + ]); + }); + }); }); test('decode', () {