From bf7ae5e82e44c2d8cd1c807295a7dfb9d12df421 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 15:51:01 -0400 Subject: [PATCH] fix(artwork): cap the blurhash snapshot at now to survive future mtimes A future-dated artwork file mtime (clock skew, or a file stamped ahead of the server clock) flowed into the reader's LastUpdated snapshot and was persisted verbatim as blur_hash_updated_at. Both the worker's freshness guard and the Jellyfin DTO use a !Before comparison against that timestamp, so a later legitimate cover change whose row/mtime clock was still behind the future value would be skipped, pinning the stored hash (and the client's cached cover) until wall time caught up. Cap the snapshot at time.Now() in process() before every freshness comparison and persist, so a real later change always advances past it. Normal past mtimes (the legitimate 'snapshot exceeds row version' case) are unaffected. --- core/artwork/blurhash_updater.go | 16 ++++++++++++---- core/artwork/e2e/blurhash_test.go | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) 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"),