mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(artwork): version the artwork ETag with the served representation
The ETag was the pixel hash of the original image, so a CoverArtQuality or EnableWebPEncoding change altered the resized bytes without changing the ETag — revalidating clients got a spurious 304 and kept the old encoding. Resized responses now carry a representation ETag (hash + size + square + encode settings) used for the ETag header and If-None-Match, while the immutable decision stays on the pixel hash (URLs remain pixel-identity per the spec, so hash-suffixed clients keep zero-request caching). Full-size originals fall back to the pixel hash as before.
This commit is contained in:
parent
9dd306eb10
commit
3eaa21229d
@ -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;
|
||||
|
||||
@ -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())
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -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(),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user