From b5147f166dd12ed24174a41aaa1516699efef26d Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 19 Sep 2026 17:39:17 -0400 Subject: [PATCH] fix(smartplaylists): stop infinite recursion on playlists that reference each other Two smart playlists referencing each other, by id or by path, recursed until the stack overflowed and the server died. The refresh now tracks visited playlists. --- persistence/smart_playlist_repository.go | 17 +++++++++++--- persistence/smart_playlist_repository_test.go | 22 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/persistence/smart_playlist_repository.go b/persistence/smart_playlist_repository.go index b6042739c..eec2aecbd 100644 --- a/persistence/smart_playlist_repository.go +++ b/persistence/smart_playlist_repository.go @@ -17,6 +17,17 @@ import ( // refreshSmartPlaylist evaluates the criteria of a smart playlist and updates its tracks accordingly. func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { + return r.refreshSmartPlaylistTree(pls, map[string]struct{}{}) +} + +// The visited set stops playlists that reference each other from recursing forever. +func (r *playlistRepository) refreshSmartPlaylistTree(pls *model.Playlist, visited map[string]struct{}) bool { + if _, seen := visited[pls.ID]; seen { + log.Trace(r.ctx, "Skipping already visited smart playlist", "playlist", pls.Name, "id", pls.ID) + return false + } + visited[pls.ID] = struct{}{} + usr := loggedUser(r.ctx) if !r.shouldRefreshSmartPlaylist(pls, usr) { return false @@ -34,7 +45,7 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { normalisedPls := pls.WithNormalizeChildPaths() rulesSQL := newSmartPlaylistCriteria(*normalisedPls.Rules, withSmartPlaylistOwner(*usr)) - if !r.refreshChildPlaylists(&normalisedPls, rulesSQL) { + if !r.refreshChildPlaylists(&normalisedPls, rulesSQL, visited) { return false } @@ -91,7 +102,7 @@ func (r *playlistRepository) shouldRefreshSmartPlaylist(pls *model.Playlist, usr // refreshChildPlaylists handles refreshing any child playlists that are referenced in the smart playlist criteria. // Returns false if child playlists could not be loaded (DB error), signaling the parent refresh should abort. -func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL smartPlaylistCriteria) bool { +func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL smartPlaylistCriteria, visited map[string]struct{}) bool { childPlaylistIds := rulesSQL.ChildPlaylistIds() childPlaylistPaths := rulesSQL.ChildPlaylistPaths() if len(childPlaylistIds) == 0 && len(childPlaylistPaths) == 0 { @@ -122,7 +133,7 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL if childPlaylists[i].Path != "" { found[norm.NFC.String(childPlaylists[i].Path)] = struct{}{} } - r.refreshSmartPlaylist(&childPlaylists[i]) + r.refreshSmartPlaylistTree(&childPlaylists[i], visited) } for _, id := range childPlaylistIds { if _, ok := found[id]; !ok { diff --git a/persistence/smart_playlist_repository_test.go b/persistence/smart_playlist_repository_test.go index a12d6b077..f6d09a5fc 100644 --- a/persistence/smart_playlist_repository_test.go +++ b/persistence/smart_playlist_repository_test.go @@ -174,6 +174,28 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() { }) }) + It("does not recurse forever when two smart playlists reference each other", func() { + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + + plsA := model.Playlist{Name: "Cycle A", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{ + Expression: criteria.All{criteria.Contains{"title": "Day"}}, + }} + Expect(repo.Put(&plsA)).To(Succeed()) + DeferCleanup(func() { _ = repo.Delete(plsA.ID) }) + + plsB := model.Playlist{Name: "Cycle B", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{ + Expression: criteria.All{criteria.InPlaylist{"id": plsA.ID}}, + }} + Expect(repo.Put(&plsB)).To(Succeed()) + DeferCleanup(func() { _ = repo.Delete(plsB.ID) }) + + plsA.Rules = &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"id": plsB.ID}}} + Expect(repo.Put(&plsA)).To(Succeed()) + + _, err := repo.GetWithTracks(plsA.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + }) + It("does not treat an empty path as a reference to every playlist without a path", func() { conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second