🐛 implement DecodeOptions.list_limit handling in Utils.combine function to prevent DoS via memory exhaustion
#37
Conversation
…a memory exhaustion
|
Warning Rate limit exceeded@techouse has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces list overflow handling to prevent denial-of-service attacks via deeply nested query string arrays. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Decoder as Decoder Logic
participant Utils as Utils
participant OverflowDict as OverflowDict
Decoder->>Decoder: Parse query string values
Decoder->>Utils: is_overflow(leaf)?
Utils-->>Decoder: true/false
alt Overflow Detected or List Limit Hit
Decoder->>Utils: combine(existing, new_val, options)
Utils->>Utils: Check if inputs are OverflowDict
Utils->>Utils: Merge/flatten with numeric indices
Utils->>Utils: Compare length vs list_limit
alt Exceeds Limit
Utils->>OverflowDict: Create with string keys
OverflowDict-->>Utils: OverflowDict instance
else Within Limit
Utils->>Utils: Return as list or dict
end
Utils-->>Decoder: Result (OverflowDict or list/dict)
else Normal Path
Decoder->>Utils: combine(existing, new_val, options)
Utils-->>Decoder: Merged result
end
Decoder->>Decoder: Finalize parse result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #37 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 17 +1
Lines 1162 1261 +99
=========================================
+ Hits 1162 1261 +99 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request implements list limit handling in the query string decoder to prevent denial-of-service (DoS) attacks via memory exhaustion. When decoded lists exceed the configured list_limit, they are converted to a special OverflowDict type (a dictionary with numeric string keys) rather than continuing to grow as lists.
Changes:
- Introduced
OverflowDictclass to represent lists that exceeded the limit - Modified
Utils.combineto enforce list limits and handle OverflowDict instances - Updated
Utils.mergeto detect and handle OverflowDict instances - Enhanced decoding logic in
decode.pyto check for overflow conditions - Added comprehensive tests including DoS attack simulation and list limit edge cases
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/qs_codec/utils/utils.py |
Adds OverflowDict class, updates combine to enforce limits and return dicts when exceeded, adds overflow detection in merge |
src/qs_codec/decode.py |
Imports OverflowDict, passes options to combine, checks for overflow in _parse_object |
tests/unit/decode_test.py |
Updates expected results for list_limit=0 cases, adds TestCVE2024 class with DoS attack tests |
Comments suppressed due to low confidence (1)
src/qs_codec/utils/utils.py:191
- When merging a non-mapping target with an
OverflowDictsource, the function extracts values and creates a list_reswithout checking if the resulting list exceedslist_limit. This could bypass the DoS protection. After creating_res, check ifoptionshas alist_limitand convert toOverflowDictif exceeded, similar to the logic in thecombinefunction.
_res: t.List[t.Any] = []
_iter1 = target if isinstance(target, (list, tuple)) else [target]
for _el in _iter1:
if not isinstance(_el, Undefined):
_res.append(_el)
_iter2 = (
source
if isinstance(source, (list, tuple))
else (list(source.values()) if Utils.is_overflow(source) else [source])
)
for _el in _iter2:
if not isinstance(_el, Undefined):
_res.append(_el)
return _res
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/qs_codec/utils/utils.py (1)
345-383: Well-implemented overflow handling incombine.The implementation correctly:
- Appends to existing
OverflowDictat sequential indices- Handles all source types (list, tuple, OverflowDict, scalar)
- Defaults to
list_limit=20when options isNone- Converts result to
OverflowDictonly whenlen(res) > list_limitOne minor note: the static analysis flagged line 352 for using a non-breaking hyphen (
‑) in the docstring instead of a regular hyphen (-). This is a cosmetic issue.🔧 Optional: Fix non-breaking hyphen in docstring
""" Concatenate two values, treating non‑sequences as singletons. - If `list_limit` is exceeded, converts the list to an `OverflowDict` - (a dict with numeric keys) to prevent memory exhaustion. + If `list_limit` is exceeded, converts the list to an `OverflowDict` + (a dict with numeric keys) to prevent memory exhaustion. """The word "non‑sequences" uses a non-breaking hyphen. Consider using a regular hyphen for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/qs_codec/decode.pysrc/qs_codec/utils/utils.pytests/comparison/package.jsontests/unit/decode_test.py
🧰 Additional context used
📓 Path-based instructions (7)
src/qs_codec/{encode.py,decode.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/qs_codec/{encode.py,decode.py}: Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Implement dot handling: allow_dots toggles dotted path parsing; encode_dot_in_keys/decode_dot_in_keys must percent-encode/decode literal dots with correct order and validation
Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Avoid repeated regex compilation and deep copies in tight loops; reuse preprocessing stages (tokenize first, structure later)
Files:
src/qs_codec/decode.py
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/decode.pysrc/qs_codec/utils/utils.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/decode.pytests/unit/decode_test.pysrc/qs_codec/utils/utils.py
src/qs_codec/decode.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Treat option dataclasses as immutable; when toggling transient flags in decode.decode, clone with dataclasses.replace
Files:
src/qs_codec/decode.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/decode_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/decode_test.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/decode.pytests/unit/decode_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
src/qs_codec/decode.pytests/unit/decode_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/decode.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/decode.pytests/unit/decode_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/decode.pytests/unit/decode_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
src/qs_codec/decode.pytests/unit/decode_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/decode.py : Treat option dataclasses as immutable; when toggling transient flags in decode.decode, clone with dataclasses.replace
Applied to files:
src/qs_codec/decode.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Avoid repeated regex compilation and deep copies in tight loops; reuse preprocessing stages (tokenize first, structure later)
Applied to files:
src/qs_codec/decode.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode_options.py,decode_options.py} : Extend option behavior by updating the corresponding dataclass with defaults and type hints (encode_options.py or decode_options.py)
Applied to files:
src/qs_codec/decode.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
src/qs_codec/decode.pytests/unit/decode_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/comparison/test_cases.json : When adding features, update cross-language comparison fixtures (test_cases.json) to maintain symmetry with Node qs
Applied to files:
tests/comparison/package.json
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {docs/**,README.md} : Document intentional divergences from npm qs in README and docs, and update docs when public behavior or options change
Applied to files:
tests/comparison/package.json
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
tests/unit/decode_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {src/qs_codec/enums/**/*.py,tests/unit/**/*.py} : For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Applied to files:
tests/unit/decode_test.py
🧬 Code graph analysis (3)
src/qs_codec/decode.py (1)
src/qs_codec/utils/utils.py (4)
OverflowDict(31-34)Utils(37-437)combine(346-383)is_overflow(341-343)
tests/unit/decode_test.py (2)
src/qs_codec/utils/utils.py (1)
OverflowDict(31-34)src/qs_codec/models/decode_options.py (1)
decode(267-281)
src/qs_codec/utils/utils.py (1)
src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)
🪛 Ruff (0.14.10)
src/qs_codec/utils/utils.py
352-352: Docstring contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (14)
tests/comparison/package.json (1)
8-8: Version bump aligns with security fix.The
qsdependency update to^6.14.1maintains parity with the upstream Node.js library's fix for the array limit DoS vulnerability (GHSA-6rw7-vpxm-498p). This ensures the Python port's behavior matches the reference implementation.tests/unit/decode_test.py (5)
13-13: LGTM!Import correctly added for the new
OverflowDicttype used in test assertions.
321-326: LGTM!Test expectation correctly updated to reflect that
list_limit=0now produces a dict-like structure with numeric string keys instead of a list.
578-594: LGTM!Test expectations correctly updated for
list_limit=0withstrict_null_handling. The dict output preservesNonevalues appropriately.
1335-1337: LGTM!Explicit test case for
list_limit=0behavior added, verifying conversion to dict with numeric string keys.
1689-1731: Good regression test coverage for the security vulnerability.The
TestCVE2024class provides comprehensive coverage:
test_dos_attack: Simulates the attack vector with 105 elements andlist_limit=100, verifying conversion toOverflowDicttest_array_limit_checks: Mirrors the JS patch tests, verifying edge cases at limits 0, 1, and 2test_no_limit_does_not_overflow: Confirms lists stay as lists when within the limitThe assertions correctly verify both the
dicttype and the specificOverflowDictsubclass. Based on learnings, these regression tests align with the requirement to add tests for limits.src/qs_codec/decode.py (4)
30-30: LGTM!Import correctly added for
OverflowDictalongside the existingUtilsimport.
291-291: LGTM!Correctly passes
optionstoUtils.combineto enable list_limit enforcement during duplicate key combination. This aligns with the coding guideline to honor Duplicates policy during merge.
363-371: LGTM!The overflow handling in
_parse_objectcorrectly:
- Detects already-overflowed leaves via
Utils.is_overflow()to avoid re-wrapping- Converts newly created lists to
OverflowDictwhen they exceedlist_limitPer coding guidelines, this enforces
list_limitexactly as specified.
395-399: LGTM!Correctly handles the case where
parse_lists=Falseand an empty bracket segment is encountered. If the leaf is already anOverflowDict, it's used directly; otherwise, the standard{"0": leaf}coercion applies.src/qs_codec/utils/utils.py (4)
31-35: LGTM!Clean implementation of
OverflowDictas a marker subclass. The docstring clearly describes its purpose.
151-154: LGTM!Correctly routes
OverflowDicttargets throughUtils.combinefor proper overflow-aware appending, maintaining the marker type through subsequent merge operations. This adheres to the coding guideline of adjusting merge logic withinUtils.merge.
183-187: LGTM!Correctly handles
OverflowDictsources by extracting values vialist(source.values())for iteration, ensuring overflow contents are properly merged.
340-343: LGTM!Simple and effective utility method for detecting
OverflowDictinstances.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/qs_codec/__init__.py (1)
30-50: Remove OverflowDict from__all__or update version and CHANGELOG.OverflowDict is currently exported in
__all__but is not part of the specified public API surface (per learnings: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined). Either remove OverflowDict from__all__since it's an internal utility, or if this public export is intentional, bump__version__from "1.3.2" and add a CHANGELOG entry documenting this new public symbol.
🤖 Fix all issues with AI agents
In @src/qs_codec/utils/utils.py:
- Around line 346-395: The combine() function mishandles OverflowDicts: (1) when
appending to an existing OverflowDict (Utils.is_overflow(a)), compute the next
numeric index by finding the maximum numeric key + 1 (parsing keys as ints and
handling sparse keys) rather than using len(a_copy); (2) when b is an
OverflowDict and a is not, iterate b.values() (or ordered items) and append each
element instead of treating b as a single element so items are merged/flattened
in order; keep the branch that handles b as list/tuple and the scalar case;
preserve ordering when merging two OverflowDicts by iterating keys in numeric
order; also replace non‑breaking hyphens in the combine() docstring with normal
hyphens to satisfy Ruff.
🧹 Nitpick comments (3)
tests/unit/utils_test.py (1)
632-671: Add regression coverage for sparse OverflowDict keys (realistic DoS vector).
Current tests only cover sequential"0".."n"keys. Please add at least one test likeOverflowDict({"999": "x"})and verifycombine()/merge()append at"1000"(not"1") and that ordering is deterministic. This will catch subtle index/ordering bugs early.
Based on learnings fortests/unit/**/*.pylimit regressions.src/qs_codec/utils/utils.py (2)
31-35: OverflowDict type is OK, but consider tightening the docstring/typing.
The one-line docstring is a bit long; optionally wrap it for readability / line-length consistency.
183-187: Dead/odd branch:isinstance(source, (list, tuple))can’t be true here.
This code path is under “sourceis a mapping”, so the list/tuple check is unreachable. Consider simplifying_iter2to only distinguishOverflowDictvs normal mapping (and be explicit about ordering expectations).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/qs_codec/__init__.pysrc/qs_codec/utils/utils.pytests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (6)
src/qs_codec/__init__.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Expose the public API only via all in qs_codec/init.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Files:
src/qs_codec/__init__.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/__init__.pytests/unit/utils_test.pysrc/qs_codec/utils/utils.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/utils_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/utils_test.py
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
src/qs_codec/__init__.pytests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/__init__.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
src/qs_codec/__init__.pytests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/__init__.pytests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/__init__.pytests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/__init__.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode_options.py,decode_options.py} : Extend option behavior by updating the corresponding dataclass with defaults and type hints (encode_options.py or decode_options.py)
Applied to files:
src/qs_codec/__init__.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {src/qs_codec/enums/**/*.py,tests/unit/**/*.py} : For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/utils_test.py
🧬 Code graph analysis (2)
src/qs_codec/__init__.py (1)
src/qs_codec/utils/utils.py (1)
OverflowDict(31-34)
src/qs_codec/utils/utils.py (1)
src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)
🪛 Ruff (0.14.10)
src/qs_codec/utils/utils.py
352-352: Docstring contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
361-361: Docstring contains ambiguous ‑ (NON-BREAKING HYPHEN). Did you mean - (HYPHEN-MINUS)?
(RUF002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
tests/unit/utils_test.py (2)
3-14: Import updates look fine and keep tests focused.
patch+OverflowDictimports are appropriate for the new delegation/overflow assertions.
896-904: This test is correct and the behavior is intentional. When merging a mapping (including OverflowDict) into a list, the merge function coerces the list to an index-keyed dict and then overlays the source mapping. The docstring at lines 65-68 explicitly states this rule: "iftargetis a sequence, coerce it to an index‑keyed dict and merge." The spread operator implementation ({...__coerced_target__, **source}at lines 170-173) correctly places source last, allowing it to overwrite colliding keys. This behavior is qs-compatible for query string parsing where array indices naturally become dict keys during structural reconstruction.src/qs_codec/utils/utils.py (2)
151-154: Good: merge delegates overflow-target handling to combine (centralizes logic).
This keeps overflow merge behavior in one place, aligning with the “don’t inline merge logic elsewhere” guideline.
340-344:Utils.is_overflow()looks good and keeps the type-check centralized.
techouse
changed the title
DecodeOptions. list_limit handling in combine function to prevent DoS via memory exhaustion
techouse
changed the title
DecodeOptions. list_limit handling in combine function to prevent DoS via memory exhaustionDecodeOptions.list_limit handling in combine function to prevent DoS via memory exhaustion
techouse
changed the title
DecodeOptions.list_limit handling in combine function to prevent DoS via memory exhaustionDecodeOptions.list_limit handling in Utils.combine function to prevent DoS via memory exhaustion
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @tests/unit/utils_test.py:
- Line 3: The file imports unittest.mock.patch but it is unused (Flake8 F401);
remove the unused import from tests/unit/utils_test.py by deleting the "patch"
import entry (i.e., remove or replace the "from unittest.mock import patch"
line) and run the tests/linter to confirm no remaining references to the patch
symbol.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/qs_codec/utils/utils.pytests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (5)
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/utils_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/utils_test.py
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {src/qs_codec/enums/**/*.py,tests/unit/**/*.py} : For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/utils/utils.py
🧬 Code graph analysis (1)
src/qs_codec/utils/utils.py (1)
src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)
🪛 Flake8 (7.3.0)
tests/unit/utils_test.py
[error] 3-3: 'unittest.mock.patch' imported but unused
(F401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.9, pypy-3.9)
🔇 Additional comments (6)
tests/unit/utils_test.py (3)
13-13:OverflowDictimport in tests is fine.
632-672: Good coverage for list_limit overflow + OverflowDict immutability/append semantics.One gap worth adding: a regression asserting
Undefined()elements are skipped when combining/merging involvingOverflowDict(otherwise sentinels can leak into intermediate structures). Based on learnings, this is especially relevant for limits/merge behavior intests/unit/**/*.py.
881-956: Nice edge-case coverage (sparse keys, numeric ordering, merge delegation).Two small nits:
- Comments referencing specific
utils.pyline numbers (e.g., “lines 403-404”) will go stale quickly; consider describing behavior instead.- Consider adding one test for non-numeric keys in an
OverflowDictto lock in desired behavior (either robust fallback or explicit failure), sinceOverflowDictis a public type at module scope.src/qs_codec/utils/utils.py (3)
31-35:OverflowDictmarker type is a clean way to distinguish overflowed lists.
183-188: Numeric-key-order iteration for OverflowDict source preserves list semantics.
341-345:Utils.is_overflow()helper is straightforward and keeps call sites readable.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/utils_test.py (1)
631-997: Consider adding edge case tests forlist_limit=0and negative limits.The new test coverage is comprehensive for the core OverflowDict functionality. Based on the PR objectives mentioning handling of
list_limit=0, consider adding explicit tests here if not already covered in theTestCVE2024class:def test_combine_list_limit_zero_creates_overflow_dict(self) -> None: """When list_limit=0, any non-empty result should overflow.""" options = DecodeOptions(list_limit=0) combined = Utils.combine(["a"], [], options) assert isinstance(combined, OverflowDict) assert combined == {"0": "a"}As per coding guidelines, regression tests for limits help ensure robustness against DoS vectors.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/qs_codec/utils/utils.pytests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (5)
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/utils_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/utils_test.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {src/qs_codec/enums/**/*.py,tests/unit/**/*.py} : For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
tests/unit/utils_test.py
🧬 Code graph analysis (2)
src/qs_codec/utils/utils.py (2)
src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)src/qs_codec/models/undefined.py (1)
Undefined(17-69)
tests/unit/utils_test.py (2)
src/qs_codec/utils/utils.py (4)
OverflowDict(31-34)Utils(37-491)combine(361-437)merge(46-206)src/qs_codec/models/undefined.py (1)
Undefined(17-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (13)
tests/unit/utils_test.py (8)
12-12: LGTM!The import correctly brings in
OverflowDictalongsideUtilsfrom the utils module.
631-639: LGTM!Good boundary test validating that exceeding the default list_limit (20) triggers OverflowDict creation, with correct assertions on type, length, and indexed values.
641-653: LGTM!Excellent test validating immutability of the original OverflowDict during combine operations, with comprehensive assertions on copy semantics and correct index assignment.
655-670: LGTM!Good coverage of default options behavior and OverflowDict-to-OverflowDict combination with correct key assignment semantics.
880-934: LGTM!Comprehensive tests for merge with OverflowDict covering:
- Target as OverflowDict delegating to combine
- Source as OverflowDict into regular dict/list
- Sparse key handling with numeric sorting verification
The reverse-order key insertion on lines 929-930 is a smart test pattern for verifying numeric sorting.
904-954: LGTM!Excellent edge case coverage for sparse OverflowDict handling. The test at line 911 correctly validates the max-key-plus-one behavior (
"1000"for sparse key"999"), ensuring the implementation doesn't incorrectly uselen()for index calculation.
956-983: LGTM!Thorough coverage of Undefined handling in combine operations. The test at lines 975-983 is particularly valuable, verifying that:
- Non-numeric keys (
"skip") are preserved but ignored for index calculation- Existing Undefined values are filtered out during copy
- New values are correctly assigned to the next numeric index
As per coding guidelines, these tests help ensure limits are enforced correctly.
985-997: LGTM!Good tests verifying that non-numeric keys in OverflowDict sources are correctly excluded during combine and merge operations, with numeric keys processed in sorted order.
src/qs_codec/utils/utils.py (5)
31-35: LGTM!Clean marker class design using dict subclass. This is an appropriate pattern for tagging overflow-converted structures without adding behavioral overhead.
152-154: LGTM!Correct delegation to
combinewhen target is an OverflowDict, maintaining overflow semantics and adhering to the guideline of keeping merge logic centralized.
183-194: LGTM!Correct handling of OverflowDict sources with numeric key ordering to preserve list semantics during merge. The implementation properly filters non-numeric keys and skips Undefined values.
343-358: LGTM!Clean helper implementations. The
_numeric_key_pairsmethod correctly handles edge cases with try/except for non-coercible keys.One minor note: negative numeric keys (e.g.,
"-1") would be accepted and sorted before positive indices. This is likely fine since QS inputs shouldn't produce negative indices, but worth being aware of if this assumption changes.
360-437: LGTM!Well-structured rewrite of
combinewith comprehensive overflow handling:
- Correct max-key-plus-one indexing for sparse OverflowDicts (line 390)
- Proper immutability via copy-on-write (lines 387-388)
- Consistent Undefined filtering across all code paths
- Clear docstring documenting the behavior including edge cases (negative limits)
The logic correctly allows exactly
list_limititems (using>comparison at line 433), converting only when exceeded.
… OverflowDict creation
…S via memory exhaustion
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/unit/decode_test.py (1)
322-336: Consider asserting OverflowDict type for list_limit=0 cases.The test expectations verify the structure but do not assert that
result["a"]is anOverflowDictinstance. For consistency with the CVE2024 tests (lines 1713-1714) that explicitly checkisinstance(result["a"], OverflowDict), consider adding type assertions here.♻️ Proposed enhancement
pytest.param( "a[]=b&a=c", DecodeOptions(list_limit=0), {"a": {"0": "b", "1": "c"}}, id="explicit-list-with-zero-limit", ), + # ... in the test method body: + assert isinstance(result["a"], OverflowDict), "Expected OverflowDict when list_limit=0"src/qs_codec/utils/utils.py (1)
178-190: Consider simplifying the overflow value extraction.The current implementation extracts
overflow_valuesseparately (line 181) and then zips it withsorted_pairs(line 187). This can be simplified by directly iterating over sorted pairs and accessing values.♻️ Proposed simplification
if Utils.is_overflow(source): source_of = t.cast(OverflowDict, source) sorted_pairs = sorted(Utils._numeric_key_pairs(source_of), key=lambda item: item[0]) - overflow_values = [source_of[k] for _, k in sorted_pairs] result = OverflowDict() offset = 0 if not isinstance(target, Undefined): result["0"] = target offset = 1 - for (numeric_key, _), val in zip(sorted_pairs, overflow_values): + for numeric_key, original_key in sorted_pairs: + val = source_of[original_key] if not isinstance(val, Undefined): result[str(numeric_key + offset)] = val return result
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/qs_codec/utils/utils.pytests/unit/decode_test.pytests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (5)
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/utils/utils.pytests/unit/decode_test.pytests/unit/utils_test.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/decode_test.pytests/unit/utils_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/decode_test.pytests/unit/utils_test.py
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
src/qs_codec/utils/utils.pytests/unit/decode_test.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
tests/unit/decode_test.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
tests/unit/decode_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/decode_test.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {src/qs_codec/enums/**/*.py,tests/unit/**/*.py} : For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Applied to files:
tests/unit/utils_test.py
🧬 Code graph analysis (2)
src/qs_codec/utils/utils.py (2)
src/qs_codec/models/undefined.py (1)
Undefined(17-69)src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)
tests/unit/decode_test.py (3)
src/qs_codec/utils/utils.py (1)
OverflowDict(31-34)src/qs_codec/models/decode_options.py (2)
DecodeOptions(20-293)decode(267-281)src/qs_codec/decode.py (1)
decode(33-103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (7)
tests/unit/decode_test.py (1)
1694-1735: LGTM! Comprehensive DoS protection tests.The CVE2024 test class effectively validates the overflow behavior:
- Verifies that exceeding list_limit produces OverflowDict with correct numeric string keys
- Tests boundary conditions (0, 1, 2, -1 limits)
- Confirms that staying within limits preserves list semantics
The logic correctly implements
len(res) > list_limitfor overflow detection.src/qs_codec/utils/utils.py (4)
31-34: LGTM! Clean marker class for overflow detection.The
OverflowDictsubclass provides a simple, type-safe way to distinguish overflowed containers from regular dicts. The minimal implementation (no custom methods) is appropriate for a marker class.
152-153: LGTM! Correct delegation to combine for overflow targets.When the target is an
OverflowDictand source is a non-mapping, delegating tocombineensures consistent overflow handling and proper index calculation.
350-365: LGTM! Clean utility methods for overflow handling.Both helpers are well-designed:
is_overflowprovides a clear type-checking interface_numeric_key_pairsrobustly extracts numeric keys while gracefully handling non-numeric keysThe error handling in
_numeric_key_pairscorrectly catches bothTypeError(for non-stringifiable keys) andValueError(for non-numeric strings).
390-444: LGTM! Robust overflow handling with comprehensive edge case coverage.The
combineimplementation correctly handles:
- Overflow appending to existing
OverflowDictusing max numeric key + 1- Sorting numeric keys when merging overflow dicts
- Preserving non-numeric keys in overflow dicts
- Filtering
Undefinedvalues throughout- Converting to
OverflowDictwhenlen(res) > list_limit(handles 0, negative, and positive limits correctly)The logic for negative
list_limitcorrectly overflows immediately since any non-empty result will havelen(res) > -1.tests/unit/utils_test.py (2)
631-683: LGTM! Comprehensive overflow behavior tests.The test suite thoroughly covers:
- Default limit (20) boundary conditions
- Zero and negative limits
- OverflowDict immutability during combine operations
- Combining OverflowDict instances with each other and with scalars/lists
All test expectations align correctly with the implementation logic.
892-1012: LGTM! Excellent test coverage for merge/combine with OverflowDict.The tests comprehensively verify:
- Merging with OverflowDict as target or source
- Sparse OverflowDict handling (numeric key ordering, max key + 1 insertion)
- Non-numeric key preservation and exclusion from index calculations
- Undefined filtering in all code paths
- Immutability of input OverflowDicts
The sparse dict test (line 935-947) correctly validates that numeric keys are sorted and offset is applied, producing
{"0": "a", "3": "b", "11": "c"}from target "a" and source with keys "2" and "10".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/qs_codec/utils/utils.py:
- Around line 209-221: The merge logic in Utils.merge uses the raw key
membership check but writes into the merged dict with str(key), causing missed
recursive merges when merge_target has non-string keys; update the membership
test to check for str(key) in merge_target (or normalize keys consistently) so
the conditional invokes Utils.merge for the same normalized key used when
assigning, ensuring recursive merges occur for existing entries in merge_target
(refer to Utils.merge, merge_target, and the comprehension producing merged).
🧹 Nitpick comments (4)
tests/unit/utils_test.py (1)
631-683: Avoid hard-coding the default list_limit in tests.These tests assume the default
list_limitis20(e.g., “default limit is 20”, “Default options should imply list_limit=20”). Consider deriving it fromDecodeOptions()(or passingoptions=DecodeOptions()) so tests won’t break if defaults change for compatibility/security reasons.src/qs_codec/utils/utils.py (3)
31-35: OverflowDict docstring is fine; consider clarifying mutability expectations.Tests appear to rely on “copy-on-write” (combine returns a copy) rather than true immutability; consider adjusting wording/comments elsewhere to avoid implying OverflowDict itself is immutable.
178-195: Simplify scalar-target + OverflowDict-source merging and avoid double iteration.Current logic builds
overflow_valuesseparately and zips it back withsorted_pairs; you can iteratesorted_pairsonce and readsource_of[k]directly. Minor, but reduces bookkeeping in a hot path.
357-373: Helpers look fine; watch out for duplicate numeric coercions.
int("01")andint("1")both coerce to1; if both keys exist, later writes tostr(numeric_key + offset)can overwrite earlier ones. If that’s acceptable (last-wins), it’d be worth documenting.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/qs_codec/utils/utils.pytests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (5)
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/utils_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/utils_test.py
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
src/qs_codec/utils/utils.pytests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
tests/unit/utils_test.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
tests/unit/utils_test.py (2)
12-12: Import path may bypass intended public API surface.If
OverflowDictis meant to be part of the public API, consider importing it fromqs_codec(package root) rather thanqs_codec.utils.utilsto ensure tests reflect the supported surface area. Based on learnings, ensure__all__inqs_codec/__init__.pyremains the single source of exported API.
892-1019: Nice coverage of overflow merge/combine edge cases.Good coverage for: sparse numeric keys, skipping
Undefined, preservation vs skipping of non-numeric keys (merge preserves, combine skips), and copy-on-write expectations when appending to an OverflowDict.src/qs_codec/utils/utils.py (2)
151-154: OverflowDict target routing in merge() looks correct.Early return to
Utils.combine(target, source, options)for overflow targets is the right way to keep list semantics without re-materializing Python lists. Based on learnings, this keeps list/index logic centralized in Utils.*.
374-451: combine(): fix docstring claim about negative list_limit and update stale comment.
Doc claims negative
list_limitoverflows "non-empty" results, but the codelen(res) > -1also triggers for empty lists—any combined result including empty ([]) gets converted. Add a test case for combining two empty lists with negative limit to confirm intended behavior.Line 399 comment states "len(a_copy) gives the next index," but the implementation uses
max(key)+1to safely handle sparse dicts. Update the comment to match the actual logic.Regarding
raise_on_limit_exceeded: the docstring correctly states this helper never raises and intentionally ignores the flag, converting toOverflowDictinstead. This is consistent with the library's documented behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/qs_codec/utils/utils.py (2)
178-195: Complex but correct overflow merging logic.The offset-based merging correctly handles the case where a scalar/sequence target is combined with an
OverflowDictsource. The logic:
- Extracts and sorts numeric keys from the source
- Reserves index "0" for the target (if not
Undefined)- Shifts source numeric indices by the offset
- Preserves non-numeric keys unchanged
This prevents index collisions and maintains list semantics.
However, consider adding an inline comment at line 189 explaining why the offset is calculated, as this logic is non-trivial:
# Offset ensures target occupies index "0"; source indices shift up by 1
209-224: Overflow status correctly preserved through merge.Tracking
is_overflow_targetand wrapping the result inOverflowDictwhen appropriate ensures that overflow protection persists through recursive merges, maintaining DoS prevention guarantees.Minor performance note: Line 210's
copy.deepcopy(target)could be expensive for large nested structures. Since the code immediately overwrites values during merge, consider whether a shallow copy with selective deep-copying of mutable values would suffice—though this optimization may be premature without profiling.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/qs_codec/utils/utils.py
🧰 Additional context used
📓 Path-based instructions (3)
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/utils/utils.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/utils/utils.py
🧬 Code graph analysis (1)
src/qs_codec/utils/utils.py (2)
src/qs_codec/models/undefined.py (1)
Undefined(17-69)src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
src/qs_codec/utils/utils.py (5)
31-34: LGTM - marker class design is appropriate.The
OverflowDictmarker pattern is clean and allows the library to distinguish overflow-converted dicts from normal dicts without enforcing immutability at the class level. This provides flexibility while the library maintains immutability guarantees through careful handling.
152-153: Overflow routing preserves list_limit semantics.Correctly delegates to
Utils.combinewhen the target is anOverflowDict, ensuring consistent overflow handling and list_limit enforcement.
361-376: Clean helper utilities for overflow detection and key extraction.Both
is_overflowand_numeric_key_pairsare well-implemented:
is_overflowprovides a clear public API for overflow detection_numeric_key_pairssafely extracts numeric keys with proper exception handling
379-427: Robust overflow handling whenaisOverflowDict.The logic correctly:
- Preserves the
OverflowDicttype whenais already overflowed- Calculates the next index using
max(numeric_keys) + 1to handle sparse dicts safely- Handles different types of
b(list, tuple,OverflowDict, scalar) with appropriate flattening- Filters
Undefinedsentinels throughoutThe use of
max(key for key, _ in key_pairs) + 1(line 408) correctly handles sparse overflow dicts like{"0": "a", "5": "b"}by appending at index 6, preserving the sparse structure intentionally created by overflow conversion.
428-455: List flattening and overflow conversion work correctly.The normal combination path properly:
- Flattens lists/tuples and scalars while filtering
Undefined- Handles
OverflowDictasbby extracting values in numeric key order (lines 438-444), preserving list semantics- Applies the
list_limitcheck with sensible defaults (20 whenoptionsisNone)- Converts to
OverflowDictwhen the limit is exceededThe condition
len(res) > list_limitcorrectly implements the overflow semantics described in the docstring, including edge cases like negative limits causing immediate overflow.Docstring accuracy note: The docstring (line 396-399) states that
combine"never raises an exception when the limit is exceeded" even ifraise_on_limit_exceededisTrue. This is accurate for the current implementation—overflow is handled silently by converting toOverflowDict. However, callers indecode.pymay need to check the return type and raise if configured to do so. Ensure the calling code properly handles this case.Based on coding guidelines, as per learnings: merge and combine should not inline limit enforcement logic elsewhere, which this implementation respects.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/qs_codec/utils/utils.py:
- Around line 210-225: The merge implementation in Utils.merge normalizes keys
with str(key) (variable normalized_key) which can collapse distinct non-string
keys (e.g., 1 vs "1"); update Utils.merge to either (a) enforce/validate that
callers only pass string keys (add a precondition check raising or converting
whole input and document it) or (b) avoid lossy normalization by keeping
original key types when merge_target contains that exact key (compare both key
and str(key) and prefer exact-type match), and update any usage of merge_target
and OverflowDict handling accordingly so behavior is deterministic and
documented.
- Around line 31-35: The review notes that OverflowDict is exported in __all__
despite the project's documented export allowlist; either remove OverflowDict
from the module export list in src/qs_codec/__init__.py (delete it from the
__all__ sequence) to keep OverflowDict internal, or explicitly add OverflowDict
to the documented export policy/allowlist so exports and docs match; locate the
OverflowDict symbol in src/qs_codec/utils/utils.py and the __all__ definition in
src/qs_codec/__init__.py and make the change consistently (remove OverflowDict
from __all__ or update the allowlist to include OverflowDict).
🧹 Nitpick comments (5)
tests/unit/utils_test.py (2)
12-13: Prefer importingOverflowDictfrom the public API (if it’s intended to be public).If
OverflowDictis now part of the supported surface, consider importing it fromqs_codec(package root) instead ofqs_codec.utils.utilsto ensure tests track the public contract.
631-683: Nice coverage forlist_limitoverflow inUtils.combine.These cases (default limit, zero, negative, options=None) are well-targeted and validate the core DoS mitigation behavior. Consider adding 1–2 small boundaries:
len(res) == list_limitshould stay a list (no overflow).list_limit < 0with an empty result (do you want[]orOverflowDict({})?).src/qs_codec/utils/utils.py (3)
31-35:OverflowDictas a marker type is fine; consider preserving the marker on.copy().Since this is a semantic marker, overriding
copy()/__copy__to returnOverflowDict(notdict) can prevent accidental “loss of overflow-ness” if callers copy it.
362-382: Helpers are clear;_numeric_key_pairsprobably shouldn’t be “public” if it’s internal-only.Even though it’s a
@staticmethod, the leading underscore suggests internal use; if it’s not meant for consumers, consider moving it to module scope or dropping it from any public re-exports.
383-461: Fixcombine()default/negativelist_limitbehavior to match DecodeOptions + docstring.
- Default list_limit is hard-coded to
20; safer to derive fromDecodeOptions()so defaults can’t drift.- Docstring says negative limit overflows “non-empty” results, but current code also overflows empty (
len([]) > -1).Proposed diff
@@ - list_limit = options.list_limit if options else 20 - if len(res) > list_limit: + list_limit = options.list_limit if options is not None else DecodeOptions().list_limit + if list_limit < 0: + return OverflowDict({str(i): x for i, x in enumerate(res)}) if res else [] + if len(res) > list_limit: # Convert to OverflowDict return OverflowDict({str(i): x for i, x in enumerate(res)})
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/qs_codec/utils/utils.pytests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (5)
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/utils_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/utils_test.py
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
tests/unit/utils_test.pysrc/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to {src/qs_codec/enums/**/*.py,tests/unit/**/*.py} : For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
tests/unit/utils_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/utils/utils.py
🧬 Code graph analysis (1)
src/qs_codec/utils/utils.py (2)
src/qs_codec/models/undefined.py (1)
Undefined(17-69)src/qs_codec/models/decode_options.py (1)
DecodeOptions(20-293)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
tests/unit/utils_test.py (1)
892-1019: OverflowDict merge/combine semantics are well exercised (sparse keys, non-numeric keys, Undefined skipping, copy-on-write).The tests around sparse numeric keys and non-numeric key handling make the intended semantics very clear and should help prevent regressions.
src/qs_codec/utils/utils.py (2)
151-155: Good:merge()delegates OverflowDict targets tocombine()for append semantics.This keeps overflow/list behavior centralized and avoids re-implementing list growth rules in multiple places. Based on learnings, this is the right direction.
178-197: OverflowDict source merge: offset + numeric ordering logic looks consistent with intended semantics.One thing to double-check is the “duplicate numeric key after int-coercion” overwrite behavior (e.g.,
"01"vs"1"): your helper doc notes it, but ensure this matches the desired qs parity.
…serve overflow markers
…ter accessibility
…y and prevent overflow with empty results
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/qs_codec/models/overflow_dict.py (1)
9-15: Consider adding__deepcopy__()to preserve the overflow marker during deep copies.The
Utils.mergemethod (line 223 in utils.py) usescopy.deepcopy(target), which will not preserve theOverflowDicttype when deep copying since__deepcopy__()is not implemented. This could cause overflow markers to be lost during recursive merges.♻️ Suggested implementation
+ def __deepcopy__(self, memo: t.Dict[int, t.Any]) -> "OverflowDict": + """Return an OverflowDict deep copy to preserve the overflow marker.""" + import copy + return OverflowDict({k: copy.deepcopy(v, memo) for k, v in self.items()})Note: You'll need to add
import typing as tat the top of the file for the type hint.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/qs_codec/decode.pysrc/qs_codec/models/overflow_dict.pysrc/qs_codec/utils/utils.pytests/unit/decode_test.pytests/unit/utils_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/qs_codec/decode.py
- tests/unit/utils_test.py
🧰 Additional context used
📓 Path-based instructions (5)
{src,tests}/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,tests}/**/*.py: Keep docstrings descriptive and parity-focused; match the existing module docstring style
Maintain 120-character line length (Black) and isort import ordering (black profile)
Files:
src/qs_codec/models/overflow_dict.pysrc/qs_codec/utils/utils.pytests/unit/decode_test.py
src/qs_codec/{decode.py,utils/utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Files:
src/qs_codec/utils/utils.py
src/qs_codec/utils/{utils.py,decode_utils.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Files:
src/qs_codec/utils/utils.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Files:
tests/unit/decode_test.py
{src/qs_codec/enums/**/*.py,tests/unit/**/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For new list or formatting strategies, add an Enum member with associated generator/formatter and tests to cover serialization/deserialization round-trip
Files:
tests/unit/decode_test.py
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/utils/{utils.py,decode_utils.py} : Do not inline merge logic elsewhere; when altering merge or list/index logic, adjust Utils.merge or decoding helpers instead
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{decode.py,utils/utils.py} : Honor Duplicates policy during merge: COMBINE accumulates lists; FIRST/LAST semantics are enforced
Applied to files:
src/qs_codec/utils/utils.pytests/unit/decode_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Preserve public function signatures; only add kwargs when necessary and maintain backward compatibility
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/__init__.py : Expose the public API only via __all__ in qs_codec/__init__.py, re-exporting: encode, decode, dumps, loads, Charset, Format, ListFormat, Duplicates, Sentinel, DecodeKind, EncodeOptions, DecodeOptions, Undefined
Applied to files:
src/qs_codec/utils/utils.pytests/unit/decode_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to tests/unit/**/*.py : Add regression tests for limits (depth, list_limit, parameter_limit), duplicate policies, charset sentinel interactions, dot-notation toggles, and comma round-trip
Applied to files:
src/qs_codec/utils/utils.pytests/unit/decode_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Do not mutate caller inputs; copy/normalize inputs before traversal (deepcopy for mappings; index-projection for sequences)
Applied to files:
src/qs_codec/utils/utils.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Enforce depth, list_limit, parameter_limit, strict_depth, and raise_on_limit_exceeded exactly as specified
Applied to files:
src/qs_codec/utils/utils.pytests/unit/decode_test.py
📚 Learning: 2025-10-09T07:54:31.752Z
Learnt from: CR
Repo: techouse/qs_codec PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-09T07:54:31.752Z
Learning: Applies to src/qs_codec/{encode.py,decode.py} : Validate inputs/options and raise ValueError (or IndexError for strict depth) with messages matching tests where asserted
Applied to files:
tests/unit/decode_test.py
🧬 Code graph analysis (1)
tests/unit/decode_test.py (3)
src/qs_codec/models/overflow_dict.py (1)
OverflowDict(6-15)src/qs_codec/decode.py (1)
decode(34-104)src/qs_codec/utils/decode_utils.py (1)
decode(163-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (pypy3.10, pypy-3.10)
- GitHub Check: Test (pypy3.8, pypy-3.8)
- GitHub Check: Test (pypy3.11, pypy-3.11)
- GitHub Check: Test (pypy3.9, pypy-3.9)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (10)
tests/unit/decode_test.py (2)
1694-1735: LGTM! Comprehensive CVE-2024 DoS regression tests.The test coverage validates:
- DoS attack scenario with 105 items exceeding limit of 100 produces OverflowDict
- Edge cases: list_limit values of 0, 1, and 2
- Boundary condition: exactly at limit (2 items, limit=2) returns list, not OverflowDict
The assertions correctly verify both the type (
isinstance(result["a"], OverflowDict)) and the structure (numeric string keys).Based on learnings: These tests align with the requirement to add regression tests for limits.
322-326: Test expectations correctly updated for zero list_limit overflow behavior.The updated assertions expect
{"a": {"0": "...", "1": "..."}}format whenlist_limit=0, which aligns with the overflow protection mechanism. The dict representation with numeric string keys prevents unbounded memory growth while preserving the data.Based on learnings: These test updates correctly validate the list_limit enforcement as specified in coding guidelines.
Also applies to: 332-336, 584-584, 596-596, 1338-1340
src/qs_codec/utils/utils.py (8)
32-46: LGTM! Helper function correctly handles numeric key extraction.The try/except block properly handles non-numeric keys, and the docstring appropriately warns about potential key collisions when distinct strings like
"01"and"1"both coerce to the same integer.
377-380: LGTM! Clean abstraction for overflow detection.
189-208: LGTM! Overflow source merging correctly applies index offset.The offset calculation at lines 195-197 ensures that when a non-Undefined target is prepended to an overflow source:
- Target occupies index
"0"- Source indices are shifted:
source[k]→result[str(k + 1)]Non-numeric keys from the source are preserved as-is (lines 203-207), which maintains mixed key semantics.
Based on learnings: Honors merge semantics as specified in coding guidelines.
227-240: LGTM! Key merging logic correctly prioritizes exact matches.The three-tier matching strategy (lines 228-235) prevents unintended key collisions:
- Exact key match (preserves type:
0vs"0")- String-normalized match (handles cross-type lookups)
- New key (adds to merged result)
Line 240 correctly preserves the OverflowDict type when the target was an overflow dict.
404-429: LGTM! OverflowDict-first combination correctly maintains numeric ordering.When
ais an OverflowDict:
- Line 408: Creates a clean copy without Undefined values
- Line 411: Safely computes next index using
max + 1or0if empty- Lines 413-428: Appends items from
b(handling list, tuple, OverflowDict, and scalar cases)The numeric key ordering is preserved via
sorted(_numeric_key_pairs(...))at line 421, ensuring list semantics are maintained.
431-460: Review the normal combination path with overflow protection.The overflow conversion logic is sound:
- Line 454: Negative
list_limit→ immediate OverflowDict conversion- Line 456:
len(res) > list_limit→ conversion (note: uses>not>=, so at-limit stays a list)- Lines 441-447: OverflowDict source is flattened to a list in numeric-key order
The Undefined filtering throughout (lines 434, 440, 446, 449) ensures sentinels are never included in the combined result.
Based on learnings: Correctly enforces list_limit as specified in coding guidelines.
454-455: No action needed—the behavior is documented and intentional.The code correctly returns an empty list when
list_limit < 0and the result is empty. This behavior is explicitly documented in the docstring (lines 397–398: "any non-empty combined result will be converted toOverflowDict") and verified by the testtest_combine_negative_list_limit_with_empty_result_stays_list, which asserts thatUtils.combine([], [], DecodeOptions(list_limit=-1))returns an empty list. The conditional patternif res else resmatches the documented design.
163-164: Delegation tocombine()when target is OverflowDict is correct.The
combine()function has explicit logic to handle OverflowDict as the first argument, creating a copy and appending subsequent values at sequential numeric indices while filtering Undefined values. This properly preserves overflow semantics during merge operations and follows the architecture principle of centralizing merge logic in utility helpers.
techouse
deleted the
fix/implement-list-limit-handling-in-combine-function-to-prevent-DoS-via-memory-exhaustion
branch
This pull request introduces a new mechanism to prevent denial-of-service (DoS) attacks caused by excessive list growth during query string decoding. It does this by converting lists that exceed a configurable
list_limitinto a specialOverflowDictobject, which uses numeric keys instead of a Python list. This approach ensures memory safety and aligns behavior with the latestqsJavaScript library. The changes also include comprehensive updates to the decoding logic, utility functions, and tests to support and verify this overflow handling.Overflow handling and DoS protection:
OverflowDictclass inutils/utils.pyto represent lists that exceed the configuredlist_limit, preventing unbounded memory usage during query string decoding.Utils.combinemethod to convert combined lists into anOverflowDictwhen the length exceeds the limit, and added logic to handle merging and appending toOverflowDictinstances.decode.pyto useOverflowDictwhen limits are exceeded, and to propagate overflow objects through recursive parsing and merging. [1] [2] [3] [4] [5]API and utility updates:
OverflowDictto the public API in__init__.pyand updated all relevant imports and usages throughout the codebase. [1] [2] [3] [4] [5]Testing and security validation:
Dependency update:
qsdependency in the test comparison suite to version 6.14.1 to ensure parity with the latest security and behavior fixes.Relates to GHSA-6rw7-vpxm-498p
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.