diff --git a/CHANGELOG.md b/CHANGELOG.md index e93d0dc..21ec3e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.8.1-dev + +* [FIX] match Node `qs` 6.15.3 cumulative list-limit enforcement across duplicate-key combinations and mixed list merges +* [FIX] reject oversized flat comma values before allocating their split lists when `throwOnLimitExceeded` is enabled +* [CHORE] add Node `qs` 6.15.3 regression coverage for unbalanced bracket keys, chunk-boundary surrogate pairs, and cyclic compaction + ## 1.8.0 * [BREAKING] stop exporting the internal `Undefined` sentinel from the public API diff --git a/lib/src/extensions/decode.dart b/lib/src/extensions/decode.dart index 956154c..f0e0ae2 100644 --- a/lib/src/extensions/decode.dart +++ b/lib/src/extensions/decode.dart @@ -34,6 +34,8 @@ extension _$Decode on QS { /// /// The `currentListLength` is used to guard incremental growth when we are /// already building a list for a given key path. + /// `isFlatListValue` distinguishes flat comma lists (`a=1,2`, `a[b]=1,2`) + /// from nested `[]=` comma groups, which count as one outer list element. /// /// **Negative `listLimit` semantics:** a negative value disables numeric-index parsing /// elsewhere (e.g. `[2]` segments become string keys). For commaโ€‘splits specifically: @@ -44,9 +46,21 @@ extension _$Decode on QS { final DecodeOptions options, final int currentListLength, final bool isListGrowthPath, + final bool isFlatListValue, ) { // Fast-path: split comma-separated scalars into a list when requested. if (val is String && val.isNotEmpty && options.comma && val.contains(',')) { + if (isFlatListValue && options.throwOnLimitExceeded) { + int commaCount = 0; + int commaIndex = val.indexOf(','); + while (commaIndex >= 0) { + commaCount++; + if (commaCount >= options.listLimit) { + Utils.throwListLimitExceeded(options.listLimit); + } + commaIndex = val.indexOf(',', commaIndex + 1); + } + } return _splitCommaValue(val); } @@ -54,19 +68,12 @@ extension _$Decode on QS { if (options.throwOnLimitExceeded && isListGrowthPath && currentListLength >= options.listLimit) { - throw RangeError(_listLimitExceededMessage(options.listLimit)); + Utils.throwListLimitExceeded(options.listLimit); } return val; } - /// Helper to generate consistent error messages for list limit violations, - /// based on the configured `listLimit`. - static String _listLimitExceededMessage(final int listLimit) => listLimit < 0 - ? 'List parsing is disabled (listLimit < 0).' - : 'List limit exceeded. Only $listLimit ' - 'element${listLimit == 1 ? '' : 's'} allowed in a list.'; - /// Applies comma-list limit handling after values have been decoded and /// after `[]` suffix wrapping, matching Node `qs` ordering. static dynamic _promoteCommaListIfNeeded( @@ -77,9 +84,14 @@ extension _$Decode on QS { return val; } if (options.throwOnLimitExceeded) { - throw RangeError(_listLimitExceededMessage(options.listLimit)); + Utils.throwListLimitExceeded(options.listLimit); } - return Utils.combine([], val, listLimit: options.listLimit); + return Utils.combine( + [], + val, + listLimit: options.listLimit, + throwOnLimitExceeded: options.throwOnLimitExceeded, + ); } /// Splits a comma-separated value into parts, respecting an optional `maxParts` limit. @@ -295,6 +307,7 @@ extension _$Decode on QS { options, currentListLength, listGrowthFromKey, + !bracketSuffix, ), (final dynamic v) => options.decodeValue(v as String?, charset: charset), @@ -328,13 +341,14 @@ extension _$Decode on QS { final bool existing = obj.containsKey(key); switch ((existing, options.duplicates)) { case (true, Duplicates.combine): - // Existing key + `combine` policy: merge old/new values. - obj[key] = Utils.combine(obj[key], val, listLimit: options.listLimit); - break; case (true, _) when bracketSuffix: - // `qs` always combines duplicate bracket-notation keys, even when - // the duplicates option is `first` or `last`. - obj[key] = Utils.combine(obj[key], val, listLimit: options.listLimit); + // Combine by policy, and always combine bracket-notation duplicates. + obj[key] = Utils.combine( + obj[key], + val, + listLimit: options.listLimit, + throwOnLimitExceeded: options.throwOnLimitExceeded, + ); break; case (false, _): case (true, Duplicates.last): @@ -411,6 +425,7 @@ extension _$Decode on QS { options, currentListLength, isListGrowthPath, + false, ); for (int i = chain.length - 1; i >= 0; --i) { @@ -428,7 +443,12 @@ extension _$Decode on QS { obj = options.allowEmptyLists && (leaf == '' || (options.strictNullHandling && leaf == null)) ? List.empty(growable: true) - : Utils.combine([], leaf, listLimit: options.listLimit); + : Utils.combine( + [], + leaf, + listLimit: options.listLimit, + throwOnLimitExceeded: options.throwOnLimitExceeded, + ); } } else { obj = {}; @@ -465,7 +485,7 @@ extension _$Decode on QS { ); obj[index] = leaf; } else if (isValidListIndex && options.throwOnLimitExceeded) { - throw RangeError(_listLimitExceededMessage(options.listLimit)); + Utils.throwListLimitExceeded(options.listLimit); } else if (isValidListIndex) { obj[index.toString()] = leaf; Utils.markOverflow(obj, index); diff --git a/lib/src/utils.dart b/lib/src/utils.dart index ade7d72..1a24330 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -52,9 +52,8 @@ final class Utils { static int _getOverflowIndex(final Map obj) => _overflowIndex[obj] ?? -1; /// Updates the tracked max numeric index for an overflow map. - static void _setOverflowIndex(final Map obj, final int maxIndex) { - _overflowIndex[obj] = maxIndex; - } + static void _setOverflowIndex(final Map obj, final int maxIndex) => + _overflowIndex[obj] = maxIndex; /// Returns the larger of the current max and the parsed numeric key (if any). static int _updateOverflowMax(final int current, final String key) { @@ -65,6 +64,27 @@ final class Utils { return parsed > current ? parsed : current; } + @internal + static Never throwListLimitExceeded(final int listLimit) => throw RangeError( + listLimit < 0 + ? 'List parsing is disabled (listLimit < 0).' + : 'List limit exceeded. Only $listLimit ' + 'element${listLimit == 1 ? '' : 's'} allowed in a list.', + ); + + static dynamic _enforceListLimit( + final List result, + final DecodeOptions? options, + ) { + if (options == null || result.length <= options.listLimit) { + return result; + } + if (options.throwOnLimitExceeded) { + throwListLimitExceeded(options.listLimit); + } + return markOverflow(createIndexMap(result), result.length - 1); + } + /// Deeply merges `source` into `target` while preserving insertion order /// and list semantics used by `qs`. /// @@ -182,11 +202,12 @@ final class Utils { } stack.removeLast(); - frame.onResult( - currentTarget is Set - ? indexedTarget.values.toSet() - : indexedTarget.values.toList(), - ); + frame.onResult(currentTarget is Set + ? indexedTarget.values.toSet() + : _enforceListLimit( + indexedTarget.values.toList(), + frame.options, + )); continue; } @@ -203,13 +224,14 @@ final class Utils { } stack.removeLast(); - frame.onResult( - currentTarget is Set - ? (Set.of(currentTarget) - ..addAll(currentSource.whereNotType())) - : (List.of(currentTarget) - ..addAll(currentSource.whereNotType())), - ); + frame.onResult(currentTarget is Set + ? (Set.of(currentTarget) + ..addAll(currentSource.whereNotType())) + : _enforceListLimit( + List.of(currentTarget) + ..addAll(currentSource.whereNotType()), + frame.options, + )); continue; } @@ -217,7 +239,7 @@ final class Utils { final List merged = List.of(currentTarget) ..add(currentSource); stack.removeLast(); - frame.onResult(merged); + frame.onResult(_enforceListLimit(merged, frame.options)); continue; } if (currentTarget is Set) { @@ -269,10 +291,12 @@ final class Utils { continue; } else { if (currentTarget is! Iterable && currentSource is Iterable) { + final List merged = [ + currentTarget, + ...currentSource.whereNotType(), + ]; stack.removeLast(); - frame.onResult( - [currentTarget, ...currentSource.whereNotType()], - ); + frame.onResult(_enforceListLimit(merged, frame.options)); continue; } stack.removeLast(); @@ -316,18 +340,17 @@ final class Utils { } stack.removeLast(); - frame.onResult( - [ - if (currentTarget is Iterable) - ...currentTarget.whereNotType() - else if (currentTarget != null) - currentTarget, - if (currentSource is Iterable) - ...(currentSource as Iterable).whereNotType() - else - currentSource, - ], - ); + final List combined = [ + if (currentTarget is Iterable) + ...currentTarget.whereNotType() + else if (currentTarget != null) + currentTarget, + if (currentSource is Iterable) + ...(currentSource as Iterable).whereNotType() + else + currentSource, + ]; + frame.onResult(_enforceListLimit(combined, frame.options)); continue; } @@ -413,7 +436,10 @@ final class Utils { } final resultList = frame.targetIsSet ? frame.indexedTarget!.values.toSet() - : frame.indexedTarget!.values.toList(); + : _enforceListLimit( + frame.indexedTarget!.values.toList(), + frame.options, + ); stack.removeLast(); frame.onResult(resultList); continue; @@ -868,10 +894,12 @@ final class Utils { /// 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`. + /// When [throwOnLimitExceeded] is true, throws instead of returning an + /// overflow map. /// /// **Note:** If [a] is already an overflow object, this method mutates [a] - /// in place by appending entries from [b]. + /// in place by appending entries from [b], unless strict limit enforcement + /// throws first. /// /// Examples: /// ```dart @@ -882,8 +910,12 @@ final class Utils { final dynamic a, final dynamic b, { final int? listLimit, + final bool throwOnLimitExceeded = false, }) { if (isOverflow(a)) { + if (throwOnLimitExceeded && listLimit != null) { + throwListLimitExceeded(listLimit); + } int newIndex = _getOverflowIndex(a); if (b is Iterable) { for (final dynamic item in b) { @@ -904,6 +936,9 @@ final class Utils { ]; if (listLimit != null && result.length > listLimit) { + if (throwOnLimitExceeded) { + throwListLimitExceeded(listLimit); + } final Map overflow = createIndexMap(result); return markOverflow(overflow, result.length - 1); } @@ -938,7 +973,7 @@ final class Utils { }; } - static bool _isProtectedObjectKey(final String key) => const { + static bool _isProtectedObjectKey(final String key) => const { '__defineGetter__', '__defineSetter__', '__lookupGetter__', diff --git a/test/unit/decode_test.dart b/test/unit/decode_test.dart index 0876292..a9c4b20 100644 --- a/test/unit/decode_test.dart +++ b/test/unit/decode_test.dart @@ -602,6 +602,134 @@ void main() { ); }); + group('unbalanced bracket keys', () { + final cases = <(String, DecodeOptions, Map)>[ + ( + 'a[bc=v', + const DecodeOptions(), + { + 'a': {'[bc': 'v'} + } + ), + ( + 'a[=v', + const DecodeOptions(), + { + 'a': {'[': 'v'} + } + ), + ( + 'a[b][c=v', + const DecodeOptions(), + { + 'a': { + 'b': {'[c': 'v'} + } + }, + ), + ( + 'a[b]c[d=v', + const DecodeOptions(), + { + 'a': { + 'b': {'[d': 'v'} + } + }, + ), + ( + 'filters[customtags:Env: Prod=v', + const DecodeOptions(), + { + 'filters': {'[customtags:Env: Prod': 'v'} + }, + ), + ( + '][a=v', + const DecodeOptions(), + { + ']': {'[a': 'v'} + } + ), + ( + 'a][b=v', + const DecodeOptions(), + { + 'a]': {'[b': 'v'} + } + ), + ( + 'a[b[c=v', + const DecodeOptions(), + { + 'a': {'[b[c': 'v'} + } + ), + ( + 'a[b[c]=v', + const DecodeOptions(), + { + 'a': {'[b[c]': 'v'} + } + ), + ( + 'a[b][c[d=v', + const DecodeOptions(), + { + 'a': { + 'b': {'[c[d': 'v'} + } + }, + ), + ('[abc=v', const DecodeOptions(), {'[abc': 'v'}), + ('[[]b=v', const DecodeOptions(), {'[[]b': 'v'}), + ( + 'a[b]c[d]e[f=v', + const DecodeOptions(depth: 5), + { + 'a': { + 'b': { + 'd': {'[f': 'v'} + } + } + }, + ), + ( + 'a[b]c[d]e[f=v', + const DecodeOptions(depth: 1), + { + 'a': { + 'b': {'[d]e[f': 'v'} + } + }, + ), + ('a[bc=v', const DecodeOptions(depth: 0), {'a[bc': 'v'}), + ( + 'a.b[c=v', + const DecodeOptions(allowDots: true), + { + 'a': { + 'b': {'[c': 'v'} + } + }, + ), + ('a]b=v', const DecodeOptions(), {'a]b': 'v'}), + ( + 'a[b]extra=v', + const DecodeOptions(), + { + 'a': {'b': 'v'} + }, + ), + ]; + + for (final (input, options, expected) in cases) { + test('$input at depth ${options.depth}', () { + expect(() => QS.decode(input, options), returnsNormally); + expect(QS.decode(input, options), expected); + }); + } + }); + test('parses a simple list', () { expect( QS.decode('a=b&a=c'), @@ -2345,15 +2473,13 @@ void main() { ); }); - test('duplicate scalar overflow promotes to a map in strict mode', () { + test('duplicate scalar overflow throws in strict mode', () { expect( - QS.decode( + () => QS.decode( 'a=1&a=2', const DecodeOptions(listLimit: 1, throwOnLimitExceeded: true), ), - equals({ - 'a': {'0': '1', '1': '2'} - }), + throwsRangeError, ); }); @@ -2461,6 +2587,182 @@ void main() { }); }); + group('qs 6.15.3 list limit parity', () { + const strictOne = DecodeOptions( + listLimit: 1, + throwOnLimitExceeded: true, + ); + + test('throws when cumulative comma groups exceed listLimit', () { + expect( + () => QS.decode( + 'a=1,2,3&a=4,5,6', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + throwsRangeError, + ); + expect( + () => QS.decode( + 'a=v,v,v,v,v&a=v,v,v,v,v&a=v,v,v,v,v', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + throwsRangeError, + ); + }); + + test('throws when duplicate keys grow past the boundary', () { + expect(() => QS.decode('a=x&a=y', strictOne), throwsRangeError); + expect(() => QS.decode('a[]=x&a[]=y', strictOne), throwsRangeError); + }); + + test('throws when mixed notation grows past the boundary', () { + expect(() => QS.decode('a=x&a[0]=y', strictOne), throwsRangeError); + expect(() => QS.decode('a[0]=x&a=y', strictOne), throwsRangeError); + expect(() => QS.decode('a[0]=x&a[]=y', strictOne), throwsRangeError); + }); + + test('mixed notation becomes an overflow map in non-strict mode', () { + const options = DecodeOptions(listLimit: 1); + const expected = { + 'a': {'0': 'x', '1': 'y'} + }; + + expect(QS.decode('a=x&a[0]=y', options), expected); + expect(QS.decode('a[0]=x&a=y', options), expected); + expect(QS.decode('a[0]=x&a[]=y', options), expected); + expect(Utils.isOverflow(QS.decode('a[0]=x&a=y', options)['a']), isTrue); + }); + + test('cumulative comma groups become an overflow map in non-strict mode', + () { + final result = QS.decode( + 'a=1,2,3&a=4,5,6', + const DecodeOptions(comma: true, listLimit: 5), + ); + + expect(result, { + 'a': {'0': '1', '1': '2', '2': '3', '3': '4', '4': '5', '5': '6'} + }); + expect(Utils.isOverflow(result['a']), isTrue); + }); + + test('throws before decoding an oversized flat comma value', () { + int decodedValues = 0; + final options = DecodeOptions( + comma: true, + listLimit: 1, + throwOnLimitExceeded: true, + decoder: ( + String? value, { + Encoding? charset, + DecodeKind? kind, + }) { + if (kind == DecodeKind.value) decodedValues++; + return Utils.decode(value, charset: charset); + }, + ); + + expect(() => QS.decode('a=1,2', options), throwsRangeError); + expect(decodedValues, 0); + }); + + test('throws for oversized nested flat comma values', () { + expect( + () => QS.decode( + 'a[b]=1,2,3,4,5,6', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + throwsRangeError, + ); + }); + + test('keeps comma values at or below listLimit', () { + expect( + QS.decode( + 'a=1,2,3&a=4', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + { + 'a': ['1', '2', '3', '4'] + }, + ); + expect( + QS.decode( + 'a=1,2,3,4,5', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + { + 'a': ['1', '2', '3', '4', '5'] + }, + ); + }); + + test('counts bracketed comma groups as nested elements', () { + expect( + QS.decode( + 'a[]=1,2,3&a[]=4,5,6', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + { + 'a': [ + ['1', '2', '3'], + ['4', '5', '6'], + ] + }, + ); + expect( + QS.decode( + 'a[]=1,2,3,4,5,6', + const DecodeOptions( + comma: true, + listLimit: 5, + throwOnLimitExceeded: true, + ), + ), + { + 'a': [ + ['1', '2', '3', '4', '5', '6'] + ] + }, + ); + expect( + () => QS.decode( + 'a[]=1,2,3', + const DecodeOptions( + comma: true, + listLimit: 0, + throwOnLimitExceeded: true, + ), + ), + throwsRangeError, + ); + }); + }); + group('array limit parity', () { test('prevents list DOS with [] notation', () { final List values = List.filled(105, 'x'); @@ -2667,8 +2969,8 @@ void main() { final resultOne = QS.decode('a[]=b&a[0]=c', const DecodeOptions(listLimit: 1)); - expect(resultOne['a'], ['b', 'c']); - expect(Utils.isOverflow(resultOne['a']), isFalse); + expect(resultOne['a'], {'0': 'b', '1': 'c'}); + expect(Utils.isOverflow(resultOne['a']), isTrue); }); test('mixed [0] and [] under tight listLimit', () { @@ -2682,8 +2984,8 @@ void main() { final resultOne = QS.decode('a[0]=b&a[]=c', const DecodeOptions(listLimit: 1)); - expect(resultOne['a'], ['b', 'c']); - expect(Utils.isOverflow(resultOne['a']), isFalse); + expect(resultOne['a'], {'0': 'b', '1': 'c'}); + expect(Utils.isOverflow(resultOne['a']), isTrue); }); test('mixed notation produces consistent overflow results', () { diff --git a/test/unit/utils_test.dart b/test/unit/utils_test.dart index 1c429d6..e6549a9 100644 --- a/test/unit/utils_test.dart +++ b/test/unit/utils_test.dart @@ -952,6 +952,61 @@ void main() { ); }); + group('with listLimit', () { + const strictOne = DecodeOptions( + listLimit: 1, + throwOnLimitExceeded: true, + ); + const softOne = DecodeOptions(listLimit: 1); + + test('converts primitive growth past the boundary to an overflow map', + () { + final merged = Utils.merge(['a'], 'b', softOne); + + expect(merged, {'0': 'a', '1': 'b'}); + expect(Utils.isOverflow(merged), isTrue); + }); + + test('converts scalar-list growth past the boundary to an overflow map', + () { + final merged = Utils.merge('a', ['b', 'c'], softOne); + + expect(merged, {'0': 'a', '1': 'b', '2': 'c'}); + expect(Utils.isOverflow(merged), isTrue); + }); + + test('converts list-list growth past the boundary to an overflow map', + () { + final merged = Utils.merge(['a'], ['b'], softOne); + + expect(merged, {'0': 'a', '1': 'b'}); + expect(Utils.isOverflow(merged), isTrue); + }); + + test('throws for every list growth path past the boundary', () { + expect(() => Utils.merge(['a'], 'b', strictOne), throwsRangeError); + expect( + () => Utils.merge('a', ['b', 'c'], strictOne), + throwsRangeError, + ); + expect( + () => Utils.merge(['a'], ['b'], strictOne), + throwsRangeError, + ); + }); + + test('keeps values at the boundary as a list', () { + expect(Utils.merge([], 'a', strictOne), ['a']); + }); + + test('preserves Set-specific behavior', () { + expect( + Utils.merge({'a'}, {'b'}, strictOne), + {'a', 'b'}, + ); + }); + }); + group('with overflow objects (from listLimit)', () { test('merges primitive into overflow object at next index', () { final overflow = @@ -1108,6 +1163,57 @@ void main() { 'c' ]); }); + + test('throws before mutating an existing overflow object', () { + final overflow = + Utils.combine(['a'], 'b', listLimit: 1) as Map; + + expect( + () => Utils.combine( + overflow, + 'c', + listLimit: 1, + throwOnLimitExceeded: true, + ), + throwsRangeError, + ); + expect(overflow, {'0': 'a', '1': 'b'}); + }); + }); + + group('with throwOnLimitExceeded', () { + test('throws when concatenation exceeds the limit', () { + expect( + () => Utils.combine( + ['a', 'b', 'c'], + 'd', + listLimit: 3, + throwOnLimitExceeded: true, + ), + throwsRangeError, + ); + expect( + () => Utils.combine( + [], + 'a', + listLimit: 0, + throwOnLimitExceeded: true, + ), + throwsRangeError, + ); + }); + + test('returns a list while within the limit', () { + expect( + Utils.combine( + ['a'], + 'b', + listLimit: 5, + throwOnLimitExceeded: true, + ), + ['a', 'b'], + ); + }); }); }); @@ -1265,6 +1371,21 @@ void main() { expect((out['parent'] as Map).containsKey('u'), isFalse); }); + test('handles multi-step cycles without infinite loop', () { + final a = {}; + final b = {}; + final c = {}; + a['b'] = b; + b['c'] = c; + c['d'] = a; + c['u'] = const Undefined(); + + final out = Utils.compact(a); + + expect(((out['b'] as Map)['c'] as Map)['d'], same(out)); + expect(((out['b'] as Map)['c'] as Map).containsKey('u'), isFalse); + }); + test('preserves order', () { final m = { 'first': 1, @@ -1293,6 +1414,44 @@ void main() { }); group('utils surrogate and charset edge cases (coverage)', () { + test('encodes a surrogate pair split across the first chunk boundary', + () { + final input = '${'a' * 1023}๐Ÿ˜€'; + + expect(Utils.encode(input), '${'a' * 1023}%F0%9F%98%80'); + }); + + test('encodes a surrogate pair split across a later chunk boundary', () { + final input = '${'a' * 2047}๐Ÿ˜€'; + + expect(Utils.encode(input), '${'a' * 2047}%F0%9F%98%80'); + }); + + test('encodes multiple boundary-split surrogate pairs', () { + final input = '${'a' * 1023}๐Ÿ˜€${'b' * 1022}๐Ÿ˜€'; + + expect( + RegExp('%F0%9F%98%80').allMatches(Utils.encode(input)).length, + 2, + ); + }); + + test('round-trips a boundary-split surrogate pair', () { + final input = '${'a' * 1023}๐Ÿ˜€'; + + expect(Uri.decodeComponent(Utils.encode(input)), input); + }); + + test('keeps lone high-surrogate behavior stable at a chunk boundary', () { + final input = '${'a' * 1023}\uD83DX'; + + expect( + Utils.encode(input).substring(1023), + Utils.encode('\uD83DX'), + ); + expect(Utils.encode(input), endsWith('%ED%A0%BDX')); + }); + test('lone high surrogate encoded as three UTF-8 bytes', () { const loneHigh = '\uD800'; final encoded = QS.encode({'s': loneHigh});