Skip to content

[AsParameters]: reject unparseable values in collection query parameters (GH-3398)#3404

Merged
jeremydmiller merged 1 commit into
mainfrom
fix-3398-collection-query-binding
Jul 13, 2026
Merged

[AsParameters]: reject unparseable values in collection query parameters (GH-3398)#3404
jeremydmiller merged 1 commit into
mainfrom
fix-3398-collection-query-binding

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #3398. For 6.18.0. Reported by @outofrange-consulting — thank you. (Supersedes #3402, which was accidentally cut on top of another branch; identical commit, rebased onto main.)

The bug

An unparseable value in a collection query parameter silently bound null instead of returning 400:

  • ?colours=Purple (invalid enum) → 200 OK, Colours == null
  • ?colours=Red&colours=Purple200 OK, Colours == null — the valid Red is dropped too
  • Same for int[] / Guid[] / long[].

null is indistinguishable from "parameter omitted", so on a filter endpoint this silently drops the predicate and returns an unfiltered 200. FluentValidation cannot compensate: it sees null, so RuleForEach has nothing to iterate. A silently wrong answer is the worst failure mode available here.

Root cause

Two codegen frames handle collection binding and neither knew about the #3372 strict flag:

  • CodeGen/ParsedArrayQueryStringValue.cs (T[])
  • CodeGen/QueryStringHandling.csParsedCollectionQueryStringValue (List<T> / IList<T> / IReadOnlyList<T> / IEnumerable<T>)

Both emit foreach + if (TryParse(...)) list.Add(...) with no else, so a bad element is simply dropped. The array frame then does if (list.Any()) prop = list.ToArray(); — so an all-bad collection leaves the property null, and a mixed one silently binds a partial filter.

The fix — completes #3372

#3372 fixed the scalar case (shipped 6.17.1/6.17.2). Both collection frames now take the same rejectUnparseableValue flag, wired from WolverineHttpOptions.RejectUnparseableQueryValues through HttpChain.TryFindOrCreateQuerystringValue, and emit a shared StrictQueryBinding.WriteRejectionBlock that calls the existing HttpHandler.WriteQueryValueParsingProblem.

Semantics mirror the scalar case exactly:

  • present-but-unparseable → 400 ProblemDetails naming the parameter
  • omitted entirely → keeps its initializer, in both modes
  • flag off → today's lenient behavior, byte-for-byte. The default is unchanged (it flips to strict in Wolverine 7.0).

All-or-nothing, deliberately

One bad element rejects the whole collection. Binding [Red] from ?colours=Red&colours=Purple would still be exactly the silently-wrong-answer this issue is about — the caller asked to filter on two values and would get results filtered on one, with a 200 and no signal. Minimal APIs likewise fail the request rather than hand back a partial collection. Rationale is in the docs too, not just here.

Verification

  • Red first: precisely the 6 new strict-mode tests failed (Failed: 6, Passed: 17) while the lenient/omitted/valid tests already passed — confirming the change is confined to the flag and cannot alter default behavior.
  • Green: strict_query_string_binding 23/23.
  • Full Wolverine.Http.Tests 844 passed, 0 failed (net9.0).
  • Full wolverine.slnx Release build: 0 warnings, 0 errors.
  • Docs: as-parameters.md gains a Collections subsection (behavior matrix + rationale); querystring.md and the RejectUnparseableQueryValues XML doc note collection coverage.

🤖 Generated with Claude Code

…ers (GH-3398)

GH-3372 made a present-but-unparseable *scalar* query string value short circuit
with a 400 ProblemDetails under the opt-in
WolverineHttpOptions.RejectUnparseableQueryValues. Collections were left as a
noted follow-up, and they are the worse failure mode: an unparseable element was
silently dropped, taking an optional filter collection to null (or to a partial
list), so a filtering endpoint answered 200 with an *unfiltered* result set while
the caller believed the results were filtered. FluentValidation cannot compensate
because it only ever sees null.

ParsedArrayQueryStringValue (T[]) and ParsedCollectionQueryStringValue
(List<T>/IList<T>/IReadOnlyList<T>/IEnumerable<T>) now take the same
rejectUnparseableValue flag as ReadHttpFrame and emit a shared rejection block: a
present but unparseable element writes the 400 ProblemDetails naming the parameter
and exits. Binding is all-or-nothing -- one bad element rejects the whole
collection rather than silently keeping the parseable ones. An omitted parameter
still keeps its initializer in both modes, string collections are untouched, and
the flag remains opt-in (default false) until the 7.0 flip.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jeremydmiller
jeremydmiller merged commit f47f2a1 into main Jul 13, 2026
26 checks passed
This was referenced Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[AsParameters]: unparseable value in a collection query parameter silently binds null instead of returning 400 (scalar case fixed by #3372)

1 participant