From 711153596343a8e463f4bfba16b60ba9077640ec Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Tue, 25 Jun 2024 17:14:17 -0400
Subject: [PATCH] Don't panic on PostScan errors. Fix #3118
---
persistence/playlist_repository.go | 6 +++++-
persistence/playlist_repository_test.go | 27 +++++++++++++++++++------
persistence/sql_base_repository.go | 5 ++++-
3 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go
index feafc844c..1cbc9fc65 100644
--- a/persistence/playlist_repository.go
+++ b/persistence/playlist_repository.go
@@ -5,6 +5,7 @@ import (
"database/sql"
"encoding/json"
"errors"
+ "fmt"
"time"
. "github.com/Masterminds/squirrel"
@@ -37,7 +38,10 @@ func (p dbPlaylist) PostMapArgs(args map[string]any) error {
var err error
if p.Playlist.IsSmartPlaylist() {
args["rules"], err = json.Marshal(p.Playlist.Rules)
- return err
+ if err != nil {
+ return fmt.Errorf("invalid criteria expression: %w", err)
+ }
+ return nil
}
delete(args, "rules")
return nil
diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go
index 863721670..8391f5c39 100644
--- a/persistence/playlist_repository_test.go
+++ b/persistence/playlist_repository_test.go
@@ -120,13 +120,28 @@ var _ = Describe("PlaylistRepository", func() {
},
}
})
- It("Put/Get", func() {
- newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
- Expect(repo.Put(&newPls)).To(Succeed())
+ Context("valid rules", func() {
+ Specify("Put/Get", func() {
+ newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
+ Expect(repo.Put(&newPls)).To(Succeed())
- savedPls, err := repo.Get(newPls.ID)
- Expect(err).ToNot(HaveOccurred())
- Expect(savedPls.Rules).To(Equal(rules))
+ savedPls, err := repo.Get(newPls.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(savedPls.Rules).To(Equal(rules))
+ })
+ })
+
+ Context("invalid rules", func() {
+ It("fails to Put it in the DB", func() {
+ rules = &criteria.Criteria{
+ // This is invalid because "contains" cannot have multiple fields
+ Expression: criteria.All{
+ criteria.Contains{"genre": "Hardcore", "filetype": "mp3"},
+ },
+ }
+ newPls := model.Playlist{Name: "Great!", OwnerID: "userid", Rules: rules}
+ Expect(repo.Put(&newPls)).To(MatchError(ContainSubstring("invalid criteria expression")))
+ })
})
})
})
diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go
index 67649c4dc..2df360a08 100644
--- a/persistence/sql_base_repository.go
+++ b/persistence/sql_base_repository.go
@@ -275,7 +275,10 @@ func (r sqlRepository) count(countQuery SelectBuilder, options ...model.QueryOpt
}
func (r sqlRepository) put(id string, m interface{}, colsToUpdate ...string) (newId string, err error) {
- values, _ := toSQLArgs(m)
+ values, err := toSQLArgs(m)
+ if err != nil {
+ return "", fmt.Errorf("error preparing values to write to DB: %w", err)
+ }
// If there's an ID, try to update first
if id != "" {
updateValues := map[string]interface{}{}