fix: handle URL parsing errors and use atomic upsert in plugin repository

Added proper error handling for url.Parse calls in PublicURL and AbsoluteURL
functions. When parsing fails, PublicURL now falls back to AbsoluteURL, and
AbsoluteURL logs the error and returns an empty string, preventing malformed
URLs from being generated.

Replaced the non-atomic UPDATE-then-INSERT pattern in plugin repository Put
method with a single atomic INSERT ... ON CONFLICT statement. This eliminates
potential race conditions and improves consistency with the upsert pattern
already used in host_kvstore.go.
This commit is contained in:
Deluan 2025-12-31 15:25:14 -05:00
parent ac3889cac9
commit a427166084
2 changed files with 45 additions and 27 deletions

View File

@ -11,6 +11,7 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
)
@ -31,17 +32,23 @@ func ImageURL(req *http.Request, artID model.ArtworkID, size int) string {
// the scheme and host from the provided http.Request.
// If req is nil and ShareURL is not set, it defaults to http://localhost.
func PublicURL(req *http.Request, u string, params url.Values) string {
if conf.Server.ShareURL != "" {
shareUrl, _ := url.Parse(conf.Server.ShareURL)
buildUrl, _ := url.Parse(u)
buildUrl.Scheme = shareUrl.Scheme
buildUrl.Host = shareUrl.Host
if len(params) > 0 {
buildUrl.RawQuery = params.Encode()
}
return buildUrl.String()
if conf.Server.ShareURL == "" {
return AbsoluteURL(req, u, params)
}
return AbsoluteURL(req, u, params)
shareUrl, err := url.Parse(conf.Server.ShareURL)
if err != nil {
return AbsoluteURL(req, u, params)
}
buildUrl, err := url.Parse(u)
if err != nil {
return AbsoluteURL(req, u, params)
}
buildUrl.Scheme = shareUrl.Scheme
buildUrl.Host = shareUrl.Host
if len(params) > 0 {
buildUrl.RawQuery = params.Encode()
}
return buildUrl.String()
}
// AbsoluteURL builds an absolute URL from a relative path.
@ -49,7 +56,11 @@ func PublicURL(req *http.Request, u string, params url.Values) string {
// the scheme and host from the http.Request.
// If req is nil and BaseHost is not set, it defaults to http://localhost.
func AbsoluteURL(req *http.Request, u string, params url.Values) string {
buildUrl, _ := url.Parse(u)
buildUrl, err := url.Parse(u)
if err != nil {
log.Error(req.Context(), "Failed to parse URL path", "url", u, err)
return ""
}
if strings.HasPrefix(u, "/") {
buildUrl.Path = path.Join(conf.Server.BasePath, buildUrl.Path)
if conf.Server.BaseHost != "" {

View File

@ -77,22 +77,29 @@ func (r *pluginRepository) Put(plugin *model.Plugin) error {
return errors.New("plugin ID cannot be empty")
}
// Try update first
values, _ := toSQLArgs(*plugin)
update := Update(r.tableName).Where(Eq{"id": plugin.ID}).SetMap(values)
count, err := r.executeSQL(update)
if err != nil {
return err
}
if count > 0 {
return nil
}
// Insert if not exists
plugin.CreatedAt = time.Now()
values, _ = toSQLArgs(*plugin)
insert := Insert(r.tableName).SetMap(values)
_, err = r.executeSQL(insert)
// Upsert using INSERT ... ON CONFLICT for atomic operation
_, err := r.db.NewQuery(`
INSERT INTO plugin (id, path, manifest, config, enabled, last_error, sha256, created_at, updated_at)
VALUES ({:id}, {:path}, {:manifest}, {:config}, {:enabled}, {:last_error}, {:sha256}, {:created_at}, {:updated_at})
ON CONFLICT(id) DO UPDATE SET
path = excluded.path,
manifest = excluded.manifest,
config = excluded.config,
enabled = excluded.enabled,
last_error = excluded.last_error,
sha256 = excluded.sha256,
updated_at = excluded.updated_at
`).Bind(dbx.Params{
"id": plugin.ID,
"path": plugin.Path,
"manifest": plugin.Manifest,
"config": plugin.Config,
"enabled": plugin.Enabled,
"last_error": plugin.LastError,
"sha256": plugin.SHA256,
"created_at": time.Now(),
"updated_at": plugin.UpdatedAt,
}).Execute()
return err
}