From 829156f53db398c4fddba4993a96c5af5f4598b8 Mon Sep 17 00:00:00 2001 From: Brad Ison Date: Tue, 11 Aug 2026 11:16:27 +0200 Subject: [PATCH] [management] Add ETag and If-Match helpers for HTTP handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read-modify-write pattern conditional requests guard is not specific to one resource — every Terraform-managed resource has the same shape, so the second consumer of this is a matter of when, not if. Deciding the entity-tag quoting, the "*" form, the list form and the weak-validator rule once here means an adopting resource inherits all of them instead of re-deriving each. Only the derivation stays on the type, since only the type knows which of its fields the representation covers. IfMatch returns a nil-safe precondition rather than a slice of tags. If-Match is defined in terms of the strong comparison function, so a weak validator can never satisfy it — and a header carrying nothing but weak validators then parses to an empty list, which the obvious call-site check reads as "no precondition given" and lets the write through unguarded. That is precisely the failure this is meant to prevent, and it is reachable by a proxy that weakens an ETag in transit. An absent header is unconditional; a header that was sent but carries nothing usable refuses. Weak validators are dropped rather than unwrapped, for the same reason: unwrapping one into a strong tag would quietly grant a match that the client's own header said it could not have. No handler uses this yet. --- shared/management/http/util/etag.go | 103 +++++++++++++++ shared/management/http/util/etag_test.go | 161 +++++++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 shared/management/http/util/etag.go create mode 100644 shared/management/http/util/etag_test.go diff --git a/shared/management/http/util/etag.go b/shared/management/http/util/etag.go new file mode 100644 index 000000000..8dcc68618 --- /dev/null +++ b/shared/management/http/util/etag.go @@ -0,0 +1,103 @@ +package util + +import ( + "net/http" + "slices" + "strconv" + "strings" +) + +const ( + etagHeader = "ETag" + ifMatchHeader = "If-Match" + + // matchAny is the If-Match value that matches any current representation + // of the resource (RFC 9110 §13.1.1). + matchAny = "*" + + // weakPrefix marks a weak validator. If-Match is defined in terms of the + // strong comparison function, under which a weak validator never matches. + weakPrefix = "W/" +) + +// SetETag writes etag as a strong ETag response header, quoted per RFC 9110. +// The value passed in is the bare validator — callers derive it (typically +// from the type being served) and this applies the wire syntax, so the quoting +// is decided in one place rather than at every handler. +// +// Call it before writing the body: once the response is committed the header +// no longer reaches the client. An empty etag writes no header at all, so a +// caller with nothing to validate against does not have to special-case it. +func SetETag(w http.ResponseWriter, etag string) { + if etag == "" { + return + } + w.Header().Set(etagHeader, strconv.Quote(etag)) +} + +// Precondition is a parsed If-Match request precondition. The zero value +// matches nothing; a nil *Precondition is an unconditional request and matches +// everything, so a handler can pass the result of IfMatch straight through +// without a presence check. +type Precondition struct { + // tags are the strong entity-tags the client will accept, unquoted. + tags []string + + // any records the "*" form, which matches any current representation. + any bool +} + +// IfMatch parses the request's If-Match precondition. It returns nil when the +// header is absent — an unconditional request, which is the back-compatible +// default: clients that know nothing of conditional requests keep working. +// +// A header that is present but carries nothing usable — empty, or nothing but +// weak validators — yields a precondition that matches nothing rather than +// nil. Failing closed is the only safe direction: a client that meant to send +// a precondition must not have it silently dropped and its write let through +// unguarded. +func IfMatch(r *http.Request) *Precondition { + values := r.Header.Values(ifMatchHeader) + if len(values) == 0 { + return nil + } + + p := &Precondition{} + for _, value := range values { + for raw := range strings.SplitSeq(value, ",") { + candidate := strings.TrimSpace(raw) + switch { + case candidate == "": + // Tolerated rather than rejected: a stray comma changes + // nothing about what the client is willing to accept. + case candidate == matchAny: + p.any = true + case strings.HasPrefix(candidate, weakPrefix): + // Dropped, not unwrapped. If-Match uses strong comparison, so + // a weak validator cannot satisfy it — and unwrapping one into + // a strong tag would quietly grant the match the client's own + // header said it could not have. + default: + p.tags = append(p.tags, strings.Trim(candidate, `"`)) + } + } + } + return p +} + +// Matches reports whether etag — the bare validator of the resource as it +// currently stands — satisfies the precondition. A nil precondition matches +// everything. +// +// Callers must establish that the resource exists before consulting this: the +// "*" form asks whether there is any current representation, a question only +// the caller can answer, and this reports true for it. +func (p *Precondition) Matches(etag string) bool { + if p == nil { + return true + } + if p.any { + return true + } + return slices.Contains(p.tags, etag) +} diff --git a/shared/management/http/util/etag_test.go b/shared/management/http/util/etag_test.go new file mode 100644 index 000000000..67c154b83 --- /dev/null +++ b/shared/management/http/util/etag_test.go @@ -0,0 +1,161 @@ +package util + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestSetETag covers the wire syntax: the bare validator goes in, a quoted +// strong entity-tag comes out. Handlers pass what the type derived, so the +// quoting has to happen here or every handler re-decides it. +func TestSetETag(t *testing.T) { + rec := httptest.NewRecorder() + SetETag(rec, "9f86d081884c7d65") + + assert.Equal(t, `"9f86d081884c7d65"`, rec.Header().Get("ETag"), + "the validator must be emitted quoted") +} + +// TestSetETagEmpty pins the no-op: a caller with nothing to validate against +// must not emit an empty entity-tag, which would be a validator that every +// later request could match. +func TestSetETagEmpty(t *testing.T) { + rec := httptest.NewRecorder() + SetETag(rec, "") + + assert.Empty(t, rec.Header().Values("ETag"), "an empty validator must write no header") +} + +// TestSetETagRoundTrip closes the loop between the two halves of the helper: +// what SetETag emits is what IfMatch accepts back. A client echoing the header +// it was given must match, or conditional requests never succeed in practice. +func TestSetETagRoundTrip(t *testing.T) { + const etag = "9f86d081884c7d65" + + rec := httptest.NewRecorder() + SetETag(rec, etag) + + r := httptest.NewRequest(http.MethodPut, "/", nil) + r.Header.Set("If-Match", rec.Header().Get("ETag")) + + assert.True(t, IfMatch(r).Matches(etag), "an echoed ETag header must satisfy the precondition") +} + +// TestIfMatchAbsent pins the back-compatibility guarantee: a request with no +// If-Match is unconditional, and the nil precondition it yields matches +// anything so handlers need no presence check. +func TestIfMatchAbsent(t *testing.T) { + p := IfMatch(httptest.NewRequest(http.MethodPut, "/", nil)) + + require.Nil(t, p, "an absent header must yield no precondition") + assert.True(t, p.Matches("9f86d081884c7d65"), "a nil precondition must match anything") + assert.True(t, p.Matches(""), "a nil precondition must not depend on the validator") +} + +// TestIfMatchParsing walks the header forms a client can send. The weak and +// unusable cases are the ones that matter: each must yield a precondition that +// exists and refuses, never one that is absent and waves the write through. +func TestIfMatchParsing(t *testing.T) { + const current = "9f86d081884c7d65" + + cases := []struct { + name string + header string + match bool + reason string + }{ + { + name: "quoted current validator", + header: `"9f86d081884c7d65"`, + match: true, + reason: "the ordinary conditional request must be honoured", + }, + { + name: "unquoted current validator", + header: "9f86d081884c7d65", + match: true, + reason: "a client that omits the quoting means the same thing, and only an exact value can match", + }, + { + name: "stale validator", + header: `"0000000000000000"`, + match: false, + reason: "a validator from an earlier read must not match", + }, + { + name: "star", + header: "*", + match: true, + reason: "* matches any current representation", + }, + { + name: "list containing the current validator", + header: `"0000000000000000", "9f86d081884c7d65"`, + match: true, + reason: "If-Match is a list; any member matching is a match", + }, + { + name: "list of stale validators", + header: `"0000000000000000", "1111111111111111"`, + match: false, + reason: "a list none of whose members match must not match", + }, + { + name: "surrounding whitespace", + header: ` "9f86d081884c7d65" `, + match: true, + reason: "list whitespace is not part of the entity-tag", + }, + { + name: "stray comma", + header: `"9f86d081884c7d65", `, + match: true, + reason: "an empty list element says nothing about what the client accepts", + }, + { + name: "weak validator of the current representation", + header: `W/"9f86d081884c7d65"`, + match: false, + reason: "If-Match uses strong comparison, so a weak validator never satisfies it", + }, + { + name: "weak validator alongside a strong one", + header: `W/"0000000000000000", "9f86d081884c7d65"`, + match: true, + reason: "dropping the weak member must not discard the rest of the list", + }, + { + name: "empty header", + header: "", + match: false, + reason: "a precondition the server cannot make sense of must fail closed, not vanish", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRequest(http.MethodPut, "/", nil) + r.Header.Set("If-Match", tc.header) + + p := IfMatch(r) + require.NotNil(t, p, "a header that was sent must yield a precondition: %s", tc.reason) + assert.Equal(t, tc.match, p.Matches(current), tc.reason) + }) + } +} + +// TestIfMatchRepeatedHeader covers the same list split across header lines, +// which is semantically identical to the comma form and which a proxy is free +// to produce. +func TestIfMatchRepeatedHeader(t *testing.T) { + r := httptest.NewRequest(http.MethodPut, "/", nil) + r.Header.Add("If-Match", `"0000000000000000"`) + r.Header.Add("If-Match", `"9f86d081884c7d65"`) + + assert.True(t, IfMatch(r).Matches("9f86d081884c7d65"), + "entity-tags split across header lines must be read as one list") +}