diff --git a/core/artwork/serving.go b/core/artwork/serving.go index 4cdd04883..9ba5647d7 100644 --- a/core/artwork/serving.go +++ b/core/artwork/serving.go @@ -27,11 +27,19 @@ var errStaleSource = errors.New("artwork: source file changed since resolution") // Image is one servable artwork response. type Image struct { io.ReadCloser - Hash string // "" for placeholders + Hash string // pixel-identity hash (immutable URL match); "" for placeholders + ETag string // served-representation validator; "" falls back to Hash (full-size original) LastUpdated time.Time // zero for placeholders Placeholder bool } +// representationTag identifies a served resized representation for HTTP validation: it changes with +// the dimensions and the encode settings (CoverArtQuality/EnableWebPEncoding), so a config change +// invalidates a revalidating client's cache even though the pixel hash is unchanged. +func representationTag(hash string, size int, square bool) string { + return fmt.Sprintf("%s.%d.%v.%s", hash, size, square, formatQualityTag()) +} + type Service interface { // Get serves resolved/provisional artwork; ErrUnavailable or model.ErrNotFound when // there is nothing to serve (absent, pending, dangling) — caller picks placeholder vs 404. @@ -121,7 +129,7 @@ func (s *service) serveHash(ctx context.Context, artID model.ArtworkID, ia *mode } return s.dangling(ctx, artID) } - return &Image{ReadCloser: stream, Hash: ia.Hash, LastUpdated: ia.UpdatedAt}, nil + return &Image{ReadCloser: stream, Hash: ia.Hash, ETag: representationTag(ia.Hash, size, square), LastUpdated: ia.UpdatedAt}, nil } // openOriginal opens the full-resolution bytes for a found state row, enforcing the @@ -218,7 +226,7 @@ func (s *service) serveBytes(ctx context.Context, hash string, data []byte, last if err != nil { return nil, err } - return &Image{ReadCloser: stream, Hash: hash, LastUpdated: lastUpdate}, nil + return &Image{ReadCloser: stream, Hash: hash, ETag: representationTag(hash, size, square), LastUpdated: lastUpdate}, nil } // serveMediaFile serves a track: own found art wins; an absent row delegates to the album; diff --git a/core/artwork/serving_test.go b/core/artwork/serving_test.go index 68ca77cd6..35e92b522 100644 --- a/core/artwork/serving_test.go +++ b/core/artwork/serving_test.go @@ -106,6 +106,10 @@ var _ = Describe("Service", func() { img, err := svc.Get(ctx, model.MustParseArtworkID("al-al1"), 100, false) Expect(err).ToNot(HaveOccurred()) + // A resized response versions its validator with the encode settings, distinct from + // the pixel hash, so a CoverArtQuality/WebP change invalidates client caches. + Expect(img.ETag).To(Equal(representationTag(img.Hash, 100, false))) + Expect(img.ETag).ToNot(Equal(img.Hash)) resized := readAll(img) cfg, _, err := image.DecodeConfig(bytes.NewReader(resized)) Expect(err).ToNot(HaveOccurred()) diff --git a/server/imghttp/headers.go b/server/imghttp/headers.go index a5f3cf43e..e7bb6d562 100644 --- a/server/imghttp/headers.go +++ b/server/imghttp/headers.go @@ -21,11 +21,18 @@ func WriteImageHeaders(w http.ResponseWriter, r *http.Request, img *artwork.Imag return false } - h.Set("ETag", `"`+img.Hash+`"`) + // The validator identifies the served representation (resized/re-encoded bytes version it via + // ETag), so a CoverArtQuality/EnableWebPEncoding change invalidates a revalidating client's + // cache. Falls back to the pixel hash for full-size originals (bytes == the hash). + etag := img.ETag + if etag == "" { + etag = img.Hash + } + h.Set("ETag", `"`+etag+`"`) if !img.LastUpdated.IsZero() { h.Set("Last-Modified", img.LastUpdated.UTC().Format(http.TimeFormat)) } - // Immutable only when the client asked for the exact current hash; bare/legacy/mismatched + // Immutable only when the client asked for the exact current pixel hash; bare/legacy/mismatched // requests get cheap ETag revalidation instead, which fixes stale art after re-resolution. if requestedHash != "" && requestedHash == img.Hash { h.Set("Cache-Control", "public, max-age=31536000, immutable") @@ -33,7 +40,7 @@ func WriteImageHeaders(w http.ResponseWriter, r *http.Request, img *artwork.Imag h.Set("Cache-Control", "public, no-cache") } - if ifNoneMatch(r.Header.Get("If-None-Match"), img.Hash) { + if ifNoneMatch(r.Header.Get("If-None-Match"), etag) { w.WriteHeader(http.StatusNotModified) return true } diff --git a/server/imghttp/headers_test.go b/server/imghttp/headers_test.go index 6bd824fbe..2d72c3563 100644 --- a/server/imghttp/headers_test.go +++ b/server/imghttp/headers_test.go @@ -28,6 +28,19 @@ func placeholder() *artwork.Image { return &artwork.Image{ReadCloser: io.NopCloser(strings.NewReader("PH")), Placeholder: true} } +const testRepTag = testHash + ".300.false.q75" + +// resized carries a representation ETag distinct from the pixel hash (as a resized/re-encoded +// response does), so the validator versions with the encode settings. +func resized() *artwork.Image { + return &artwork.Image{ + ReadCloser: io.NopCloser(strings.NewReader("IMG")), + Hash: testHash, + ETag: testRepTag, + LastUpdated: lastMod, + } +} + func TestWriteImageHeaders(t *testing.T) { tests := []struct { name string @@ -67,6 +80,32 @@ func TestWriteImageHeaders(t *testing.T) { wantETag: `"` + testHash + `"`, wantLastMod: true, }, + { + name: "resized keeps pixel-hash immutable but serves the representation ETag", + img: resized(), + requestedHash: testHash, + wantCache: "public, max-age=31536000, immutable", + wantETag: `"` + testRepTag + `"`, + wantLastMod: true, + }, + { + name: "resized 304s on the representation ETag, not the pixel hash", + img: resized(), + ifNoneMatch: `"` + testRepTag + `"`, + want304: true, + wantCache: "public, no-cache", + wantETag: `"` + testRepTag + `"`, + wantLastMod: true, + }, + { + name: "resized does not 304 on a stale pixel-hash validator (config changed)", + img: resized(), + ifNoneMatch: `"` + testHash + `"`, + want304: false, + wantCache: "public, no-cache", + wantETag: `"` + testRepTag + `"`, + wantLastMod: true, + }, { name: "If-None-Match matching the hash yields 304", img: found(),