fix(lyrics): distinguish malformed structured sources

- stop malformed structured content from becoming plain lyrics
- let source resolution continue to valid lower-priority lyrics

Signed-off-by: ranokay <github@ranokay.com>

core/lyrics/lyrics_test.go

@@ -230,7 +230,7 @@ var _ = Describe("Lyrics", func() {
 		}))
 	})
 
-	It("returns a non-Lyricsfile YAML sidecar as plain text, shadowing lower-priority sources", func() {
+	It("skips a non-Lyricsfile YAML sidecar and resolves the next source", func() {
 		dir, err := os.MkdirTemp("", "lyrics-yaml-fallback-*")
 		Expect(err).ToNot(HaveOccurred())
 		DeferCleanup(func() {
@@ -247,14 +247,11 @@ var _ = Describe("Lyrics", func() {
 			Path:        "song.mp3",
 		})
 
-		// ParseLyrics falls back to plain text for any suffix when the content
-		// doesn't match the structured format, so the .yaml hit is non-empty and
-		// shadows the lower-priority .lrc entirely.
 		Expect(err).To(BeNil())
 		Expect(list).To(HaveLen(1))
-		Expect(list[0].Synced).To(BeFalse())
+		Expect(list[0].Synced).To(BeTrue())
 		Expect(list[0].Line).To(Equal([]model.Line{
-			{Value: "title: not lyricsfile"},
+			{Start: new(int64(1000)), Value: "Fallback line"},
 		}))
 	})
 

model/lyrics_lyricsfile.go

@@ -71,6 +71,16 @@ func parseLyricsfile(lang string, contents []byte) (LyricList, error) {
 
 const lyricsfileVersion = "1.0"
 
+func hasLyricsfileVersion(contents []byte) bool {
+	var header struct {
+		Version string `yaml:"version"`
+	}
+	if err := yaml.Unmarshal(contents, &header); err != nil {
+		return false
+	}
+	return strings.TrimSpace(header.Version) == lyricsfileVersion
+}
+
 type lyricsfileDocument struct {
 	Version  string                `yaml:"version"`
 	Metadata lyricsfileMetadata    `yaml:"metadata"`

model/lyrics_parse.go

@@ -3,32 +3,46 @@ package model
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
+	"regexp"
 	"slices"
 	"strings"
 
 	"github.com/navidrome/navidrome/log"
 )
 
-// lyricParser returns an empty list (not an error) when the input is not its
-// format, so parsers can be tried in order. lang is the default for formats that
-// do not carry their own.
+// lyricParser parses content already claimed by its format. A nil list with no
+// error means a recognized, valid document that contains no lyrics.
 type lyricParser func(lang string, contents []byte) (LyricList, error)
 
+var errLyricsFormatMismatch = errors.New("lyrics format mismatch")
+
+type lyricFormat struct {
+	name     string
+	suffixes []string
+	claims   func([]byte) bool
+	parse    lyricParser
+}
+
 // lyricFormats is the structured formats in content-sniff probe order; each
 // row's suffixes drive sidecar dispatch. LRC/plain is the unlisted fallback floor.
-var lyricFormats = []struct {
-	suffixes []string
-	parse    lyricParser
-}{
-	{[]string{".ttml"}, parseTTML},
-	{[]string{".srt"}, parseSRT},
-	{[]string{".yaml", ".yml"}, parseLyricsfile},
+var lyricFormats = []lyricFormat{
+	{name: "TTML", suffixes: []string{".ttml"}, claims: claimsTTML, parse: parseTTML},
+	{name: "SRT", suffixes: []string{".srt"}, claims: claimsSRT, parse: parseSRT},
+	{name: "Lyricsfile", suffixes: []string{".yaml", ".yml"}, claims: claimsLyricsfile, parse: parseLyricsfile},
 }
 
+var (
+	ttmlRootPrefixRegex = regexp.MustCompile(`(?is)^\s*(?:<\?xml\b[^>]*\?>\s*)?(?:<!--.*?-->\s*)*<(?:[[:alpha:]_][[:alnum:]_.-]*:)?tt(?:\s|/?>)`)
+	srtClaimRegex       = regexp.MustCompile(`(?m)^\s*(?:\d+\s*\n\s*)?\d{1,2}:\d{2}:\d{2}[,.]\d{1,3}\s*-->\s*\d{1,2}:\d{2}:\d{2}[,.]\d{1,3}(?:\s|$)`)
+	lyricsfileRegex     = regexp.MustCompile(`(?mi)^\s*["']?version["']?\s*:\s*["']?1\.0["']?\s*(?:#.*)?$`)
+)
+
 // ParseLyrics is the single entry point for parsing lyrics. A known suffix routes
-// to that format's parser; an empty or "auto" suffix content-sniffs. Either way,
-// a structured parser that does not match falls back to the LRC/plain-text floor.
+// to that format's parser; an empty or "auto" suffix content-sniffs. Explicit
+// structured suffixes are strict: malformed or mismatched structured content is
+// returned as an error so a source resolver can continue to its next source.
 //
 // Parse failures are logged through ctx; callers that know the source should
 // attach it for attribution, e.g. log.NewContext(ctx, "file", path).
@@ -37,37 +51,80 @@ func ParseLyrics(ctx context.Context, suffix, lang string, contents []byte) (Lyr
 	suffix = strings.ToLower(suffix)
 	sniff := suffix == "" || suffix == "auto"
 
-	// Sniffing tries every format in order; a known suffix selects just its own.
-	// Unmatched suffixes leave no candidates, so parseFirstMatch falls to plain.
-	candidates := make([]lyricParser, 0, len(lyricFormats))
+	// Sniffing tries every structured format in order. A known structured suffix
+	// selects one strict parser; LRC/text and unknown textual suffixes retain the
+	// longstanding LRC/plain fallback.
+	candidates := make([]lyricFormat, 0, len(lyricFormats))
 	for _, f := range lyricFormats {
 		if sniff || slices.Contains(f.suffixes, suffix) {
-			candidates = append(candidates, f.parse)
+			candidates = append(candidates, f)
 		}
 	}
-	return parseFirstMatch(ctx, sniff, lang, contents, candidates...)
-}
 
-func parseFirstMatch(ctx context.Context, sniff bool, lang string, contents []byte, candidates ...lyricParser) (LyricList, error) {
-	for _, parse := range candidates {
-		list, err := parse(lang, contents)
-		if err == nil && len(list) > 0 {
-			return list, nil
+	if !sniff && len(candidates) > 0 {
+		format := candidates[0]
+		list, err := parseClaimedLyrics(format, lang, contents)
+		if errors.Is(err, errLyricsFormatMismatch) {
+			err = fmt.Errorf("declared %s lyrics do not match the format: %w", format.name, err)
 		}
 		if err != nil {
-			// While sniffing, a probe rejecting content it does not own is expected
-			// control flow, so keep it at trace. A failure under an explicit suffix
-			// means the declared format is malformed and deserves a warning.
-			if sniff {
-				log.Trace(ctx, "Lyrics probe did not match, trying next format", err)
-			} else {
-				log.Warn(ctx, "Error parsing lyrics, falling back to plain text", err)
-			}
+			log.Warn(ctx, "Error parsing declared lyrics", "format", format.name, err)
 		}
+		return list, err
+	}
+
+	return parseFirstMatch(ctx, lang, contents, candidates...)
+}
+
+func parseFirstMatch(ctx context.Context, lang string, contents []byte, candidates ...lyricFormat) (LyricList, error) {
+	for _, format := range candidates {
+		list, err := parseClaimedLyrics(format, lang, contents)
+		if errors.Is(err, errLyricsFormatMismatch) {
+			log.Trace(ctx, "Lyrics probe did not match, trying next format", "format", format.name)
+			continue
+		}
+		if err != nil {
+			return nil, fmt.Errorf("parsing claimed %s lyrics: %w", format.name, err)
+		}
+		// A claimed, valid-empty document owns the content and deliberately stops
+		// sniffing instead of being reinterpreted as another format or plain text.
+		return list, nil
 	}
 	return plainLRC(lang, contents)
 }
 
+func parseClaimedLyrics(format lyricFormat, lang string, contents []byte) (LyricList, error) {
+	if !format.claims(contents) {
+		return nil, errLyricsFormatMismatch
+	}
+	list, err := format.parse(lang, contents)
+	if err != nil {
+		return nil, err
+	}
+	return list, nil
+}
+
+func claimsTTML(contents []byte) bool {
+	trimmed := bytes.TrimSpace(contents)
+	if len(trimmed) == 0 || trimmed[0] != '<' {
+		return false
+	}
+	return isTTMLDocument(contents) || ttmlRootPrefixRegex.Match(contents)
+}
+
+func claimsSRT(contents []byte) bool {
+	raw := bytes.ReplaceAll(contents, []byte("\r\n"), []byte("\n"))
+	raw = bytes.ReplaceAll(raw, []byte("\r"), []byte("\n"))
+	return srtClaimRegex.Match(raw)
+}
+
+func claimsLyricsfile(contents []byte) bool {
+	if lyricsfileRegex.Match(contents) {
+		return true
+	}
+	return hasLyricsfileVersion(contents)
+}
+
 func plainLRC(lang string, contents []byte) (LyricList, error) {
 	lyric, err := parseLRC(lang, string(contents))
 	if err != nil {

model/lyrics_parse_test.go

@@ -1,8 +1,6 @@
 package model
 
 import (
-	"strings"
-
 	"github.com/navidrome/navidrome/log"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -34,6 +32,19 @@ var _ = Describe("ParseLyrics", func() {
 		Expect(list[0].Line[0].Value).To(Equal("auto ttml"))
 	})
 
+	DescribeTable("accepts valid TTML XML prologs",
+		func(suffix, prolog string) {
+			contents := prolog + `<tt xmlns="http://www.w3.org/ns/ttml"><body><div><p begin="00:00.000" end="00:01.000">prolog ttml</p></div></body></tt>`
+			list, err := ParseLyrics(GinkgoT().Context(), suffix, "eng", []byte(contents))
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(list).To(HaveLen(1))
+			Expect(list[0].Line[0].Value).To(Equal("prolog ttml"))
+		},
+		Entry("explicit suffix with processing instruction", ".ttml", `<?xml version="1.0"?><?lyrics source="test"?>`),
+		Entry("content sniffing with doctype", "", `<!DOCTYPE tt>`),
+	)
+
 	It("empty suffix content-sniffs (YAML)", func() {
 		yaml := "version: \"1.0\"\nmetadata:\n  language: eng\nlines:\n  - text: auto yaml\n    start_ms: 1000\n"
 		list, err := ParseLyrics(GinkgoT().Context(), "auto", "eng", []byte(yaml))
@@ -42,14 +53,62 @@ var _ = Describe("ParseLyrics", func() {
 		Expect(list[0].Line[0].Value).To(Equal("auto yaml"))
 	})
 
-	It("falls back to plain text when a known suffix fails to parse structurally", func() {
+	DescribeTable("accepts quoted Lyricsfile version keys",
+		func(suffix string) {
+			contents := "\"version\": \"1.0\"\nmetadata:\n  language: eng\nlines:\n  - text: quoted version\n    start_ms: 1000\n"
+			list, err := ParseLyrics(GinkgoT().Context(), suffix, "eng", []byte(contents))
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(list).To(HaveLen(1))
+			Expect(list[0].Line[0].Value).To(Equal("quoted version"))
+		},
+		Entry("explicit YAML", ".yaml"),
+		Entry("content sniffing", "auto"),
+	)
+
+	It("returns an error when an explicit structured suffix does not match", func() {
 		list, err := ParseLyrics(GinkgoT().Context(), ".srt", "eng", []byte("not actually an srt file"))
+		Expect(err).To(MatchError(ContainSubstring("declared SRT lyrics do not match")))
+		Expect(list).To(BeNil())
+	})
+
+	It("retains LRC/plain fallback for textual suffixes", func() {
+		list, err := ParseLyrics(GinkgoT().Context(), ".lrc", "eng", []byte("not actually timed"))
 		Expect(err).ToNot(HaveOccurred())
 		Expect(list).To(HaveLen(1))
 		Expect(list[0].Synced).To(BeFalse())
-		Expect(list[0].Line[0].Value).To(Equal("not actually an srt file"))
+		Expect(list[0].Line[0].Value).To(Equal("not actually timed"))
 	})
 
+	DescribeTable("recognized valid-empty structured documents stop sniffing",
+		func(contents string) {
+			list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(contents))
+			Expect(err).ToNot(HaveOccurred())
+			Expect(list).To(BeEmpty())
+		},
+		Entry("TTML", `<tt xmlns="http://www.w3.org/ns/ttml"><body /></tt>`),
+		Entry("SRT", "1\n00:00:01,000 --> 00:00:02,000\n"),
+		Entry("Lyricsfile", "version: \"1.0\"\nmetadata:\n  language: eng\n"),
+	)
+
+	It("does not reinterpret malformed claimed structured content as plain text", func() {
+		list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(`<tt><body><p>Broken`))
+		Expect(err).To(MatchError(ContainSubstring("parsing claimed TTML lyrics")))
+		Expect(list).To(BeNil())
+	})
+
+	DescribeTable("generic content remains eligible for plain fallback",
+		func(contents string) {
+			list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(contents))
+			Expect(err).ToNot(HaveOccurred())
+			Expect(list).To(HaveLen(1))
+			Expect(list[0].Synced).To(BeFalse())
+		},
+		Entry("prose", "A plain lyric"),
+		Entry("generic XML", "<lyrics>A generic lyric</lyrics>"),
+		Entry("generic YAML", "title: A generic lyric"),
+	)
+
 	Describe("logging on parser probe failures", func() {
 		var hook *test.Hook
 
@@ -92,10 +151,10 @@ var _ = Describe("ParseLyrics", func() {
 		// failure is worth surfacing loudly — and it must name the file.
 		It("warns and names the file when a requested suffix fails to parse", func() {
 			ctx := log.NewContext(GinkgoT().Context(), "file", "/music/song.yaml")
-			list, err := ParseLyrics(ctx, ".yaml", "eng", []byte("not: [valid, yaml\n"))
+			list, err := ParseLyrics(ctx, ".yaml", "eng", []byte("version: \"1.0\"\nnot: [valid, yaml\n"))
 
-			Expect(err).ToNot(HaveOccurred())
-			Expect(list).To(HaveLen(1)) // still falls back to plain text
+			Expect(err).To(HaveOccurred())
+			Expect(list).To(BeNil())
 			entry := hook.LastEntry()
 			Expect(entry).ToNot(BeNil())
 			Expect(entry.Level).To(Equal(logrus.WarnLevel))
@@ -234,25 +293,13 @@ Another subtitle line`
 		Expect(list[0].Line[0].Cue).To(HaveLen(2))
 	})
 
-	It("should fall back to plain lyrics when embedded TTML is invalid", func() {
-		content := `<tt xmlns="http://www.w3.org/ns/ttml">
-  <body>
-    <p begin="not-a-time">Broken</p>
-  </body>
-</tt>`
+	It("should reject malformed embedded TTML instead of returning raw markup", func() {
+		content := `<tt xmlns="http://www.w3.org/ns/ttml"><body><p>Broken`
 
 		list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(content))
 
-		Expect(err).ToNot(HaveOccurred())
-		Expect(list).To(HaveLen(1))
-		Expect(list[0].Lang).To(Equal("eng"))
-		Expect(list[0].Synced).To(BeFalse())
-		Expect(list[0].Line).ToNot(BeEmpty())
-		values := make([]string, 0, len(list[0].Line))
-		for _, line := range list[0].Line {
-			values = append(values, line.Value)
-		}
-		Expect(strings.Join(values, "\n")).To(ContainSubstring("Broken"))
+		Expect(err).To(HaveOccurred())
+		Expect(list).To(BeNil())
 	})
 
 	It("detects a Lyricsfile YAML payload via content-sniffing", func() {

model/lyrics_ttml.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/xml"
 	"errors"
+	"fmt"
 	"io"
 	"math"
 	"regexp"
@@ -123,6 +124,9 @@ func parseTTML(defaultLang string, contents []byte) (LyricList, error) {
 	// text — isTTMLDocument does a cheap decode that stops at the first element.
 	// Checked after the encoding fixup so UTF-16-declared documents are recognized.
 	if !isTTMLDocument(contents) {
+		if claimsTTML(contents) {
+			return nil, fmt.Errorf("malformed TTML document")
+		}
 		return nil, nil
 	}
 

plugins/lyrics_adapter_test.go

@@ -150,6 +150,22 @@ var _ = Describe("LyricsPlugin", Ordered, func() {
 			Entry("lrc", "lrc", true, "plugin lrc line"),
 			Entry("plain", "plain", false, "plugin plain line"),
 		)
+
+		It("keeps valid entries when another plugin lyric is malformed", func() {
+			manager, _ := createTestManagerWithPlugins(map[string]map[string]string{
+				"test-lyrics": {"format": "mixed"},
+			}, "test-lyrics"+PackageExtension)
+
+			p, ok := manager.LoadLyricsProvider("test-lyrics")
+			Expect(ok).To(BeTrue())
+
+			result, err := p.GetLyrics(GinkgoT().Context(), &model.MediaFile{ID: "track-1"})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(result).To(HaveLen(1))
+			Expect(result[0].Line).To(Equal([]model.Line{
+				{Start: new(int64(1000)), Value: "valid plugin line"},
+			}))
+		})
 	})
 
 	Describe("PluginNames", func() {

plugins/testdata/test-lyrics/main.go

@@ -48,6 +48,13 @@ func (t *testLyrics) GetLyrics(input lyrics.GetLyricsRequest) (lyrics.GetLyricsR
 		case "plain":
 			lang = "eng"
 			text = "plugin plain line"
+		case "mixed":
+			return lyrics.GetLyricsResponse{
+				Lyrics: []lyrics.LyricsText{
+					{Lang: "eng", Text: `<tt><body><p>malformed`},
+					{Lang: "eng", Text: "[00:01.00]valid plugin line"},
+				},
+			}, nil
 		}
 		if text != "" {
 			return lyrics.GetLyricsResponse{