🐛 implement `DecodeOptions.ListLimit` handling to prevent DoS via memory exhaustion #44

techouse · 2026-01-11T23:16:53Z

This pull request introduces significant changes to how decoded lists and arrays are represented and handled when certain limits are reached, as well as updates to the test suite and dependency versions. The main focus is on converting lists to dictionaries (maps) when a specified ListLimit is exceeded, and on tracking this "overflow" state internally. The changes are thoroughly tested with new and updated unit tests.

Key changes include:

Core logic and data structure changes

Updated the logic in Decoder.cs and Utils.cs so that when a decoded list exceeds the configured ListLimit, it is converted from a List<object?> to a Dictionary<object, object?> (map), and an internal overflow state is tracked using a ConditionalWeakTable. This ensures consistent handling of oversized lists and allows for correct merging and combining of overflowed collections. [1] [2] [3] [4] [5]

Test suite updates

Updated numerous test cases in QsNet.Tests/DecodeTests.cs to expect dictionaries (maps) instead of lists when the ListLimit is exceeded or set to zero, ensuring the tests reflect the new overflow behavior. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]
Added comprehensive new tests for overflow and limit behavior, including boundary conditions, implicit array limits, duplicate value handling, and correct merging/combining of overflowed collections. This ensures robust coverage of all edge cases related to list size limits. [1] [2]

Dependency update

Updated the qs JavaScript library dependency version from ^6.14.0 to ^6.14.1 in package.json.

These changes improve the reliability and predictability of list handling in the decoder, especially in edge cases involving large or complex query strings.

Relates to GHSA-6rw7-vpxm-498p

Summary by CodeRabbit

Improvements
- Query string decoder now features refined list overflow handling with automatic conversion to indexed dictionaries when configured limits are exceeded, ensuring more predictable behavior in constraint scenarios.
Chores
- Updated parser dependency.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-11T23:16:58Z

📝 Walkthrough

Walkthrough

This PR introduces overflow tracking for list limits in query string decoding. When values exceed ListLimit, lists are converted to index-keyed dictionaries with overflow state metadata. The decoder integrates new limit-aware merging logic, and test expectations are updated to reflect dictionary-based representations under constraint conditions.

Changes

Cohort / File(s)	Summary
Dependency Update `QsNet.Comparison/js/package.json`	Bumps "qs" dependency from ^6.14.0 to ^6.14.1
Core Overflow Logic `QsNet/Internal/Utils.cs`	Introduces internal OverflowState tracking, OverflowTable per-object storage, and helper methods (IsOverflow, GetOverflowMaxIndex, SetOverflowMaxIndex, MarkOverflow, TryGetArrayIndex, GetMaxIndexFromMap). Extends Merge to honor overflow state when targets are dictionaries. Adds CombineWithLimit function that converts lists exceeding ListLimit to index-mapped dictionaries. Implements ListToIndexMap for list-to-dictionary conversion with numeric string keys
Decoder Integration `QsNet/Internal/Decoder.cs`	Replaces Utils.Combine calls with Utils.CombineWithLimit to apply list-length constraints. Adds overflow guards in list handling to propagate overflow state without further wrapping
Test Updates: Decode Behavior `QsNet.Tests/DecodeTests.cs`	Updates test expectations where lists previously asserted as List\<object?\> now expect Dictionary\<string, object?\> with numeric string keys when ListLimit is zero. Adds new tests for ListLimit boundary conditions and implicit array-to-map conversions
Test Coverage: Utils Utilities `QsNet.Tests/UtilsTests.cs`	Adds extensive unit tests for Utils.CombineWithLimit and Utils.Merge overflow/limit handling, covering over-limit scenarios, zero ListLimit conversions, overflow indexing, and overflow-aware merging with primitives and mixed keys

Sequence Diagram(s)

sequenceDiagram
    participant Decoder
    participant CombineWithLimit
    participant Merge
    participant OverflowTable
    
    Decoder->>CombineWithLimit: CombineWithLimit(existingVal, value, options)
    activate CombineWithLimit
    CombineWithLimit->>CombineWithLimit: Check if combined size exceeds ListLimit
    alt Within Limit
        CombineWithLimit->>Merge: Merge values normally
    else Exceeds Limit
        CombineWithLimit->>CombineWithLimit: ListToIndexMap(result to Dict)
        CombineWithLimit->>OverflowTable: MarkOverflow(dict, maxIndex)
    end
    CombineWithLimit-->>Decoder: Return merged value (List or Dict with overflow)
    deactivate CombineWithLimit
    
    Decoder->>OverflowTable: Check IsOverflow(leaf)
    alt Overflow Detected
        Decoder-->>Decoder: Propagate leaf as-is
    else No Overflow
        Decoder->>CombineWithLimit: CombineWithLimit([], leaf, options)
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚸 change Qs.Decode return from Dictionary<object, object?> to Dictionary<string, object?> #2: Modifies how lists/maps are represented with numeric index conversions to string keys and list-to-index-map conversions, directly related to the overflow tracking mechanism introduced here.
⚡ optimize ParseQueryStringValues #13: Changes ListLimit-related merge logic in Decoder and adds limit-aware value combination, with overlapping goals around constrained list handling.
⚡ various optimizations #8: Updates duplicate-key merge handling and merge utilities in both Decoder and Utils, similar scope of refactoring merge operations.

Poem

🐰 Lists overflow? No fright!
Index keys map them just right,
Limits respected with care,
Overflow state tracked with flair,
Query strings merge, clean and bright! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is comprehensive and covers key changes, testing updates, and dependency changes. All major sections are well-documented with clear explanations and references.
Title check	✅ Passed	The title directly and clearly describes the main change: implementing DecodeOptions.ListLimit handling to prevent DoS via memory exhaustion, which aligns with the core objective of the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-01-11T23:20:02Z

Codecov Report

❌ Patch coverage is 88.99083% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.40%. Comparing base (fe5ea33) to head (26db1f9).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
QsNet/Internal/Utils.cs	88.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
- Coverage   98.14%   97.40%   -0.75%     
==========================================
  Files          15       15              
  Lines        1400     1500     +100     
  Branches      396      427      +31     
==========================================
+ Hits         1374     1461      +87     
- Misses         26       39      +13

Flag	Coverage Δ
unittests	`97.40% <88.99%> (-0.75%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…meric keys

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

QsNet.Tests/DecodeTests.cs (1)
2580-2590: Good coverage for implicit array limit overflow.

The test correctly verifies that when the number of elements (105) exceeds ListLimit (100), the result becomes an IDictionary with all values preserved.
💡 Optional: Consider using FluentAssertions' Which clause for cleaner assertions
 result.Should().ContainKey("a");
-result["a"].Should().BeAssignableTo<IDictionary>();
-((IDictionary)result["a"]!).Count.Should().Be(105);
+result["a"].Should().BeAssignableTo<IDictionary>()
+    .Which.Count.Should().Be(105);

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe5ea33 and 8e54046.

📒 Files selected for processing (5)

QsNet.Comparison/js/package.json
QsNet.Tests/DecodeTests.cs
QsNet.Tests/UtilsTests.cs
QsNet/Internal/Decoder.cs
QsNet/Internal/Utils.cs

🧰 Additional context used

📓 Path-based instructions (4)

QsNet.Tests/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

QsNet.Tests/**/*.cs: Tests must use xUnit with FluentAssertions
Test method naming should follow Should style

Files:

QsNet.Tests/UtilsTests.cs
QsNet.Tests/DecodeTests.cs

QsNet.Tests/**/DecodeTests.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

When extending decode tests, clone analogous existing cases in DecodeTests.cs to ensure js-qs parity

Files:

QsNet.Tests/DecodeTests.cs

QsNet/{Internal,Models,Enums,Constants}/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

All code under Internal/, Models/, Enums/, Constants/ is implementation detail and must not expose public surface

Files:

QsNet/Internal/Utils.cs
QsNet/Internal/Decoder.cs

QsNet/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Add concise XML docs on new public members reflecting actual behavior for DocFX

Files:

QsNet/Internal/Utils.cs
QsNet/Internal/Decoder.cs

🧠 Learnings (16)

📓 Common learnings

Learnt from: CR
Repo: techouse/qs-net PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-07T20:35:41.523Z
Learning: Applies to Models/DecodeOptions.cs : Preserve Depth, ParameterLimit, and ListLimit guards; keep defaults protective and make extensions feature-gated

Learnt from: CR
Repo: techouse/qs-net PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-07T20:35:41.523Z
Learning: Applies to QsNet.Tests/**/DecodeTests.cs : When extending decode tests, clone analogous existing cases in DecodeTests.cs to ensure js-qs parity

Learnt from: CR
Repo: techouse/qs-net PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-07T20:35:41.523Z
Learning: Applies to Internal/**/Decoder*.cs : Escalate large indices to dictionary when index exceeds ListLimit

Learnt from: CR
Repo: techouse/qs-net PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-07T20:35:41.523Z
Learning: Applies to QsNet.Tests/**/EncodeTests.cs : When extending encode tests, clone analogous existing cases in EncodeTests.cs to ensure js-qs parity