fix(artwork): give full-size disc art a real ETag

Regression from 04d5a556. Keying the resize cache on identity meant the
disc response no longer carried a content hash, and the full-size branch
set no ETag either — so WriteImageHeaders fell back to the empty hash
and emitted `ETag: ""` for every full-size disc image. Since ifNoneMatch
compares the unquoted value, a client echoing that back matched, and got
a 304 even after the image was replaced.

The full-size branch now carries the same identity validator the sized
branch uses, so it moves when the folder's images change.

WriteImageHeaders is hardened against the class as well: an empty
validator is no validator, so it is neither emitted nor matched.

Reported by Codex on #5847.

core/artwork/serving.go

@@ -313,20 +313,19 @@ func (s *service) serveDisc(ctx context.Context, artID model.ArtworkID, size int
 		return selectImageReader(ctx, artID, funcs...)
 	}
 	albumArtID := model.ArtworkID{Kind: model.KindAlbumArtwork, ID: dr.album.ID}
+	// Disc art has no state row, so there is no stored content hash — to key the resize cache
+	// on, or to fall back to as a validator. Keying on the id and the album's mtime, as the
+	// legacy reader did, lets a warm cache answer without touching the filesystem; the chain
+	// runs only on a miss. The key carries DiscArtPriority so changing it invalidates.
+	key := fmt.Sprintf("%s|%d|%s", artID.ID, dr.cacheTime().UnixNano(), conf.Server.DiscArtPriority)
 	if size == 0 && !square {
-		r, path, err := selectImage()
+		r, _, err := selectImage()
 		if err != nil || r == nil {
 			return s.Get(ctx, albumArtID, size, square)
 		}
-		return &Image{ReadCloser: r, LastUpdated: unixMtime(mtimeViaFS(dr.lib.FS, path))}, nil
+		return &Image{ReadCloser: r, ETag: representationTag(key, size, square), LastUpdated: dr.cacheTime()}, nil
 	}
 
-	// Disc art has no state row, so there is no stored content hash to key the resize cache on.
-	// Keying on the id and the album's mtime — as the legacy reader did — lets a warm cache
-	// answer without touching the filesystem at all; the chain runs only on a miss. The key
-	// carries DiscArtPriority so changing it invalidates. resizedItem.hash is key material
-	// here, not a content hash, so the response carries no Image.Hash.
-	key := fmt.Sprintf("%s|%d|%s", artID.ID, dr.cacheTime().UnixNano(), conf.Server.DiscArtPriority)
 	item := &resizedItem{
 		hash:       key,
 		size:       size,

core/artwork/serving_test.go

@@ -393,6 +393,28 @@ var _ = Describe("Service", func() {
 			Expect(second.ETag).ToNot(Equal(firstKey), "a replaced image must not keep the old cache entry")
 		})
 
+		// Disc art has no content hash to fall back to, so without an explicit validator the
+		// full-size response would carry an empty ETag — identical for every disc image.
+		It("gives a full-size disc image a validator that tracks the source", func() {
+			folderRepo.result = []model.Folder{{
+				Path: "tests/fixtures/artist/an-album", ImageFiles: []string{"cover.jpg"},
+				ImagesUpdatedAt: time.Now().Add(-time.Hour),
+			}}
+			albumRepo.SetData(model.Albums{{ID: "aldc5", Name: "Album", FolderIDs: []string{"f1"}}})
+			discID := model.NewArtworkID(model.KindDiscArtwork, model.DiscArtworkID("aldc5", 1), nil)
+
+			first, err := svc.Get(ctx, discID, 0, false)
+			Expect(err).ToNot(HaveOccurred())
+			readAll(first)
+			Expect(first.ETag).ToNot(BeEmpty())
+
+			folderRepo.result[0].ImagesUpdatedAt = time.Now()
+			second, err := svc.Get(ctx, discID, 0, false)
+			Expect(err).ToNot(HaveOccurred())
+			readAll(second)
+			Expect(second.ETag).ToNot(Equal(first.ETag), "a replaced image must not revalidate as unchanged")
+		})
+
 		It("falls back to album art when no disc image matches", func() {
 			folderRepo.result = nil
 			albumRepo.SetData(model.Albums{{ID: "aldc2", Name: "Album"}})

server/imghttp/headers.go

@@ -28,7 +28,11 @@ func WriteImageHeaders(w http.ResponseWriter, r *http.Request, img *artwork.Imag
 	if etag == "" {
 		etag = img.Hash
 	}
-	h.Set("ETag", `"`+etag+`"`)
+	// An empty validator is not one: emitting it would hand every such response the same ETag,
+	// and matching it would 304 a client that echoed it back even after the bytes changed.
+	if etag != "" {
+		h.Set("ETag", `"`+etag+`"`)
+	}
 	if !img.LastUpdated.IsZero() {
 		h.Set("Last-Modified", img.LastUpdated.UTC().Format(http.TimeFormat))
 	}
@@ -40,7 +44,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"), etag) {
+	if etag != "" && ifNoneMatch(r.Header.Get("If-None-Match"), etag) {
 		w.WriteHeader(http.StatusNotModified)
 		return true
 	}

server/imghttp/headers_test.go

@@ -41,6 +41,11 @@ func resized() *artwork.Image {
 	}
 }
 
+// unvalidated stands for a response with no content hash and no representation tag.
+func unvalidated() *artwork.Image {
+	return &artwork.Image{ReadCloser: io.NopCloser(strings.NewReader("IMG")), LastUpdated: lastMod}
+}
+
 var _ = Describe("WriteImageHeaders", func() {
 	type testCase struct {
 		img           *artwork.Image
@@ -101,5 +106,13 @@ var _ = Describe("WriteImageHeaders", func() {
 			testCase{img: found(), ifNoneMatch: `"deadbeefdeadbeef"`, want304: false, wantCache: "public, no-cache", wantETag: `"` + testHash + `"`, wantLastMod: true}),
 		Entry("placeholder ignores If-None-Match and never 304s",
 			testCase{img: placeholder(), ifNoneMatch: "*", want304: false, wantCache: "no-store"}),
+
+		// An image with neither ETag nor Hash has no validator. Emitting one would give every
+		// such response the same empty tag, and matching it would 304 changed bytes.
+		Entry("omits the ETag entirely when there is no validator",
+			testCase{img: unvalidated(), wantCache: "public, no-cache", wantLastMod: true}),
+		Entry("never 304s an empty validator echoed back by the client",
+			testCase{img: unvalidated(), ifNoneMatch: `""`, want304: false,
+				wantCache: "public, no-cache", wantLastMod: true}),
 	)
 })