Skip to content

fix: trim OWS after commas when splitting list parameters - #1089

Open
stareezy-1 wants to merge 1 commit into
danielgtaylor:mainfrom
stareezy-1:fix/1084-conditional-ows
Open

fix: trim OWS after commas when splitting list parameters#1089
stareezy-1 wants to merge 1 commit into
danielgtaylor:mainfrom
stareezy-1:fix/1084-conditional-ows

Conversation

@stareezy-1

Copy link
Copy Markdown

Summary

Fixes #1084

Problem

parseInto splits list-style header/query params on a literal comma:

values = strings.Split(value, ",")

RFC 9110 §5.6.1 list grammar permits optional whitespace (OWS) around each comma, so a conventional If-None-Match: "a", "b" produces ["\"a\"", " \"b\""] — every element after the first keeps a leading space. conditional.trimETag strips quotes but not whitespace, so the space-tainted element can never equal a bare ETag: a 304-eligible request silently returns a full 200.

Reproduction:

If-None-Match: "target-tag"          → 304 (correct)
If-None-Match: "other-tag","target-tag" → 304 (correct)
If-None-Match: "other-tag", "target-tag" → 200 (bug)

Fix

Trim OWS (" \t") from each element at the shared split site in parseInto. This fixes every list-style header/query param at once rather than just the conditional package, matching the fix location suggested in the issue.

Tests

TestIfNoneMatchOWSAfterComma in conditional/params_test.go registers a real operation with embedded Params and asserts:

Case Verifies
"other-tag", "target-tag" (OWS on second element) 304 — fails without the fix
"target-tag", "other-tag" (OWS elsewhere) 304
"other-tag","target-tag" (no space) 304
non-matching list 200

All existing tests continue to pass (go test -race ./...).

RFC 9110 $5.6.1 list grammar permits optional whitespace around each
comma, e.g. 'If-None-Match: "a", "b"'. The list-splitting code in
parseInto kept the leading space on every element after the first, so
conditional.Params ETag comparisons silently missed legitimate matches
and served a 200 instead of a 304.
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.

conditional: If-None-Match/If-Match lists with RFC 9110 OWS after a comma fail to match

1 participant