danielgtaylor / danielgtaylor/huma

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

Open Beginner friendly
#1,084 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
4.4k
Forks
285
Avg merge
40m
Merged PRs (30d)
1

Description

## Version

`github.com/danielgtaylor/huma/v2 v2.39.1` (module cache path:
`github.com/danielgtaylor/huma/v2@v2.39.1`), specifically the
`conditional` subpackage combined with the core request-parameter parser
in `huma.go`.

## Summary

A multi-value `If-None-Match` (or `If-Match`) header whose elements are
separated by `", "` (comma + a space, or any other optional whitespace) —
which RFC 9110 §5.6.1's list grammar explicitly permits — fails to match
an ETag that would match if the same header were sent without the space.
This means `conditional.Params.PreconditionFailed` can silently miss a
legitimate conditional-request match for any list element after the
first, causing a 304-eligible request to instead return a full 200 with
the complete response body.

## Root cause

1. `huma.go`'s parameter parser splits a list-style header/query param on
a literal comma with no whitespace handling:

```go
// huma.go:1871 (parseInto, reflect.Slice case, non-Explode path)
values = strings.Split(value, ",")
```

For `If-None-Match: "a", "b"` this produces `[]string{`"a"`, ` "b"`}` —
note the leading space preserved on the second element.

2. `conditional.trimETag` (`conditional/params.go:24-29`) is the only place
that later tries to normalize an incoming list element for comparison,
and it only strips a `W/` prefix and `"` characters:

```go
// conditional/params.go:24-29
func trimETag(value string) string {
if strings.HasPrefix(value, "W/") && len(value) > 2 {
value = value[2:]
}
return strings.Trim(value, "\"")
}
```

`strings.Trim(value, "\"")` only removes leading and trailing characters
that are themselves `"`. Given ` "b"` the leading character is a space,
not a quote, so `Trim` stops immediately on that side and strips only the
trailing quote:

```
input=" \"b\"" Trim(input, "\"") = " \"b" equals bare tag "b"? false
```

The element keeps both its leading space and its opening quote, so it can
never equal the bare tag a caller passes into `PreconditionFailed`.

3. `PreconditionFailed` (`conditional/params.go:66-152`) then compares
`trimETag(match) == etag` for each list element, so the space-tainted
element never matches even though its "real" value is a legitimate,
correctly-quoted entity-tag.

## RFC citation

RFC 9110 §5.6.1 defines the list extension used by `If-Match` (§13.1.1)
and `If-None-Match` (§13.1.2):

> `#element => [ element ] *( OWS "," OWS [ element ] )`

`OWS` (optional whitespace) is explicitly part of the grammar around each
comma. A client sending `If-None-Match: "a", "b"` — the conventional,
human-readable way to write a comma list, and what `net/http`'s own
`http.Header.Add`-then-join behavior and most intermediary proxies/CDNs
produce — is fully RFC-conformant. huma should not require callers to
omit that whitespace to get a correct match.

## Minimal reproduction

```go
package main

import (
"fmt"
"time"

"github.com/danielgtaylor/huma/v2/conditional"
)

func main() {
p := &conditional.Params{
// Note the space after the comma — RFC 9110 §5.6.1 OWS.
IfNoneMatch: []string{`"other-tag"`, ` "target-tag"`},
}

// A resource whose current ETag is "target-tag".
err := p.PreconditionFailed("target-tag", time.Time{})
if err != nil {
fmt.Println("matched (304 expected):", err.GetStatus())
} else {
fmt.Println("did NOT match (200 will be served) — bug reproduced")
}
}
```

Or, against any server built on `humago` + `conditional.Params` embedded
in an operation's input struct:

```
curl -i -H 'If-None-Match: "other-tag", "target-tag"' https://example/resource
```

## Expected vs. actual

| Request | Expected | Actual |
|---|---|---|
| `If-None-Match: "target-tag"` | 304 | 304 (correct) |
| `If-None-Match: "other-tag","target-tag"` (no space) | 304 | 304 (correct) |
| `If-None-Match: "other-tag", "target-tag"` (RFC-legal space) | 304 | **200** |
| `If-None-Match: "target-tag", "other-tag"` (space lands on the *other* element) | 304 | 304 (correct, but only by luck of position) |

Only the first list element is ever compared reliably; every subsequent
element is one un-trimmed leading space away from silently failing to
match.

## Suggested fix

Trim OWS (`" \t"`) from each element after splitting, either at the split
site or inside `trimETag`. Smallest fix, at the split site in
`huma.go`'s `parseInto`:

```go
// huma.go:1871
values = strings.Split(value, ",")
for i, v := range values {
values[i] = strings.Trim(v, " \t") // RFC 9110 §5.6.1 OWS
}
```

This is a general fix for every list-style header/query param huma parses
(not `conditional`-specific), which is probably the more correct place
for it — `conditional.trimETag` only ever sees what `parseInto` already
handed it, so fixing the split site fixes every caller at once rather
than just this one package.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in huma.go at parseInto's reflect.Slice non-Explode path, then read conditional/params.go and trimETag to understand how list elements reach PreconditionFailed. Add coverage for RFC 9110 optional whitespace in multi-value If-Match and If-None-Match headers, and verify that tags after a comma and space match as expected without breaking existing no-space behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.