From 219d38a1bb3fa20f64f29ebe56b41ae764ffa2b7 Mon Sep 17 00:00:00 2001 From: Klemen Tusar Date: Sat, 4 Jul 2026 11:23:07 +0100 Subject: [PATCH 1/2] :bug: match qs 6.15.3 list limit enforcement Enforce cumulative limits across duplicate and mixed list merges. Reject oversized flat comma values before splitting or decoding, and add upstream parity regressions. --- src/qs_codec/decode.py | 29 +++--- src/qs_codec/utils/utils.py | 167 ++++++++++++++++---------------- tests/unit/decode_test.py | 185 +++++++++++++++++++++++++++++++++++- tests/unit/utils_test.py | 121 +++++++++++++++++++++-- 4 files changed, 395 insertions(+), 107 deletions(-) diff --git a/src/qs_codec/decode.py b/src/qs_codec/decode.py index 3e948fd..df73d23 100644 --- a/src/qs_codec/decode.py +++ b/src/qs_codec/decode.py @@ -63,7 +63,8 @@ def decode( Notes ----- - Empty/falsey ``value`` returns an empty dict. - - When the *number of top-level tokens* exceeds ``list_limit`` and ``parse_lists`` is enabled, the parser temporarily **disables list parsing** for this invocation to avoid quadratic work. This mirrors the behavior of other ports and keeps large flat query strings efficient. + - ``parse_lists`` is honored directly throughout decoding. ``list_limit`` is enforced while constructing and + merging lists without changing the configured list-parsing mode. """ obj: t.Dict[str, t.Any] = {} @@ -78,17 +79,6 @@ def decode( str_value: str = t.cast(str, value) if decode_from_string else "" mapping_value: t.Mapping[str, t.Any] = t.cast(t.Mapping[str, t.Any], value) if not decode_from_string else {} - parse_lists_effective: bool = opts.parse_lists - if decode_from_string and parse_lists_effective: - # Keep caller options immutable: compute a local parse_lists switch only for this invocation. - query = str_value.replace("?", "", 1) if opts.ignore_query_prefix else str_value - if isinstance(opts.delimiter, re.Pattern): - parts_count = len(re.split(opts.delimiter, query)) if query else 0 - else: - parts_count = (query.count(opts.delimiter) + 1) if query else 0 - if 0 < opts.list_limit < parts_count: - parse_lists_effective = False - if decode_from_string: temp_obj: t.Optional[t.Dict[str, t.Any]] = _parse_query_string_values(str_value, opts) else: @@ -118,7 +108,7 @@ def decode( obj[key] = val continue - new_obj: t.Any = _parse_keys(key, val, opts, decode_from_string, parse_lists=parse_lists_effective) + new_obj: t.Any = _parse_keys(key, val, opts, decode_from_string, parse_lists=opts.parse_lists) if not obj and isinstance(new_obj, dict): obj = new_obj @@ -253,10 +243,17 @@ def _parse_array_value( Either the original value or a list of values, without decoding (that happens later). """ if isinstance(value, str) and value and options.comma and "," in value: + if enforce_comma_limit and options.raise_on_limit_exceeded: + comma_count = 0 + comma_index = value.find(",") + while comma_index >= 0: + comma_count += 1 + if comma_count >= options.list_limit: + raise ValueError(_list_limit_exceeded_message(options.list_limit)) + comma_index = value.find(",", comma_index + 1) + split_val: t.List[str] = value.split(",") if enforce_comma_limit and len(split_val) > options.list_limit: - if options.raise_on_limit_exceeded: - raise ValueError(_list_limit_exceeded_message(options.list_limit)) return CommaOverflowDict({str(i): item for i, item in enumerate(split_val)}) return split_val @@ -384,7 +381,7 @@ def _parse_query_string_values(value: str, options: DecodeOptions) -> t.Dict[str part[pos + 1 :], options, len(obj[key]) if key in obj and isinstance(obj[key], (list, tuple)) else 0, - enforce_comma_limit=False, + enforce_comma_limit=not bracket_array_assignment, ) list_limit_exceeded = isinstance(parsed_value, (list, tuple)) and len(parsed_value) > options.list_limit if isinstance(parsed_value, (list, tuple)): diff --git a/src/qs_codec/utils/utils.py b/src/qs_codec/utils/utils.py index ad0e6b6..972934a 100644 --- a/src/qs_codec/utils/utils.py +++ b/src/qs_codec/utils/utils.py @@ -54,6 +54,16 @@ def _copy_overflow_append_value(value: t.Any) -> t.Any: return value +def _enforce_list_limit(values: t.List[t.Any], options: DecodeOptions) -> t.Union[t.List[t.Any], OverflowDict]: + """Return list values within the configured limit, or raise/degrade on overflow.""" + if not values or len(values) <= options.list_limit: + return values + if options.raise_on_limit_exceeded: + limit = options.list_limit + raise ValueError(f"List limit exceeded: Only {limit} element{'' if limit == 1 else 's'} allowed in a list.") + return OverflowDict({str(i): value for i, value in enumerate(values) if not isinstance(value, Undefined)}) + + @dataclass class _MergeFrame: target: t.Any @@ -66,7 +76,6 @@ class _MergeFrame: source_items: t.List[t.Tuple[t.Any, t.Any]] = field(default_factory=list) entry_index: int = 0 pending_key: t.Any = None - list_target: t.Dict[int, t.Any] = field(default_factory=dict) list_source: t.Dict[int, t.Any] = field(default_factory=dict) list_max_len: int = 0 list_index: int = 0 @@ -97,18 +106,20 @@ def merge( ------------------ - If `source` is ``None``: return `target` unchanged. - If `source` is **not** a mapping: - * `target` is a sequence → append/extend, skipping :class:`Undefined`. - * `target` is a mapping → write items from the sequence under string indices ("0", "1", …). - * otherwise → return ``[target, source]`` (skipping :class:`Undefined` where applicable). + * `target` is a sequence → merge sequence positions or append a scalar, retaining sparse holes as needed. + * `target` is a mapping → convert sequence positions to string keys ("0", "1", …) and deep-merge them. + * otherwise → combine the values, retaining sequence holes until final compaction. - If `source` **is** a mapping: - * `target` is not a mapping → if `target` is a sequence, coerce it to an index-keyed dict and merge; otherwise, concatenate as a list ``[target, source]`` while skipping :class:`Undefined`. + * `target` is not a mapping → if `target` is a sequence, coerce it to an index-keyed dict and merge; + otherwise, concatenate as a list ``[target, source]`` while skipping :class:`Undefined`. * `target` is a mapping → deep-merge keys; where keys collide, merge values recursively. List handling ------------- - When a list that already contains :class:`Undefined` must receive new values and ``options.parse_lists`` is ``False``, - the list is promoted to a dict with string indices so positions can be addressed deterministically. Otherwise, - sentinels are simply removed as we go. + :class:`Undefined` entries model sparse-array holes. They may remain in intermediate lists so indices and + ``list_limit`` checks match JavaScript array-length semantics. When ``options.parse_lists`` is ``False``, sparse + lists are promoted to dicts with string indices. Public decoding calls :meth:`compact` before returning, which + removes any retained placeholders; direct callers of :meth:`merge` may observe them in intermediate list results. Parameters ---------- @@ -147,7 +158,7 @@ def merge( if isinstance(current_target, (list, tuple)): # If the target sequence contains `Undefined`, we may need to promote it # to a dict keyed by indices for stable writes. - if any(isinstance(el, Undefined) for el in current_target): + if not frame.options.parse_lists and any(isinstance(el, Undefined) for el in current_target): target_by_index: t.Dict[int, t.Any] = dict(enumerate(current_target)) if isinstance(current_source, (list, tuple)): @@ -158,9 +169,7 @@ def merge( target_by_index[len(target_by_index)] = current_source # When list parsing is disabled, collapse to a string-keyed dict and drop sentinels. - if not frame.options.parse_lists and any( - isinstance(value, Undefined) for value in target_by_index.values() - ): + if any(isinstance(value, Undefined) for value in target_by_index.values()): result: t.Any = { str(i): target_by_index[i] for i in target_by_index @@ -169,28 +178,24 @@ def merge( else: result = [el for el in target_by_index.values() if not isinstance(el, Undefined)] stack.pop() - last_result = result + last_result = ( + _enforce_list_limit(result, frame.options) if isinstance(result, list) else result + ) continue if isinstance(current_source, (list, tuple)): - if all(isinstance(el, (ABCMapping, Undefined)) for el in current_target) and all( - isinstance(el, (ABCMapping, Undefined)) for el in current_source - ): - frame.list_target = dict(enumerate(current_target)) - frame.list_source = dict(enumerate(current_source)) - frame.list_max_len = max(len(frame.list_target), len(frame.list_source)) - frame.list_index = 0 - frame.list_merged = [] - frame.phase = "list_iter" - continue + frame.list_source = dict(enumerate(current_source)) + frame.list_max_len = len(current_source) + frame.list_index = 0 + frame.list_merged = list(current_target) + frame.phase = "list_iter" + continue - mutable_target: t.List[t.Any] = ( - list(current_target) if isinstance(current_target, tuple) else current_target - ) - # Mutates in-place by design for list targets to preserve merge performance. - mutable_target.extend(el for el in current_source if not isinstance(el, Undefined)) + candidate = [*current_target, current_source] + enforced = _enforce_list_limit(candidate, frame.options) + if isinstance(enforced, OverflowDict): stack.pop() - last_result = mutable_target + last_result = enforced continue mutable_target = list(current_target) if isinstance(current_target, tuple) else current_target @@ -200,19 +205,20 @@ def merge( continue if isinstance(current_target, ABCMapping): - if Utils.is_overflow(current_target): - stack.pop() - last_result = Utils.combine(current_target, current_source, frame.options) + if isinstance(current_source, (list, tuple)): + if Utils.is_overflow(current_target): + overflow_target = t.cast(OverflowDict, current_target) + frame.target = overflow_target.copy() + else: + frame.target = dict(current_target) + frame.source = { + str(i): item for i, item in enumerate(current_source) if not isinstance(item, Undefined) + } continue - # Target is a mapping but source is a sequence — coerce indices to string keys. - if isinstance(current_source, (list, tuple)): - new_target = dict(current_target) - for i, item in enumerate(current_source): - if not isinstance(item, Undefined): - new_target[str(i)] = item + if Utils.is_overflow(current_target): stack.pop() - last_result = new_target + last_result = Utils.combine(current_target, current_source, frame.options) continue if isinstance(current_source, Undefined) or current_source == "": @@ -238,10 +244,10 @@ def merge( if not isinstance(current_target, (list, tuple)) and isinstance(current_source, (list, tuple)): stack.pop() - last_result = [ - current_target, - *(el for el in current_source if not isinstance(el, Undefined)), - ] + last_result = _enforce_list_limit( + [current_target, *current_source], + frame.options, + ) continue stack.pop() @@ -252,13 +258,12 @@ def merge( # concatenate as a list, then proceed. if current_target is None or not isinstance(current_target, ABCMapping): if isinstance(current_target, (list, tuple)): - stack.pop() - last_result = { - **{ - str(i): item for i, item in enumerate(current_target) if not isinstance(item, Undefined) - }, - **current_source, + target_values = { + str(i): item for i, item in enumerate(current_target) if not isinstance(item, Undefined) } + frame.target = ( + OverflowDict(target_values) if Utils.is_overflow(current_source) else target_values + ) continue if Utils.is_overflow(current_source): @@ -292,7 +297,7 @@ def merge( if not isinstance(current_source, Undefined): result_list.append(current_source) stack.pop() - last_result = result_list + last_result = _enforce_list_limit(result_list, frame.options) continue # Prepare a mutable target we can merge into; reuse dict targets for performance. @@ -347,39 +352,37 @@ def merge( if frame.phase == "list_iter": if frame.list_index >= frame.list_max_len: stack.pop() - last_result = frame.list_merged + enforced = _enforce_list_limit(frame.list_merged, frame.options) + if isinstance(frame.target, list) and isinstance(enforced, list): + frame.target[:] = enforced + last_result = frame.target + else: + last_result = enforced continue idx = frame.list_index frame.list_index += 1 - has_target = idx in frame.list_target - has_source = idx in frame.list_source - - if has_target and has_source: - target_value = frame.list_target[idx] - source_value = frame.list_source[idx] - - if isinstance(source_value, Undefined): - if not isinstance(target_value, Undefined): - frame.list_merged.append(target_value) - continue - - frame.phase = "list_wait_child" - stack.append(_MergeFrame(target=target_value, source=source_value, options=frame.options)) + source_value = frame.list_source[idx] + if isinstance(source_value, Undefined): continue + has_target = idx < len(frame.list_merged) and not isinstance(frame.list_merged[idx], Undefined) if has_target: - target_value = frame.list_target[idx] - if not isinstance(target_value, Undefined): - frame.list_merged.append(target_value) - elif has_source: - source_value = frame.list_source[idx] - if not isinstance(source_value, Undefined): + target_value = frame.list_merged[idx] + if isinstance(target_value, ABCMapping) and isinstance(source_value, ABCMapping): + frame.phase = "list_wait_child" + stack.append(_MergeFrame(target=target_value, source=source_value, options=frame.options)) + else: frame.list_merged.append(source_value) + continue + + while len(frame.list_merged) <= idx: + frame.list_merged.append(Undefined()) + frame.list_merged[idx] = source_value continue # frame.phase == "list_wait_child" - frame.list_merged.append(last_result) + frame.list_merged[frame.list_index - 1] = last_result frame.phase = "list_iter" return last_result @@ -548,12 +551,15 @@ def combine( ``list_limit`` from :class:`DecodeOptions` is used. A negative ``list_limit`` is treated as "overflow immediately": any non-empty combined result will be converted to :class:`OverflowDict`. - This helper never raises an exception when the limit is exceeded; even - if :class:`DecodeOptions` has ``raise_on_limit_exceeded`` set to - ``True``, ``combine`` will still handle overflow only by converting the - list to :class:`OverflowDict`. + When :attr:`DecodeOptions.raise_on_limit_exceeded` is ``True``, an + over-limit result raises ``ValueError`` instead of being converted. """ if Utils.is_overflow(a): + if options is not None and options.raise_on_limit_exceeded: + limit = options.list_limit + raise ValueError( + f"List limit exceeded: Only {limit} element{'' if limit == 1 else 's'} allowed in a list." + ) # a is already an OverflowDict. Append b as one value at the next numeric index. orig_a: OverflowDict = t.cast(OverflowDict, a) a_copy: OverflowDict = orig_a.__class__({k: v for k, v in orig_a.items() if not isinstance(v, Undefined)}) @@ -589,14 +595,7 @@ def combine( res: t.List[t.Any] = [*list_a, *list_b] - list_limit: int = options.list_limit if options else DecodeOptions().list_limit - if list_limit < 0: - return OverflowDict({str(i): x for i, x in enumerate(res)}) if res else res - if len(res) > list_limit: - # Convert to OverflowDict - return OverflowDict({str(i): x for i, x in enumerate(res)}) - - return res + return _enforce_list_limit(res, options if options is not None else DecodeOptions()) @staticmethod def apply( diff --git a/tests/unit/decode_test.py b/tests/unit/decode_test.py index 6e0d1a3..05ddca2 100644 --- a/tests/unit/decode_test.py +++ b/tests/unit/decode_test.py @@ -1107,6 +1107,51 @@ def test_parses_empty_keys(self, encoded: str, decoded: t.Mapping[str, t.Any]) - assert decode(encoded) == decoded +class TestUnbalancedBracketKeys: + @pytest.mark.parametrize( + "query, options, expected", + [ + pytest.param("a[bc=v", DecodeOptions(), {"a": {"[bc": "v"}}, id="unclosed-group"), + pytest.param("a[=v", DecodeOptions(), {"a": {"[": "v"}}, id="bare-unclosed-bracket"), + pytest.param("a[b][c=v", DecodeOptions(), {"a": {"b": {"[c": "v"}}}, id="after-valid-group"), + pytest.param("a[b]c[d=v", DecodeOptions(), {"a": {"b": {"[d": "v"}}}, id="after-text"), + pytest.param( + "filters[customtags:Env: Prod=v", + DecodeOptions(), + {"filters": {"[customtags:Env: Prod": "v"}}, + id="issue-558", + ), + pytest.param("][a=v", DecodeOptions(), {"]": {"[a": "v"}}, id="stray-close-prefix"), + pytest.param("a][b=v", DecodeOptions(), {"a]": {"[b": "v"}}, id="stray-close-parent"), + pytest.param("a[b[c=v", DecodeOptions(), {"a": {"[b[c": "v"}}, id="inner-open-bracket"), + pytest.param("a[b[c]=v", DecodeOptions(), {"a": {"[b[c]": "v"}}, id="inner-balanced-bracket"), + pytest.param("a[b][c[d=v", DecodeOptions(), {"a": {"b": {"[c[d": "v"}}}, id="inner-open-after-valid"), + pytest.param("[abc=v", DecodeOptions(), {"[abc": "v"}, id="bracket-prefixed"), + pytest.param("[[]b=v", DecodeOptions(), {"[[]b": "v"}, id="nested-bracket-prefixed"), + pytest.param( + "a[b]c[d]e[f=v", + DecodeOptions(depth=5), + {"a": {"b": {"d": {"[f": "v"}}}}, + id="depth-five", + ), + pytest.param( + "a[b]c[d]e[f=v", + DecodeOptions(depth=1), + {"a": {"b": {"[d]e[f": "v"}}}, + id="depth-one", + ), + pytest.param("a[bc=v", DecodeOptions(depth=0), {"a[bc": "v"}, id="depth-zero"), + pytest.param("a.b[c=v", DecodeOptions(allow_dots=True), {"a": {"b": {"[c": "v"}}}, id="allow-dots"), + pytest.param("a]b=v", DecodeOptions(), {"a]b": "v"}, id="stray-close-literal"), + pytest.param("a[b]extra=v", DecodeOptions(), {"a": {"b": "v"}}, id="trailing-text"), + ], + ) + def test_decodes_unbalanced_brackets_leniently( + self, query: str, options: DecodeOptions, expected: t.Mapping[str, t.Any] + ) -> None: + assert decode(query, options) == expected + + class TestCharset: url_encoded_checkmark_in_utf_8: str = "%E2%9C%93" url_encoded_oslash_in_utf_8: str = "%C3%B8" @@ -1446,6 +1491,134 @@ def test_parameter_limit( class TestListLimit: + @pytest.mark.parametrize( + "query, options", + [ + pytest.param( + "a=1,2,3&a=4,5,6", + DecodeOptions(comma=True, list_limit=5, raise_on_limit_exceeded=True), + id="cumulative-comma-groups", + ), + pytest.param( + "a=v,v,v,v,v&a=v,v,v,v,v&a=v,v,v,v,v", + DecodeOptions(comma=True, list_limit=5, raise_on_limit_exceeded=True), + id="already-overflowed-comma-groups", + ), + pytest.param("a=x&a=y", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True), id="duplicate-scalars"), + pytest.param( + "a[]=x&a[]=y", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True), id="duplicate-brackets" + ), + pytest.param( + "a=x&a[0]=y", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True), id="scalar-then-index" + ), + pytest.param( + "a[0]=x&a=y", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True), id="index-then-scalar" + ), + pytest.param( + "a[0]=x&a[]=y", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True), id="index-then-bracket" + ), + ], + ) + def test_raises_when_cumulative_list_growth_exceeds_limit(self, query: str, options: DecodeOptions) -> None: + with pytest.raises(ValueError, match="List limit exceeded"): + decode(query, options) + + @pytest.mark.parametrize( + "query", + [ + pytest.param("a=x&a[0]=y", id="scalar-then-index"), + pytest.param("a[0]=x&a=y", id="index-then-scalar"), + pytest.param("a[0]=x&a[]=y", id="index-then-bracket"), + ], + ) + def test_mixed_notation_becomes_overflow_dict_without_raising(self, query: str) -> None: + result = decode(query, DecodeOptions(list_limit=1)) + + assert result == {"a": {"0": "x", "1": "y"}} + assert isinstance(result["a"], OverflowDict) + + def test_cumulative_comma_groups_become_overflow_dict_without_raising(self) -> None: + result = decode("a=1,2,3&a=4,5,6", DecodeOptions(comma=True, list_limit=5)) + + assert result == {"a": {"0": "1", "1": "2", "2": "3", "3": "4", "4": "5", "5": "6"}} + assert isinstance(result["a"], OverflowDict) + + @pytest.mark.parametrize( + "query, expected", + [ + pytest.param( + "a=1,2&a[]=x", + {"a": {"0": ["1", "x"], "1": "2"}}, + id="overflow-then-bracket", + ), + pytest.param( + "a[]=x&a=1,2", + {"a": {"0": ["x", "1"], "1": "2"}}, + id="bracket-then-overflow", + ), + pytest.param( + "a=x&a=x&a[]=x", + {"a": {"0": ["x", "x"], "1": "x"}}, + id="duplicates-then-bracket", + ), + ], + ) + def test_overflow_values_merge_with_bracket_assignments(self, query: str, expected: t.Mapping[str, t.Any]) -> None: + result = decode(query, DecodeOptions(comma=True, list_limit=1)) + + assert result == expected + assert isinstance(result["a"], OverflowDict) + + def test_mapping_values_merge_with_later_bracket_assignment(self) -> None: + result = decode("a=1,2&a[2]=z&a[]=x", DecodeOptions(comma=True, list_limit=2)) + + assert result == {"a": {"0": ["1", "x"], "1": "2", "2": "z"}} + + def test_raises_before_decoding_oversized_flat_comma_value(self) -> None: + decoded_values: t.List[str] = [] + + def decoder(value: t.Optional[str], charset: t.Optional[Charset], kind: DecodeKind) -> t.Optional[str]: + if kind == DecodeKind.VALUE and value is not None: + decoded_values.append(value) + return DecodeUtils.decode(value, charset) + + options = DecodeOptions( + comma=True, + list_limit=1, + raise_on_limit_exceeded=True, + decoder=decoder, + ) + + with pytest.raises(ValueError, match="List limit exceeded"): + decode("a=1,2", options) + + assert decoded_values == [] + + def test_raises_before_decoding_oversized_nested_flat_comma_value(self) -> None: + with pytest.raises(ValueError, match="List limit exceeded"): + decode( + "a[b]=1,2,3,4,5,6", + DecodeOptions(comma=True, list_limit=5, raise_on_limit_exceeded=True), + ) + + def test_keeps_flat_comma_values_at_or_below_limit(self) -> None: + options = DecodeOptions(comma=True, list_limit=5, raise_on_limit_exceeded=True) + + assert decode("a=1,2,3&a=4", options) == {"a": ["1", "2", "3", "4"]} + assert decode("a=1,2,3,4,5", options) == {"a": ["1", "2", "3", "4", "5"]} + + def test_counts_bracketed_comma_groups_as_outer_elements(self) -> None: + options = DecodeOptions(comma=True, list_limit=5, raise_on_limit_exceeded=True) + + assert decode("a[]=1,2,3&a[]=4,5,6", options) == {"a": [["1", "2", "3"], ["4", "5", "6"]]} + assert decode("a[]=1,2,3,4,5,6", options) == {"a": [["1", "2", "3", "4", "5", "6"]]} + + with pytest.raises(ValueError, match="List limit exceeded"): + decode( + "a[]=1,2,3", + DecodeOptions(comma=True, list_limit=0, raise_on_limit_exceeded=True), + ) + def test_current_list_length_calculation(self) -> None: # Test for line 166 in decode.py # This test creates a scenario where the current list length is calculated @@ -1603,6 +1776,11 @@ def test_bracket_comma_list_negative_limit_converts_wrapped_value_to_overflow_di {"foo": {"0": [["1", "2", "3", "4"]]}}, id="parse-lists-disabled", ), + pytest.param( + DecodeOptions(comma=True, list_limit=0, parse_lists=False), + {"foo": {"0": ["1", "2", "3", "4"]}}, + id="parse-lists-disabled-over-outer-list-limit", + ), ], ) def test_mapping_bracket_comma_list_matches_query_string_path( @@ -1847,7 +2025,12 @@ class TestDecodeMixedBypassParity: [ pytest.param("a=1&a[b]=2", None, {"a": ["1", {"b": "2"}]}, id="flat-before-structured"), pytest.param("a[b]=2&a=1", None, {"a": [{"b": "2"}, "1"]}, id="structured-before-flat"), - pytest.param("0=y&[]=x", None, {"0": "x"}, id="flat-zero-collides-leading-bracket-root"), + pytest.param( + "0=y&[]=x", + None, + {"0": ["y", "x"]}, + id="flat-zero-combines-leading-bracket-root", + ), pytest.param("[]=x&0=y", None, {"0": ["x", "y"]}, id="leading-bracket-root-collides-flat-zero"), pytest.param( "a[b]=1&002=2", diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py index 354e39f..09c94a4 100644 --- a/tests/unit/utils_test.py +++ b/tests/unit/utils_test.py @@ -545,11 +545,19 @@ def test_merge_with_none_source_returns_target(self) -> None: assert result == {"a": 1} assert result is target - def test_merge_promotes_list_with_undefined_when_lists_disabled(self) -> None: + @pytest.mark.parametrize( + "source, expected", + [ + pytest.param([Undefined()], {"1": "keep"}, id="undefined-source-keeps-sparse-dict"), + pytest.param(["replace"], ["replace", "keep"], id="list-source-fills-sparse-slot"), + pytest.param("append", {"1": "keep", "2": "append"}, id="scalar-source-appends-to-sparse-dict"), + ], + ) + def test_merge_promotes_list_with_undefined_when_lists_disabled(self, source: t.Any, expected: t.Any) -> None: options = DecodeOptions(parse_lists=False) target = [Undefined(), "keep"] - result = Utils.merge(target, [Undefined()], options) - assert result == {"1": "keep"} + result = Utils.merge(target, source, options) + assert result == expected def test_merges_two_dicts_with_same_key(self) -> None: assert Utils.merge({"a": "b"}, {"a": "c"}) == {"a": ["b", "c"]} @@ -570,6 +578,79 @@ def test_merges_object_sandwiched_by_two_standalones_into_array(self) -> None: def test_merges_two_arrays_into_an_array(self) -> None: assert Utils.merge({"foo": ["baz"]}, {"foo": ["bar", "xyzzy"]}) == {"foo": ["baz", "bar", "xyzzy"]} + @pytest.mark.parametrize( + "target, source", + [ + pytest.param(["a"], "b", id="list-scalar"), + pytest.param("a", ["b", "c"], id="scalar-list"), + pytest.param("a", {"b": "c"}, id="scalar-mapping"), + pytest.param(["a"], ["b"], id="list-list"), + ], + ) + def test_merge_raises_when_list_growth_exceeds_limit(self, target: t.Any, source: t.Any) -> None: + original_target = list(target) if isinstance(target, list) else target + options = DecodeOptions(list_limit=1, raise_on_limit_exceeded=True) + + with pytest.raises(ValueError, match="List limit exceeded"): + Utils.merge(target, source, options) + + assert target == original_target + + @pytest.mark.parametrize( + "target, source, expected", + [ + pytest.param(["a"], "b", {"0": "a", "1": "b"}, id="list-scalar"), + pytest.param("a", ["b", "c"], {"0": "a", "1": "b", "2": "c"}, id="scalar-list"), + pytest.param("a", {"b": "c"}, {"0": "a", "1": {"b": "c"}}, id="scalar-mapping"), + pytest.param(["a"], ["b"], {"0": "a", "1": "b"}, id="list-list"), + ], + ) + def test_merge_converts_over_limit_growth_to_overflow_dict( + self, target: t.Any, source: t.Any, expected: t.Mapping[str, t.Any] + ) -> None: + result = Utils.merge(target, source, DecodeOptions(list_limit=1)) + + assert result == expected + assert isinstance(result, OverflowDict) + + def test_merge_keeps_list_at_limit(self) -> None: + assert Utils.merge([], "a", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True)) == ["a"] + + def test_merge_enforces_limit_after_nested_list_merge(self) -> None: + target = [{"a": 1}] + source = [{"b": 2}, {"c": 3}] + + result = Utils.merge(target, source, DecodeOptions(list_limit=1)) + + assert result == {"0": {"a": 1, "b": 2}, "1": {"c": 3}} + assert isinstance(result, OverflowDict) + + with pytest.raises(ValueError, match="List limit exceeded"): + Utils.merge([{"a": 1}], source, DecodeOptions(list_limit=1, raise_on_limit_exceeded=True)) + + def test_merge_combines_overflow_target_with_list_by_index(self) -> None: + target = OverflowDict({"0": "1", "1": "2"}) + + result = Utils.merge(target, ["x"], DecodeOptions(list_limit=1)) + + assert result == {"0": ["1", "x"], "1": "2"} + assert isinstance(result, OverflowDict) + + def test_merge_combines_list_target_with_overflow_source_by_index(self) -> None: + source = OverflowDict({"0": "1", "1": "2"}) + + result = Utils.merge(["x"], source, DecodeOptions(list_limit=1)) + + assert result == {"0": ["x", "1"], "1": "2"} + assert isinstance(result, OverflowDict) + + def test_merge_combines_mapping_target_with_list_by_index(self) -> None: + target = {"0": "1", "1": "2", "2": "z"} + + result = Utils.merge(target, ["x"], DecodeOptions(list_limit=2)) + + assert result == {"0": ["1", "x"], "1": "2", "2": "z"} + def test_merge_preserves_target_only_slots_in_structured_lists(self) -> None: options = DecodeOptions() target = [{"a": 1}, {"orphan": 2}] @@ -742,6 +823,21 @@ def test_combine_at_list_limit_stays_list(self) -> None: combined = Utils.combine(["a"], ["b"], options) assert combined == ["a", "b"] + def test_combine_raises_when_list_limit_is_exceeded(self) -> None: + options = DecodeOptions(list_limit=1, raise_on_limit_exceeded=True) + + with pytest.raises(ValueError, match="List limit exceeded"): + Utils.combine(["a"], "b", options) + + def test_combine_raises_before_copying_existing_overflow_dict(self) -> None: + overflow = Utils.combine(["a"], "b", DecodeOptions(list_limit=1)) + assert isinstance(overflow, OverflowDict) + + with pytest.raises(ValueError, match="List limit exceeded"): + Utils.combine(overflow, "c", DecodeOptions(list_limit=1, raise_on_limit_exceeded=True)) + + assert overflow == {"0": "a", "1": "b"} + def test_combine_negative_list_limit_with_empty_result_stays_list(self) -> None: options = DecodeOptions(list_limit=-1) combined = Utils.combine([], [], options) @@ -794,6 +890,20 @@ def test_compact_removes_undefined_entries_and_avoids_cycles(self) -> None: assert "drop" not in root assert root["nested"][0] == {"keep": "ok"} + def test_compact_handles_multi_step_cycles(self) -> None: + a: t.Dict[str, t.Any] = {} + b: t.Dict[str, t.Any] = {} + c: t.Dict[str, t.Any] = {} + a["b"] = b + b["c"] = c + c["d"] = a + c["drop"] = Undefined() + + result = Utils.compact(a) + + assert result["b"]["c"]["d"] is result + assert "drop" not in result["b"]["c"] + def test_remove_undefined_from_list_handles_nested_structures(self) -> None: data: t.List[t.Any] = [ Undefined(), @@ -1041,9 +1151,8 @@ def test_merge_source_is_overflow_dict_into_list(self) -> None: # source has key '0', which collides with target's index 0 source = OverflowDict({"0": "b"}) result = Utils.merge(target, source) # type: ignore[arg-type] - assert isinstance(result, dict) - # Source overwrites target at key '0' - assert result == {"0": "b"} + assert isinstance(result, OverflowDict) + assert result == {"0": ["a", "b"]} def test_merge_overflow_dict_with_mapping_preserves_overflow(self) -> None: target = OverflowDict({"0": "a"}) From 088d918c48e78d7cee22b85c68f30cd3904e2605 Mon Sep 17 00:00:00 2001 From: Klemen Tusar Date: Sat, 4 Jul 2026 11:23:21 +0100 Subject: [PATCH 2/2] :memo: document qs 6.15.3 list limit behavior Describe cumulative enforcement, numeric-key overflow, ValueError semantics, and bracketed comma groups across the changelog, public docs, option docstrings, and qs-codec skill. --- CHANGELOG.md | 6 ++++++ README.rst | 31 +++++++++++++++++++++------ docs/README.rst | 19 ++++++++++++++++ docs/index.rst | 2 +- skills/qs-codec/SKILL.md | 11 +++++++--- src/qs_codec/models/decode_options.py | 10 ++++++--- 6 files changed, 65 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb08ee6..01c45cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.6.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 or decoding their values when `raise_on_limit_exceeded` is enabled +* [CHORE] add Node `qs` 6.15.3 regression coverage for unbalanced bracket keys and cyclic compaction + ## 1.6.0 * [CHORE] make `Undefined` internal by removing `qs_codec.Undefined` and its public documentation diff --git a/README.rst b/README.rst index 024d2e5..4d96cf8 100644 --- a/README.rst +++ b/README.rst @@ -25,7 +25,7 @@ Highlights - Pluggable hooks: custom ``encoder``/``decoder`` callables; options to sort keys, filter output, and control percent-encoding (keys-only, values-only). - Nulls & empties: ``strict_null_handling`` and ``skip_nulls``; support for empty lists/arrays when desired. - Dates: ``serialize_date`` for ISO 8601 or custom (e.g., UNIX timestamp). -- Safety limits: configurable decode depth and encode max depth, parameter limit, and list index limit; optional strict-depth errors; duplicate-key strategies (combine/first/last). +- Safety limits: configurable decode depth and encode max depth, parameter limit, and list element limit; optional strict-depth errors; duplicate-key strategies (combine/first/last). - Extras: numeric entity decoding (e.g. ``☺`` → ☺), alternate delimiters/regex, and query-prefix helpers. Compatibility @@ -497,12 +497,11 @@ Note that an empty ``str``\ing is also a value, and will be preserved: assert qs.decode('a[0]=b&a[1]=&a[2]=c') == {'a': ['b', '', 'c']} -`decode `__ will also limit specifying indices -in a ``list`` to a maximum index of ``20``. Any ``list`` members with an -index of greater than ``20`` will instead be converted to a ``dict`` with -the index as the key. This is needed to handle cases when someone sent, -for example, ``a[999999999]`` and it will take significant time to iterate -over this huge ``list``. +`decode `__ also limits each ``list`` to a +maximum element count of ``20``. Index ``19`` is the last index that can create +a default ``list``; index ``20`` and higher are converted to a ``dict`` with +the index as the key. This prevents inputs such as ``a[999999999]`` from +creating massive sparse lists. .. code:: python @@ -522,6 +521,24 @@ option: qs.DecodeOptions(list_limit=0), ) == {'a': {'1': 'b'}} +The same limit is enforced cumulatively when duplicate keys, mixed list +notation, or comma-separated values grow a list. A result exactly at the limit +remains a ``list``. Above the limit, decoding uses a numeric-keyed ``dict`` by +default, or raises ``ValueError`` when ``raise_on_limit_exceeded=True``. + +.. code:: python + + import qs_codec as qs + + assert qs.decode( + 'a=x&a=y', + qs.DecodeOptions(list_limit=1), + ) == {'a': {'0': 'x', '1': 'y'}} + +With ``comma=True``, a flat comma value is subject to the same limit. A value +assigned through ``[]=`` counts as one outer list element, so its inner +comma-separated group may contain more values than ``list_limit``. + To disable ``list`` parsing entirely, set `parse_lists `__ to ``False``. diff --git a/docs/README.rst b/docs/README.rst index 0d693ab..1dcae6c 100644 --- a/docs/README.rst +++ b/docs/README.rst @@ -427,6 +427,25 @@ option: qs.DecodeOptions(list_limit=0), ) == {'a': {'1': 'b'}} +The same limit is enforced cumulatively when duplicate keys, mixed list +notation, or comma-separated values grow a list. A result exactly at the limit +remains a ``list``. Above the limit, decoding uses a numeric-keyed ``dict`` by +default, or raises ``ValueError`` when +:py:attr:`raise_on_limit_exceeded ` is ``True``. + +.. code:: python + + import qs_codec as qs + + assert qs.decode( + 'a=x&a=y', + qs.DecodeOptions(list_limit=1), + ) == {'a': {'0': 'x', '1': 'y'}} + +With ``comma=True``, a flat comma value is subject to the same limit. A value +assigned through ``[]=`` counts as one outer list element, so its inner +comma-separated group may contain more values than ``list_limit``. + To disable ``list`` parsing entirely, set :py:attr:`parse_lists ` to ``False``. diff --git a/docs/index.rst b/docs/index.rst index 709bdbb..c5c85ae 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -29,7 +29,7 @@ Highlights - Pluggable hooks: custom ``encoder``/``decoder`` callables; options to sort keys, filter output, and control percent-encoding (keys-only, values-only). - Nulls & empties: ``strict_null_handling`` and ``skip_nulls``; support for empty lists/arrays when desired. - Dates: ``serialize_date`` for ISO 8601 or custom (e.g., UNIX timestamp). -- Safety limits: configurable decode depth and encode max depth, parameter limit, and list index limit; optional strict-depth errors; duplicate-key strategies (combine/first/last). +- Safety limits: configurable decode depth and encode max depth, parameter limit, and list element limit; optional strict-depth errors; duplicate-key strategies (combine/first/last). - Extras: numeric entity decoding (e.g. ``☺`` → ☺), alternate delimiters/regex, and query-prefix helpers. Compatibility diff --git a/skills/qs-codec/SKILL.md b/skills/qs-codec/SKILL.md index b554924..209da8d 100644 --- a/skills/qs-codec/SKILL.md +++ b/skills/qs-codec/SKILL.md @@ -184,8 +184,11 @@ Use these options with `qs.decode(query, qs.DecodeOptions(...))`: list; use `qs.Duplicates.FIRST` or `qs.Duplicates.LAST` to collapse. - Bracket lists: enabled by default; set `parse_lists=False` to treat list syntax as dictionary keys. -- Large or sparse list indices: default `list_limit` is `20`; indices above the - limit become dictionary keys. +- List limits: default `list_limit` is `20`; numeric indices at or above the + limit become dictionary keys. The limit also applies cumulatively to lists + grown by duplicate keys, mixed notation, or comma-separated values. Exact-limit + results remain lists; soft overflow becomes a numeric-keyed dictionary, while + `raise_on_limit_exceeded=True` raises `ValueError`. - Comma-separated values such as `a=b,c`: `comma=True`. - Tokens without `=` as `None`: `strict_null_handling=True`. - Custom delimiters: `delimiter=";"` or `delimiter=re.compile(r"[;,]")`. @@ -279,7 +282,9 @@ Warn or adjust before giving code for these cases: silently truncating. - `list_limit` has nuanced list-construction behavior; negative values disable numeric-index list parsing, and `raise_on_limit_exceeded=True` turns list - limit violations into `ValueError`. + limit violations into `ValueError`. With `comma=True`, a flat comma value is + checked before value decoding, while a comma group assigned through `[]=` + counts as one outer list element. - Built-in charset handling supports only `qs.Charset.UTF8` and `qs.Charset.LATIN1`; other encodings require a custom `encoder` or `decoder`. - `EncodeOptions.encoder` is ignored when `encode=False`. diff --git a/src/qs_codec/models/decode_options.py b/src/qs_codec/models/decode_options.py index 871273b..0a13bc2 100644 --- a/src/qs_codec/models/decode_options.py +++ b/src/qs_codec/models/decode_options.py @@ -47,7 +47,11 @@ class DecodeOptions: limit, index ``19`` is the last index that can create a list; index ``20`` already overflows to a ``dict``. - This limit also applies to decoded list growth from comma-split values when ``comma=True``. + This limit also applies cumulatively to list growth from duplicate keys, mixed list + notation, and comma-split values when ``comma=True``. A result exactly at the limit remains + a list. Above the limit, decoding either uses a numeric-keyed mapping or raises ``ValueError`` + when ``raise_on_limit_exceeded=True``. + For bracket-array assignments such as ``foo[]=1,2,3``, the comma-split payload is wrapped as a single outer list element, so the inner payload may contain more values than ``list_limit`` while still respecting the outer container limit. @@ -122,11 +126,11 @@ class DecodeOptions: """Raise instead of degrading gracefully when limits are exceeded. When ``True``, the decoder raises: - - a ``DecodeError`` for parameter and list limit violations; and + - a ``ValueError`` for parameter and list limit violations; and - an ``IndexError`` when nesting deeper than ``depth`` **and** ``strict_depth=True``. When ``False`` (default), the decoder degrades gracefully: it slices the parameter list - at ``parameter_limit``, stops adding items beyond ``list_limit``, and—if + at ``parameter_limit``, represents list overflows as numeric-keyed mappings, and—if ``strict_depth=True``—stops descending once ``depth`` is reached without raising. """