From 9e7fd2eb90f946e451c08626c970cd7665282afd Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Sat, 24 Oct 2026 19:34:41 +0530 Subject: [PATCH] filer: match If-Match and If-None-Match against "*" and ETag lists (#12026) * filer: match If-Match and If-None-Match against "*" and ETag lists checkPreconditions compared the whole header as one ETag, so "*", a list of ETags and a weak W/ tag in If-None-Match never matched (RFC 9110 13.1.1, 13.1.2). * filer: split etag lists only outside quoted tags A comma inside a quoted opaque tag is a legal etagc, not a list separator; strings.Split cut such tags into fragments that CanonicalizeETag could turn into a false match. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: match repeated If-Match and If-None-Match header lines Header.Get reads only the first field line, so a match on a later repeated header was missed; join all field values like any other comma-separated list. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: ignore empty elements in etag precondition lists * filer: let a present-but-empty precondition line suppress the date check --------- Co-authored-by: user.email Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: chrislusf --- weed/server/filer_server_handlers_read.go | 58 ++++++++- ..._server_handlers_read_precondition_test.go | 115 ++++++++++++++++++ 2 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 weed/server/filer_server_handlers_read_precondition_test.go diff --git a/weed/server/filer_server_handlers_read.go b/weed/server/filer_server_handlers_read.go index 2fa32fa2ff..cf1e2391e2 100644 --- a/weed/server/filer_server_handlers_read.go +++ b/weed/server/filer_server_handlers_read.go @@ -50,10 +50,10 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, entry *filer.Ent } w.Header().Set("Last-Modified", entry.Attr.Mtime.UTC().Format(http.TimeFormat)) - ifMatchETagHeader := r.Header.Get("If-Match") + ifMatchValues := r.Header.Values("If-Match") ifUnmodifiedSinceHeader := r.Header.Get("If-Unmodified-Since") - if ifMatchETagHeader != "" { - if util.CanonicalizeETag(etag) != util.CanonicalizeETag(ifMatchETagHeader) { + if len(ifMatchValues) != 0 { + if !etagListMatches(strings.Join(ifMatchValues, ","), etag, false) { w.WriteHeader(http.StatusPreconditionFailed) return true } @@ -66,10 +66,10 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, entry *filer.Ent } } - ifNoneMatchETagHeader := r.Header.Get("If-None-Match") + ifNoneMatchValues := r.Header.Values("If-None-Match") ifModifiedSinceHeader := r.Header.Get("If-Modified-Since") - if ifNoneMatchETagHeader != "" { - if util.CanonicalizeETag(etag) == util.CanonicalizeETag(ifNoneMatchETagHeader) { + if len(ifNoneMatchValues) != 0 { + if etagListMatches(strings.Join(ifNoneMatchValues, ","), etag, true) { SetEtag(w, etag) w.WriteHeader(http.StatusNotModified) return true @@ -87,6 +87,52 @@ func checkPreconditions(w http.ResponseWriter, r *http.Request, entry *filer.Ent return false } +// etagListMatches reports whether an If-Match or If-None-Match value, "*" or a +// list of entity tags, matches etag (RFC 9110 13.1.1, 13.1.2). If-Match uses the +// strong comparison, so a weak tag never matches; If-None-Match the weak one. +func etagListMatches(header, etag string, weak bool) bool { + if strings.TrimSpace(header) == "*" { + return true + } + for _, tag := range splitETagList(header) { + tag = strings.TrimSpace(tag) + if strings.HasPrefix(tag, "W/") { + if !weak { + continue + } + tag = tag[len("W/"):] + } + // Empty list elements are ignored, so padding like a trailing + // comma cannot match an empty opaque tag. + if tag == "" { + continue + } + if util.CanonicalizeETag(tag) == util.CanonicalizeETag(etag) { + return true + } + } + return false +} + +// splitETagList splits an entity-tag list on the commas between tags; a comma +// inside a quoted opaque tag does not split. +func splitETagList(header string) []string { + var tags []string + start, quoted := 0, false + for i := 0; i < len(header); i++ { + switch header[i] { + case '"': + quoted = !quoted + case ',': + if !quoted { + tags = append(tags, header[start:i]) + start = i + 1 + } + } + } + return append(tags, header[start:]) +} + func (fs *FilerServer) GetOrHeadHandler(w http.ResponseWriter, r *http.Request) { ctx := r.Context() path := r.URL.Path diff --git a/weed/server/filer_server_handlers_read_precondition_test.go b/weed/server/filer_server_handlers_read_precondition_test.go new file mode 100644 index 0000000000..053056a130 --- /dev/null +++ b/weed/server/filer_server_handlers_read_precondition_test.go @@ -0,0 +1,115 @@ +package weed_server + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" +) + +func TestCheckPreconditionsETagLists(t *testing.T) { + entry := &filer.Entry{Attr: filer.Attr{Mtime: time.Unix(1700000000, 0), Md5: []byte{0xb1, 0x94}}} + const etag = `"b194"` + + tests := []struct { + header string + values []string + want int + }{ + {"If-None-Match", []string{etag}, http.StatusNotModified}, + {"If-None-Match", []string{`*`}, http.StatusNotModified}, + {"If-None-Match", []string{`W/` + etag}, http.StatusNotModified}, + {"If-None-Match", []string{`"x", ` + etag}, http.StatusNotModified}, + {"If-None-Match", []string{`"x"`, etag}, http.StatusNotModified}, + {"If-None-Match", []string{`"x"`}, http.StatusOK}, + {"If-None-Match", []string{`"b194,old"`}, http.StatusOK}, + {"If-Match", []string{etag}, http.StatusOK}, + {"If-Match", []string{`*`}, http.StatusOK}, + {"If-Match", []string{`"x", ` + etag}, http.StatusOK}, + {"If-Match", []string{`"x"`, etag}, http.StatusOK}, + {"If-Match", []string{`W/` + etag}, http.StatusPreconditionFailed}, + {"If-Match", []string{`"x"`}, http.StatusPreconditionFailed}, + {"If-Match", []string{`"b194,old"`}, http.StatusPreconditionFailed}, + } + for _, tt := range tests { + r := httptest.NewRequest(http.MethodGet, "/d/f.txt", nil) + for _, v := range tt.values { + r.Header.Add(tt.header, v) + } + w := httptest.NewRecorder() + got := http.StatusOK + if checkPreconditions(w, r, entry) { + got = w.Code + } + if got != tt.want { + t.Errorf("%s: %q -> %d, want %d", tt.header, tt.values, got, tt.want) + } + } +} + +func TestCheckPreconditionsPresentButEmptyHeaders(t *testing.T) { + entry := &filer.Entry{Attr: filer.Attr{Mtime: time.Unix(1700000000, 0), Md5: []byte{0xb1, 0x94}}} + before := time.Unix(1700000000-60, 0).UTC().Format(http.TimeFormat) + after := time.Unix(1700000000+60, 0).UTC().Format(http.TimeFormat) + + // An empty If-Match line is still an If-Match: it matches nothing, and + // If-Unmodified-Since stays ignored. + r := httptest.NewRequest(http.MethodGet, "/d/f.txt", nil) + r.Header.Set("If-Match", "") + r.Header.Set("If-Unmodified-Since", after) + if w := httptest.NewRecorder(); !checkPreconditions(w, r, entry) || w.Code != http.StatusPreconditionFailed { + t.Errorf("empty If-Match -> %d, want 412", w.Code) + } + + // An empty If-None-Match line matches nothing and suppresses + // If-Modified-Since, so the request proceeds. + r = httptest.NewRequest(http.MethodGet, "/d/f.txt", nil) + r.Header.Set("If-None-Match", "") + r.Header.Set("If-Modified-Since", after) + if checkPreconditions(httptest.NewRecorder(), r, entry) { + t.Error("empty If-None-Match with matching If-Modified-Since -> 304, want 200") + } + + r = httptest.NewRequest(http.MethodGet, "/d/f.txt", nil) + r.Header.Set("If-Modified-Since", before) + if checkPreconditions(httptest.NewRecorder(), r, entry) { + t.Error("old If-Modified-Since alone -> 304, want 200") + } +} + +func TestCheckPreconditionsIgnoresEmptyETagListElements(t *testing.T) { + entry := &filer.Entry{ + Attr: filer.Attr{Mtime: time.Unix(1700000000, 0)}, + Remote: &filer_pb.RemoteEntry{RemoteSize: 1}, + } + + tests := []struct { + header string + values []string + want int + }{ + {"If-None-Match", []string{`"x",`}, http.StatusOK}, + {"If-None-Match", []string{`"x",,"y"`}, http.StatusOK}, + {"If-None-Match", []string{`""`}, http.StatusNotModified}, + {"If-None-Match", []string{`W/""`}, http.StatusNotModified}, + {"If-Match", []string{`"x",`}, http.StatusPreconditionFailed}, + {"If-Match", []string{`""`}, http.StatusOK}, + } + for _, tt := range tests { + r := httptest.NewRequest(http.MethodGet, "/d/f.txt", nil) + for _, v := range tt.values { + r.Header.Add(tt.header, v) + } + w := httptest.NewRecorder() + got := http.StatusOK + if checkPreconditions(w, r, entry) { + got = w.Code + } + if got != tt.want { + t.Errorf("%s: %q -> %d, want %d", tt.header, tt.values, got, tt.want) + } + } +}