Closes the last #253 concurrency-contract piece: make If-Match evaluation RFC 7232-correct.
ConcurrencyHeaders.ParseIfMatch classified any non-canonical/weak/list If-Match as Malformed -> 400 (a deliberate fail-safe, deferred from the reference-aggregate PR). Per RFC 7232 §3.1 a syntactically-valid entity-tag that simply doesn't strong-match must be 412; 400 is only for a genuine grammar violation.
What changed
Real RFC 7232 parser (If-Match = "*" / 1#entity-tag): walks the comma-separated list, validates each [W/]DQUOTE *etagc DQUOTE member, collects the strong members whose opaque text is our canonical decimal.
weak (W/"3"), empty (""), non-canonical ("03", "3.0", "+3"), out-of-range -> valid but contribute no version -> 412 (a Version-kind with an empty set = guaranteed no-match).
list ("3", "5") -> any strong member matches -> proceed.
No wire-contract change: 400 + 412 already declared on every PUT; the field is header-derived internal command state — no DTO/route/response-type change, so no OpenAPI regen. Verified: generated artifacts unchanged; the local docgen IHost error reproduces on untouched main (pre-existing env issue, CI boots fine).
Tests
ConcurrencyHeadersTests rewritten for the new classification (canonical, lists, weak/empty/non-canonical -> Version/empty-set, grammar violations -> Malformed).
New VersionedAggregateExtensionsTests for CheckVersion membership / empty-set -> 412 / force-write.
## Summary
Closes the last #253 concurrency-contract piece: make `If-Match` evaluation RFC 7232-correct.
`ConcurrencyHeaders.ParseIfMatch` classified **any** non-canonical/weak/list `If-Match` as `Malformed -> 400` (a deliberate fail-safe, deferred from the reference-aggregate PR). Per **RFC 7232 §3.1** a syntactically-valid entity-tag that simply doesn't strong-match must be **412**; **400** is only for a genuine grammar violation.
## What changed
- **Real RFC 7232 parser** (`If-Match = "*" / 1#entity-tag`): walks the comma-separated list, validates each `[W/]DQUOTE *etagc DQUOTE` member, collects the **strong** members whose opaque text is our canonical decimal.
- weak (`W/"3"`), empty (`""`), non-canonical (`"03"`, `"3.0"`, `"+3"`), out-of-range -> valid but contribute no version -> **412** (a `Version`-kind with an *empty* set = guaranteed no-match).
- list (`"3", "5"`) -> any strong member matches -> proceed.
- unquoted / SP-in-tag / unterminated / garbage / separator-only -> **400**.
- **Type reshape**: `IfMatchCondition.ExpectedVersion : Option<int>` -> `ExpectedVersions : Option<Seq<int>>`; `CheckVersion(Option<int>)` -> `CheckVersion(Option<Seq<int>>)` (set membership). Threads through 10 replace/update commands + handlers + request mappers + 9 controllers.
- **No wire-contract change**: 400 + 412 already declared on every PUT; the field is header-derived internal command state — no DTO/route/response-type change, so **no OpenAPI regen**. Verified: generated artifacts unchanged; the local docgen `IHost` error reproduces on untouched `main` (pre-existing env issue, CI boots fine).
## Tests
- `ConcurrencyHeadersTests` rewritten for the new classification (canonical, lists, weak/empty/non-canonical -> Version/empty-set, grammar violations -> Malformed).
- New `VersionedAggregateExtensionsTests` for `CheckVersion` membership / empty-set -> 412 / force-write.
- Full `ErsatzTV.Tests` green locally (1547 passing).
## Docs
- `api-conventions.md` §7a rewritten; `decisions.md` entry appended.
Refs #253 #197 · fixes #265
Codex (initial diff): BLOCKED, 2 real Highs the fork missed:
H1 — "3,5": comma (0x2C) is a valid etagc, so it can appear inside a quoted opaque-tag; the Split(',') broke one valid tag into two → 400 instead of 412.
H2 — * : string.Trim() strips NBSP/all-Unicode-whitespace, letting a malformed value masquerade as the * force-write escape.
Fix commit: replaced the split with a quote-aware position scanner (comma separates only outside quotes); trims only RFC OWS (SP/HTAB); + comma-in-tag / empty-element / NBSP / trailing-junk / lowercase-weak / wildcard-in-list tests; de-BOM'd the 8 touched Application files for the format gate.
Codex re-review of the fix commit: MERGEABLE — both Highs CLOSED, new-defect sweep clean (loop bounds, W/ case-sensitivity, IsEtagc ranges, sawTag, closing-quote arithmetic), all earlier-verified facts re-confirmed (CheckVersion membership, empty-set→412, None→force-write, no ExpectedVersions DTO leak).
Full ErsatzTV.Tests green locally (1556). Rebased onto origin/main (#319).
Review-verdict: MERGEABLE @ 50cd29d841c530c79ea2bea1f3a18b6129881ea7
Independent review loop complete on the current head:
- **Cold zero-context fork** (initial diff): MERGEABLE-WITH-NITS — 2 nits (misleading canonical-guard comment; missing `w/"3"` / `"3", *` test cases) → both applied.
- **Codex** (initial diff): BLOCKED, 2 real Highs the fork missed:
- H1 — `"3,5"`: comma (0x2C) is a valid `etagc`, so it can appear *inside* a quoted opaque-tag; the `Split(',')` broke one valid tag into two → 400 instead of 412.
- H2 — ` * `: `string.Trim()` strips NBSP/all-Unicode-whitespace, letting a malformed value masquerade as the `*` force-write escape.
- **Fix commit**: replaced the split with a quote-aware position scanner (comma separates only outside quotes); trims only RFC OWS (`SP`/`HTAB`); + comma-in-tag / empty-element / NBSP / trailing-junk / lowercase-weak / wildcard-in-list tests; de-BOM'd the 8 touched Application files for the format gate.
- **Codex re-review of the fix commit**: **MERGEABLE** — both Highs CLOSED, new-defect sweep clean (loop bounds, `W/` case-sensitivity, `IsEtagc` ranges, `sawTag`, closing-quote arithmetic), all earlier-verified facts re-confirmed (CheckVersion membership, empty-set→412, None→force-write, no `ExpectedVersions` DTO leak).
Full `ErsatzTV.Tests` green locally (1556). Rebased onto origin/main (#319).
The fix-as-you-touch format gate requires every .cs this PR touches to fully conform to
.editorconfig, including pre-existing whitespace on lines the change didn't edit. dotnet format
(whitespace) applied to the 11 touched files; legacy files left untouched (no big-bang reformat).
Whitespace/layout only — no behavior change. Refs #265
Head advanced from 50cd29d8 only by a whitespace-only formatting commit (ffece01c) to satisfy the #311 fix-as-you-touch format gate on the 11 touched files — git diff -w shows no substantive change (one with-block line-wrap aside). Carries forward the Codex fix-commit re-review (MERGEABLE: both Highs closed, new-defect sweep clean) — no logic re-review needed for a pure-formatting delta.
Review-verdict: MERGEABLE @ ffece01c320fbe88de2fdcc16740c48b7e0a3093
Head advanced from 50cd29d8 only by a **whitespace-only** formatting commit (ffece01c) to satisfy the #311 fix-as-you-touch format gate on the 11 touched files — `git diff -w` shows no substantive change (one `with`-block line-wrap aside). Carries forward the Codex fix-commit re-review (MERGEABLE: both Highs closed, new-defect sweep clean) — no logic re-review needed for a pure-formatting delta.
timothy
merged commit c197de421a into main2026-07-12 23:32:26 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Closes the last #253 concurrency-contract piece: make
If-Matchevaluation RFC 7232-correct.ConcurrencyHeaders.ParseIfMatchclassified any non-canonical/weak/listIf-MatchasMalformed -> 400(a deliberate fail-safe, deferred from the reference-aggregate PR). Per RFC 7232 §3.1 a syntactically-valid entity-tag that simply doesn't strong-match must be 412; 400 is only for a genuine grammar violation.What changed
If-Match = "*" / 1#entity-tag): walks the comma-separated list, validates each[W/]DQUOTE *etagc DQUOTEmember, collects the strong members whose opaque text is our canonical decimal.W/"3"), empty (""), non-canonical ("03","3.0","+3"), out-of-range -> valid but contribute no version -> 412 (aVersion-kind with an empty set = guaranteed no-match)."3", "5") -> any strong member matches -> proceed.IfMatchCondition.ExpectedVersion : Option<int>->ExpectedVersions : Option<Seq<int>>;CheckVersion(Option<int>)->CheckVersion(Option<Seq<int>>)(set membership). Threads through 10 replace/update commands + handlers + request mappers + 9 controllers.IHosterror reproduces on untouchedmain(pre-existing env issue, CI boots fine).Tests
ConcurrencyHeadersTestsrewritten for the new classification (canonical, lists, weak/empty/non-canonical -> Version/empty-set, grammar violations -> Malformed).VersionedAggregateExtensionsTestsforCheckVersionmembership / empty-set -> 412 / force-write.ErsatzTV.Testsgreen locally (1547 passing).Docs
api-conventions.md§7a rewritten;decisions.mdentry appended.Refs #253 #197 · fixes #265
c07812bc85to50cd29d841Review-verdict: MERGEABLE @
50cd29d841Independent review loop complete on the current head:
w/"3"/"3", *test cases) → both applied."3,5": comma (0x2C) is a validetagc, so it can appear inside a quoted opaque-tag; theSplit(',')broke one valid tag into two → 400 instead of 412.*:string.Trim()strips NBSP/all-Unicode-whitespace, letting a malformed value masquerade as the*force-write escape.SP/HTAB); + comma-in-tag / empty-element / NBSP / trailing-junk / lowercase-weak / wildcard-in-list tests; de-BOM'd the 8 touched Application files for the format gate.W/case-sensitivity,IsEtagcranges,sawTag, closing-quote arithmetic), all earlier-verified facts re-confirmed (CheckVersion membership, empty-set→412, None→force-write, noExpectedVersionsDTO leak).Full
ErsatzTV.Testsgreen locally (1556). Rebased onto origin/main (#319).Review-verdict: MERGEABLE @
ffece01c32Head advanced from
50cd29d8only by a whitespace-only formatting commit (ffece01c) to satisfy the #311 fix-as-you-touch format gate on the 11 touched files —git diff -wshows no substantive change (onewith-block line-wrap aside). Carries forward the Codex fix-commit re-review (MERGEABLE: both Highs closed, new-defect sweep clean) — no logic re-review needed for a pure-formatting delta.