diff --git a/core/artwork/blurhash_updater.go b/core/artwork/blurhash_updater.go index 37a39fdd9..224e2e087 100644 --- a/core/artwork/blurhash_updater.go +++ b/core/artwork/blurhash_updater.go @@ -171,6 +171,14 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re log.Trace(ctx, "BlurHash: could not load entity", "artID", artID, err) return } + // A future-dated artwork file mtime (clock skew, a future-stamped file) would otherwise be + // persisted verbatim and, via the !Before checks here and in the DTO, pin the stored hash until + // wall time caught up. Cap the snapshot at now so a later real change always moves past it. + now := time.Now() + snapshot := req.snapshot + if snapshot.After(now) { + snapshot = now + } if req.gone { // Only the failed serve witnesses a deletion; clear a stored hash so the DTO falls back to // the rotating fake. An empty hash means there is nothing to clear. @@ -183,7 +191,7 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re } // snapshot folds row timestamps and file mtimes into one clock; a stored hash at or after it is // already current. This is the only freshness comparison the fill trigger needs. - if stored != "" && storedAt != nil && !storedAt.Before(req.snapshot) { + if stored != "" && storedAt != nil && !storedAt.Before(snapshot) { return } hash, err := u.computeFromArtwork(ctx, artID) @@ -197,7 +205,7 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re // An empty hash means the served bytes are a placeholder: the cover is gone. Clear a stored // hash so the DTO stops describing artwork no longer served. if stored != "" { - if err := u.persist(ctx, artID, "", req.snapshot); err != nil { + if err := u.persist(ctx, artID, "", snapshot); err != nil { log.Warn(ctx, "BlurHash: error clearing stale hash", "artID", artID, err) } } @@ -205,10 +213,10 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re } // Unchanged hash with an unmoved snapshot needs no write — keeps cache-disabled installs (which // fill on every original serve) from hammering the DB. - if hash == stored && storedAt != nil && !req.snapshot.After(*storedAt) { + if hash == stored && storedAt != nil && !snapshot.After(*storedAt) { return } - if err := u.persist(ctx, artID, hash, req.snapshot); err != nil { + if err := u.persist(ctx, artID, hash, snapshot); err != nil { log.Warn(ctx, "BlurHash: error persisting", "artID", artID, err) } } diff --git a/core/artwork/e2e/blurhash_test.go b/core/artwork/e2e/blurhash_test.go index e92efee86..2dbf8da00 100644 --- a/core/artwork/e2e/blurhash_test.go +++ b/core/artwork/e2e/blurhash_test.go @@ -2,6 +2,7 @@ package artworke2e_test import ( "testing/fstest" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -35,6 +36,28 @@ var _ = Describe("BlurHash", func() { }, "10s", "100ms").Should(Succeed()) }) + It("does not persist a future-dated blurhash timestamp", func() { + cover := realPNG("future-cover") + cover.ModTime = time.Now().Add(500 * time.Hour) // clock skew / future-stamped file + setLayout(fstest.MapFS{ + "Artist/Album/01 - Song.mp3": trackFile(1, "Song"), + "Artist/Album/cover.png": cover, + }) + scan() + al := firstAlbum() + readArtwork(al.CoverArtID()) + + Eventually(func(g Gomega) { + updated, err := ds.Album(ctx).Get(al.ID) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(updated.BlurHash).ToNot(BeEmpty()) + g.Expect(updated.BlurHashUpdatedAt).ToNot(BeNil()) + // A future file mtime must be capped at now, or the !Before checks would pin the hash + // (and the client's cover cache) until wall time caught up. + g.Expect(updated.BlurHashUpdatedAt.After(time.Now())).To(BeFalse()) + }, "10s", "100ms").Should(Succeed()) + }) + It("recomputes when the cover is swapped in place", func() { setLayout(fstest.MapFS{ "Artist/Album/01 - Song.mp3": trackFile(1, "Song"),