From f52cc623dff6ede49f11f42596eeab4623d99f65 Mon Sep 17 00:00:00 2001 From: Steve Date: Tue, 1 Sep 2026 12:21:06 +0530 Subject: [PATCH] fix(lyrics): match external sidecar with alternate Unicode normalization - #4148 External lyrics files were resolved with a byte-exact path lookup, so a sidecar written next to a track in a different Unicode normalization form than the track's own filename was never found. macOS commonly stores names as NFD while Linux and Windows keep NFC, so a .lrc placed alongside a FLAC with non-ASCII characters (for example Japanese katakana with a dakuten) showed up as 'No lyrics' on the web. openSidecar now retries with the alternate NFC/NFD form when the exact path is missing, reusing the same approach the playlist importer already applies for cross-platform path matching. Closes #4148 Signed-off-by: Steve --- core/lyrics/lyrics_test.go | 36 ++++++++++++++++++++++++ core/lyrics/sources.go | 24 +++++++++++++++- core/lyrics/sources_test.go | 56 +++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/core/lyrics/lyrics_test.go b/core/lyrics/lyrics_test.go index b00bcd576..c9c89bc22 100644 --- a/core/lyrics/lyrics_test.go +++ b/core/lyrics/lyrics_test.go @@ -6,15 +6,19 @@ import ( "fmt" "os" "path/filepath" + "testing/fstest" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/lyrics" + "github.com/navidrome/navidrome/core/storage/storagetest" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" "github.com/navidrome/navidrome/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "golang.org/x/text/unicode/norm" ) var _ = Describe("Lyrics", func() { @@ -258,6 +262,38 @@ var _ = Describe("Lyrics", func() { })) }) + It("resolves an external sidecar whose Unicode normalization differs from the track path (issue #4148)", func() { + // Reproduces the reported case: a Japanese filename whose sidecar is stored + // in a different normalization form than the track. An in-memory FS keeps + // the mismatch byte-exact regardless of the host filesystem, which may + // normalize names on lookup and hide the bug. + const scheme = "fake-lyrics-e2e" + fsys := &storagetest.FakeFS{} + storagetest.Register(scheme, fsys) + + // "ガ" is katakana KA plus a dakuten, so its NFC and NFD forms differ. + const base = "03. 鬱P feat. 初音ミク - ガ" + modTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + fsys.SetFiles(fstest.MapFS{ + norm.NFC.String(base) + ".flac": &fstest.MapFile{Data: []byte("audio"), ModTime: modTime}, + norm.NFD.String(base) + ".lrc": &fstest.MapFile{Data: []byte("[00:18.80]We're no strangers to love"), ModTime: modTime}, + }) + + // Default priority from the bug report: ".lrc,.txt,embedded". + conf.Server.LyricsPriority = ".lrc,.txt,embedded" + svc := lyrics.NewLyrics(nil, nil) + list, err := svc.GetLyrics(ctx, &model.MediaFile{ + LibraryPath: scheme + ":///music", + Path: norm.NFC.String(base) + ".flac", + }) + + Expect(err).To(BeNil()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Synced).To(BeTrue()) + Expect(list[0].Line).To(HaveLen(1)) + Expect(list[0].Line[0].Value).To(Equal("We're no strangers to love")) + }) + Context("Errors", func() { var RegularUserContext = XContext var isRegularUser = os.Getuid() != 0 diff --git a/core/lyrics/sources.go b/core/lyrics/sources.go index 23c20122d..291d9f9e1 100644 --- a/core/lyrics/sources.go +++ b/core/lyrics/sources.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/ioutils" + "golang.org/x/text/unicode/norm" ) func fromEmbedded(ctx context.Context, mf *model.MediaFile) (model.LyricList, error) { @@ -39,7 +40,7 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) ( return nil, fmt.Errorf("opening library filesystem: %w", err) } - f, err := fsys.Open(sidecarRelPath) + f, err := openSidecar(fsys, sidecarRelPath) if errors.Is(err, fs.ErrNotExist) { log.Trace(ctx, "no lyrics found at path") return nil, nil @@ -68,6 +69,27 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) ( return list, nil } +// openSidecar opens the sidecar lyrics file, retrying with the alternate Unicode +// normalization form when the exact path is missing. A sidecar written next to a +// track can use a different form than the path stored for the media file (macOS +// commonly yields NFD, while Linux and Windows typically keep NFC), so a +// byte-exact lookup would miss a file that is really on disk. Issue #4148. +func openSidecar(fsys fs.FS, relPath string) (fs.File, error) { + f, err := fsys.Open(relPath) + if !errors.Is(err, fs.ErrNotExist) { + return f, err + } + + altPath := norm.NFD.String(relPath) + if altPath == relPath { + altPath = norm.NFC.String(relPath) + } + if altPath == relPath { + return f, err + } + return fsys.Open(altPath) +} + // fromPlugin attempts to load lyrics from a plugin with the given name. func (l *lyricsService) fromPlugin(ctx context.Context, mf *model.MediaFile, pluginName string) (model.LyricList, error) { if l.pluginLoader == nil { diff --git a/core/lyrics/sources_test.go b/core/lyrics/sources_test.go index 7c7922bfd..6ab7c89a1 100644 --- a/core/lyrics/sources_test.go +++ b/core/lyrics/sources_test.go @@ -4,10 +4,14 @@ import ( "context" "encoding/json" "path/filepath" + "testing/fstest" + "time" + "github.com/navidrome/navidrome/core/storage/storagetest" "github.com/navidrome/navidrome/model" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "golang.org/x/text/unicode/norm" ) var _ = Describe("sources", func() { @@ -150,4 +154,56 @@ var _ = Describe("sources", func() { Expect(lyrics[0].Line[1].Value).To(Equal("UTF16 line two")) }) }) + + // A sidecar written next to a track can use a different Unicode normalization + // form than the track's own filename (macOS commonly stores names as NFD, + // while Linux and Windows keep NFC). A byte-exact lookup then misses a sidecar + // that is really on disk, so the retry has to try the other form. Issue #4148. + Describe("fromExternalFile with mismatched Unicode normalization", func() { + const scheme = "fake-lyrics-norm" + // "ガ" is katakana KA plus a dakuten, so it has distinct NFC (a single + // codepoint) and NFD (base plus combining mark) forms, unlike the plain + // CJK ideographs around it. It mirrors the filename from the bug report. + const base = "03. 鬱P feat. 初音ミク - ガ" + + var fsys *storagetest.FakeFS + + BeforeEach(func() { + fsys = &storagetest.FakeFS{} + storagetest.Register(scheme, fsys) + }) + + lyricsFor := func(trackForm, sidecarForm norm.Form) (model.LyricList, error) { + trackBase := trackForm.String(base) + sidecarBase := sidecarForm.String(base) + Expect(trackBase).ToNot(Equal(sidecarBase)) + + // A non-zero ModTime is required so the FakeFS directory-timestamp + // bookkeeping settles instead of looping. + modTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + fsys.SetFiles(fstest.MapFS{ + trackBase + ".flac": &fstest.MapFile{Data: []byte("audio"), ModTime: modTime}, + sidecarBase + ".lrc": &fstest.MapFile{Data: []byte("[00:18.80]We're no strangers to love"), ModTime: modTime}, + }) + + mf := &model.MediaFile{LibraryPath: scheme + ":///music", Path: trackBase + ".flac"} + return fromExternalFile(ctx, mf, ".lrc") + } + + It("finds an NFD-encoded sidecar for an NFC track path", func() { + lyrics, err := lyricsFor(norm.NFC, norm.NFD) + + Expect(err).To(BeNil()) + Expect(lyrics).To(HaveLen(1)) + Expect(lyrics[0].Synced).To(BeTrue()) + }) + + It("finds an NFC-encoded sidecar for an NFD track path", func() { + lyrics, err := lyricsFor(norm.NFD, norm.NFC) + + Expect(err).To(BeNil()) + Expect(lyrics).To(HaveLen(1)) + Expect(lyrics[0].Synced).To(BeTrue()) + }) + }) })