From e25071f87e3d744bb19fda2ec253f9bc13c51b0f Mon Sep 17 00:00:00 2001 From: hiyukoim <11803292+hiyukoim@users.noreply.github.com> Date: Tue, 7 Jul 2026 19:25:18 +0200 Subject: [PATCH] fix(radio): address ICY parser review feedback Signed-off-by: hiyukoim <11803292+hiyukoim@users.noreply.github.com> --- core/radio/icy/parser.go | 22 +++++++++++++++------- core/radio/icy/parser_test.go | 16 ++++++++++++++++ core/radio/provider.go | 2 +- core/radio/provider_test.go | 20 ++++++++++++++++++++ 4 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 core/radio/provider_test.go diff --git a/core/radio/icy/parser.go b/core/radio/icy/parser.go index 1ab7b0898..39371dfca 100644 --- a/core/radio/icy/parser.go +++ b/core/radio/icy/parser.go @@ -17,17 +17,22 @@ var ( errEndOfStream = errors.New("icy stream ended") ) +const ( + maxMetaInt = 1024 * 1024 + maxMetadataLen = 255 * 16 +) + // ReadStreamTitles reads ICY metadata blocks from r and emits changed StreamTitle values. func ReadStreamTitles(ctx context.Context, r io.Reader, metaInt int, handleTitle func(string)) error { - if metaInt <= 0 { + if metaInt <= 0 || metaInt > maxMetaInt { return ErrInvalidMetaInt } if handleTitle == nil { return ErrMissingTitleHandler } - audio := make([]byte, metaInt) length := make([]byte, 1) + metadataBuf := make([]byte, maxMetadataLen) lastTitle := "" for { @@ -35,8 +40,8 @@ func ReadStreamTitles(ctx context.Context, r io.Reader, metaInt int, handleTitle return err } - if err := readFullOrEnd(r, audio); err != nil { - if errors.Is(err, errEndOfStream) { + if _, err := io.CopyN(io.Discard, r, int64(metaInt)); err != nil { + if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { return nil } return err @@ -58,7 +63,7 @@ func ReadStreamTitles(ctx context.Context, r io.Reader, metaInt int, handleTitle continue } - metadata := make([]byte, metadataLen) + metadata := metadataBuf[:metadataLen] if err := readFullOrEnd(r, metadata); err != nil { if errors.Is(err, errEndOfStream) { return nil @@ -96,9 +101,12 @@ func streamTitle(metadata []byte) string { } start += len(prefix) - end := strings.IndexByte(text[start:], '\'') + end := strings.Index(text[start:], "';") if end < 0 { - return "" + end = strings.IndexByte(text[start:], '\'') + if end < 0 { + return "" + } } return strings.TrimSpace(text[start : start+end]) diff --git a/core/radio/icy/parser_test.go b/core/radio/icy/parser_test.go index 0170bbb96..1877c2baa 100644 --- a/core/radio/icy/parser_test.go +++ b/core/radio/icy/parser_test.go @@ -73,6 +73,22 @@ var _ = Describe("ReadStreamTitles", func() { Expect(titles).To(BeEmpty()) }) + It("returns an error for huge metadata intervals", func() { + titles, err := readTitles(context.Background(), nil, maxMetaInt+1) + + Expect(err).To(MatchError(ErrInvalidMetaInt)) + Expect(titles).To(BeEmpty()) + }) + + It("keeps single quotes inside StreamTitle values", func() { + stream := icyStream(metaInt, "StreamTitle='L'Oiseau - Track';") + + titles, err := readTitles(context.Background(), stream, metaInt) + + Expect(err).ToNot(HaveOccurred()) + Expect(titles).To(Equal([]string{"L'Oiseau - Track"})) + }) + It("stops when context is cancelled", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() diff --git a/core/radio/provider.go b/core/radio/provider.go index f67e535fe..8a2558fab 100644 --- a/core/radio/provider.go +++ b/core/radio/provider.go @@ -35,7 +35,7 @@ func NewMetadataManagerService(publisher TitlePublisher) *MetadataManager { // ICY metadata or a client-error HTTP status — so the reader backs off for // much longer instead of hammering the station. func classifyICYError(err error) error { - if errors.Is(err, icy.ErrMissingMetaInt) { + if errors.Is(err, icy.ErrMissingMetaInt) || errors.Is(err, icy.ErrInvalidMetaInt) { return MarkPermanent(err) } var statusErr *icy.ResponseStatusError diff --git a/core/radio/provider_test.go b/core/radio/provider_test.go new file mode 100644 index 000000000..f73ef770b --- /dev/null +++ b/core/radio/provider_test.go @@ -0,0 +1,20 @@ +package radio + +import ( + "fmt" + + "github.com/navidrome/navidrome/core/radio/icy" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Metadata provider", func() { + Describe("classifyICYError", func() { + It("marks invalid metadata intervals as permanent", func() { + err := classifyICYError(fmt.Errorf("%w: %q", icy.ErrInvalidMetaInt, "999999999")) + + Expect(isPermanent(err)).To(BeTrue()) + Expect(err).To(MatchError(ContainSubstring(icy.ErrInvalidMetaInt.Error()))) + }) + }) +})