nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(artwork): cap the blurhash snapshot at now to survive future mtimes

Browse Source 
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.
This commit is contained in:
Deluan 2026-07-17 15:51:01 -04:00
parent 5cd75503cb
commit bf7ae5e82e
2 changed files with 35 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
core/artwork/blurhash_updater.go
View File

@ -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)
}
}

23
core/artwork/e2e/blurhash_test.go
View File

@ -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"),