mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
fix(plugins): build public URLs on the caller's address instead of localhost (#6059)
* fix(plugins): build public URLs on the caller's address instead of localhost The artwork host service had no `*http.Request`, so it passed `nil` to `publicurl.ImageURL`. With neither `ShareURL` nor `BaseURL` configured, that produced `http://localhost/share/img/...`, which is useless to anything outside the server. The Discord Rich Presence plugin explicitly drops localhost URLs, so it fell back to the Navidrome logo instead of the real cover art. `serverAddressMiddleware` already works out the client-facing scheme and host from the `X-Forwarded-*` headers. It now also records them in the request context, and `publicurl` takes a `context.Context` instead of an `*http.Request` so any caller can reach them. Extism passes the caller's context through to host functions, so plugins invoked during a request now get a reachable URL with no configuration. Switching the parameter also removes the need for a second, parallel entry point: the package previously wanted only a scheme, a host, and a context, and took a whole request to get them. `AbsoluteURL` no longer dereferences a possibly-nil request on its parse-error path. Plugin calls that start from `context.Background()` (scheduler and websocket callbacks, the buffered scrobble drain) still fall back to localhost, since they have no request to learn from. A debug log now points at `ShareURL` when that happens. * fix(publicurl): include the configured port in the localhost fallback The last-resort fallback built `http://localhost/...`, which points at port 80 and so is unreachable for a server listening anywhere else — the default 4533 included. Use `conf.Server.Port` so a consumer on the same machine can actually fetch the URL. * fix(publicurl): use https in the localhost fallback when TLS is configured The fallback hardcoded the http scheme, so a TLS-only server with no BaseURL advertised a URL it does not answer on. Mirror the server's own switch, which requires both a certificate and a key. * refactor(publicurl): tidy the localhost fallback and its tests Use gg.If for the fallback scheme so it reads as an expression, like the BaseScheme branch above it, instead of assigning http and overwriting it. Drop two tests the ctx refactor left redundant: one asserted PublicURL "works without a request" but became a byte-identical copy of the ShareURL spec once the *http.Request parameter went away, and the two port specs differed only in the integer, where the non-default port is the stronger assertion. * refactor(conf): add TLSEnabled and use it instead of repeating the predicate Whether the server speaks HTTPS was decided inline in three unconnected places. This PR added the third, in a URL-building package that has no business inferring the transport config. Move the rule to conf, next to the fields it derives from, and call it from publicurl and the insights collector. server.Run keeps its own expression: it takes the certificate and key as parameters, and its test passes values that do not come from the config.
This commit is contained in:
parent
96b051ffa7
commit
1f861d27ef
17 changed files with 245 additions and 85 deletions
|
|
@ -315,6 +315,12 @@ var currentGOOS = func() string {
|
|||
return runtime.GOOS
|
||||
}
|
||||
|
||||
// TLSEnabled reports whether the server serves HTTPS. Both halves are required,
|
||||
// so callers cannot infer it from the certificate alone.
|
||||
func (c *configOptions) TLSEnabled() bool {
|
||||
return c.TLSCert != "" && c.TLSKey != ""
|
||||
}
|
||||
|
||||
var (
|
||||
Server = &configOptions{}
|
||||
hooks []func()
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import (
|
|||
"testing"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
|
|
@ -453,3 +454,29 @@ var _ = Describe("Configuration", func() {
|
|||
Entry("JSON format", "json"),
|
||||
)
|
||||
})
|
||||
|
||||
var _ = Describe("TLSEnabled", func() {
|
||||
BeforeEach(func() {
|
||||
DeferCleanup(configtest.SetupConfig())
|
||||
})
|
||||
|
||||
It("is false when neither the certificate nor the key is set", func() {
|
||||
Expect(conf.Server.TLSEnabled()).To(BeFalse())
|
||||
})
|
||||
|
||||
It("is true when both the certificate and the key are set", func() {
|
||||
conf.Server.TLSCert = "cert.pem"
|
||||
conf.Server.TLSKey = "key.pem"
|
||||
Expect(conf.Server.TLSEnabled()).To(BeTrue())
|
||||
})
|
||||
|
||||
It("is false when only the certificate is set", func() {
|
||||
conf.Server.TLSCert = "cert.pem"
|
||||
Expect(conf.Server.TLSEnabled()).To(BeFalse())
|
||||
})
|
||||
|
||||
It("is false when only the key is set", func() {
|
||||
conf.Server.TLSKey = "key.pem"
|
||||
Expect(conf.Server.TLSEnabled()).To(BeFalse())
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -198,7 +198,7 @@ var staticData = sync.OnceValue(func() insights.Data {
|
|||
// Config info
|
||||
data.Config.LogLevel = conf.Server.LogLevel
|
||||
data.Config.LogFileConfigured = conf.Server.LogFile != ""
|
||||
data.Config.TLSConfigured = conf.Server.TLSCert != "" && conf.Server.TLSKey != ""
|
||||
data.Config.TLSConfigured = conf.Server.TLSEnabled()
|
||||
data.Config.DefaultBackgroundURLSet = conf.Server.UILoginBackgroundURL == consts.DefaultUILoginBackgroundURL
|
||||
data.Config.EnableArtworkPrecache = conf.Server.EnableArtworkPrecache
|
||||
data.Config.EnableArtworkUpload = conf.Server.EnableArtworkUpload
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ package publicurl
|
|||
|
||||
import (
|
||||
"cmp"
|
||||
"net/http"
|
||||
"context"
|
||||
"net/url"
|
||||
"path"
|
||||
"strconv"
|
||||
|
|
@ -13,35 +13,36 @@ import (
|
|||
"github.com/navidrome/navidrome/core/auth"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/utils/gg"
|
||||
)
|
||||
|
||||
// ImageURL generates a public URL for artwork images.
|
||||
// It creates a signed token for the artwork ID and builds a complete public URL.
|
||||
func ImageURL(req *http.Request, artID model.ArtworkID, size int) string {
|
||||
func ImageURL(ctx context.Context, artID model.ArtworkID, size int) string {
|
||||
token, _ := auth.CreatePublicToken(auth.Claims{ID: artID.String()})
|
||||
uri := path.Join(consts.URLPathPublicImages, token)
|
||||
params := url.Values{}
|
||||
if size > 0 {
|
||||
params.Add("size", strconv.Itoa(size))
|
||||
}
|
||||
return PublicURL(req, uri, params)
|
||||
return PublicURL(ctx, uri, params)
|
||||
}
|
||||
|
||||
// PublicURL builds a full URL for public-facing resources.
|
||||
// It uses ShareURL from config if available, otherwise falls back to extracting
|
||||
// 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 {
|
||||
// It uses ShareURL from config if available, otherwise falls back to the address the
|
||||
// client used to reach the server, recorded in the context.
|
||||
func PublicURL(ctx context.Context, u string, params url.Values) string {
|
||||
if conf.Server.ShareURL == "" {
|
||||
return AbsoluteURL(req, u, params)
|
||||
return AbsoluteURL(ctx, u, params)
|
||||
}
|
||||
shareUrl, err := url.Parse(conf.Server.ShareURL)
|
||||
if err != nil {
|
||||
return AbsoluteURL(req, u, params)
|
||||
return AbsoluteURL(ctx, u, params)
|
||||
}
|
||||
buildUrl, err := url.Parse(u)
|
||||
if err != nil {
|
||||
return AbsoluteURL(req, u, params)
|
||||
return AbsoluteURL(ctx, u, params)
|
||||
}
|
||||
buildUrl.Scheme = shareUrl.Scheme
|
||||
buildUrl.Host = shareUrl.Host
|
||||
|
|
@ -55,13 +56,12 @@ func PublicURL(req *http.Request, u string, params url.Values) string {
|
|||
}
|
||||
|
||||
// AbsoluteURL builds an absolute URL from a relative path.
|
||||
// It uses BaseHost/BaseScheme from config if available, otherwise extracts
|
||||
// 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 {
|
||||
// It uses BaseHost/BaseScheme from config if available, otherwise the address the client
|
||||
// used to reach the server, recorded in the context by the server's address middleware.
|
||||
func AbsoluteURL(ctx context.Context, u string, params url.Values) string {
|
||||
buildUrl, err := url.Parse(u)
|
||||
if err != nil {
|
||||
log.Error(req.Context(), "Failed to parse URL path", "url", u, err)
|
||||
log.Error(ctx, "Failed to parse URL path", "url", u, err)
|
||||
return ""
|
||||
}
|
||||
if strings.HasPrefix(u, "/") {
|
||||
|
|
@ -69,12 +69,13 @@ func AbsoluteURL(req *http.Request, u string, params url.Values) string {
|
|||
if conf.Server.BaseHost != "" {
|
||||
buildUrl.Scheme = cmp.Or(conf.Server.BaseScheme, "http")
|
||||
buildUrl.Host = conf.Server.BaseHost
|
||||
} else if req != nil {
|
||||
buildUrl.Scheme = req.URL.Scheme
|
||||
buildUrl.Host = req.Host
|
||||
} else if scheme, host, ok := request.ServerAddressFrom(ctx); ok {
|
||||
buildUrl.Scheme = scheme
|
||||
buildUrl.Host = host
|
||||
} else {
|
||||
buildUrl.Scheme = "http"
|
||||
buildUrl.Host = "localhost"
|
||||
log.Debug(ctx, "Building a public URL with no public address available; set ShareURL to make it reachable", "url", u)
|
||||
buildUrl.Scheme = gg.If(conf.Server.TLSEnabled(), "https", "http")
|
||||
buildUrl.Host = "localhost:" + strconv.Itoa(conf.Server.Port)
|
||||
}
|
||||
}
|
||||
if len(params) > 0 {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
package publicurl_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"context"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
|
|
@ -12,6 +12,7 @@ import (
|
|||
"github.com/navidrome/navidrome/core/publicurl"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
|
|
@ -36,24 +37,17 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
})
|
||||
|
||||
It("uses ShareURL as the base", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
result := publicurl.PublicURL(r, "/path/to/resource", nil)
|
||||
result := publicurl.PublicURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://share.example.com/path/to/resource"))
|
||||
})
|
||||
|
||||
It("includes query parameters", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
params := url.Values{"size": []string{"300"}, "format": []string{"png"}}
|
||||
result := publicurl.PublicURL(r, "/image/123", params)
|
||||
result := publicurl.PublicURL(context.Background(), "/image/123", params)
|
||||
Expect(result).To(ContainSubstring("https://share.example.com/image/123"))
|
||||
Expect(result).To(ContainSubstring("size=300"))
|
||||
Expect(result).To(ContainSubstring("format=png"))
|
||||
})
|
||||
|
||||
It("works without a request", func() {
|
||||
result := publicurl.PublicURL(nil, "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://share.example.com/path/to/resource"))
|
||||
})
|
||||
})
|
||||
|
||||
When("ShareURL includes a path", func() {
|
||||
|
|
@ -62,21 +56,19 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
})
|
||||
|
||||
It("prepends the ShareURL path to the resource", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
result := publicurl.PublicURL(r, "/share/img/hash", nil)
|
||||
result := publicurl.PublicURL(context.Background(), "/share/img/hash", nil)
|
||||
Expect(result).To(Equal("https://example.com/navi/share/img/hash"))
|
||||
})
|
||||
|
||||
It("prepends the ShareURL path and includes query parameters", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
params := url.Values{"size": []string{"600"}}
|
||||
result := publicurl.PublicURL(r, "/share/img/hash", params)
|
||||
result := publicurl.PublicURL(context.Background(), "/share/img/hash", params)
|
||||
Expect(result).To(Equal("https://example.com/navi/share/img/hash?size=600"))
|
||||
})
|
||||
|
||||
It("handles trailing slash in ShareURL path", func() {
|
||||
conf.Server.ShareURL = "https://example.com/navi/"
|
||||
result := publicurl.PublicURL(nil, "/share/img/hash", nil)
|
||||
result := publicurl.PublicURL(context.Background(), "/share/img/hash", nil)
|
||||
Expect(result).To(Equal("https://example.com/navi/share/img/hash"))
|
||||
})
|
||||
})
|
||||
|
|
@ -87,15 +79,15 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
})
|
||||
|
||||
It("falls back to AbsoluteURL with request", func() {
|
||||
r, _ := http.NewRequest("GET", "https://myserver.com/test", nil)
|
||||
r.Host = "myserver.com"
|
||||
result := publicurl.PublicURL(r, "/path/to/resource", nil)
|
||||
ctx := request.WithServerAddress(context.Background(), "https", "myserver.com")
|
||||
result := publicurl.PublicURL(ctx, "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://myserver.com/path/to/resource"))
|
||||
})
|
||||
|
||||
It("falls back to localhost without request", func() {
|
||||
result := publicurl.PublicURL(nil, "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("http://localhost/path/to/resource"))
|
||||
It("falls back to localhost on the configured port without request", func() {
|
||||
conf.Server.Port = 4533
|
||||
result := publicurl.PublicURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("http://localhost:4533/path/to/resource"))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -109,15 +101,13 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
})
|
||||
|
||||
It("uses BaseHost and BaseScheme", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://configured.example.com/path/to/resource"))
|
||||
})
|
||||
|
||||
It("defaults to http scheme if BaseScheme is empty", func() {
|
||||
conf.Server.BaseScheme = ""
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("http://configured.example.com/path/to/resource"))
|
||||
})
|
||||
})
|
||||
|
|
@ -129,15 +119,30 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
})
|
||||
|
||||
It("extracts host from request", func() {
|
||||
r, _ := http.NewRequest("GET", "https://request.example.com/test", nil)
|
||||
r.Host = "request.example.com"
|
||||
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
|
||||
ctx := request.WithServerAddress(context.Background(), "https", "request.example.com")
|
||||
result := publicurl.AbsoluteURL(ctx, "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://request.example.com/path/to/resource"))
|
||||
})
|
||||
|
||||
It("falls back to localhost without request", func() {
|
||||
result := publicurl.AbsoluteURL(nil, "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("http://localhost/path/to/resource"))
|
||||
It("falls back to localhost on the configured port without request", func() {
|
||||
conf.Server.Port = 8080
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("http://localhost:8080/path/to/resource"))
|
||||
})
|
||||
|
||||
It("uses https in the fallback when TLS is configured", func() {
|
||||
conf.Server.Port = 4533
|
||||
conf.Server.TLSCert = "cert.pem"
|
||||
conf.Server.TLSKey = "key.pem"
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://localhost:4533/path/to/resource"))
|
||||
})
|
||||
|
||||
It("stays on http when only the certificate is configured", func() {
|
||||
conf.Server.Port = 4533
|
||||
conf.Server.TLSCert = "cert.pem"
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("http://localhost:4533/path/to/resource"))
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -149,24 +154,21 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
})
|
||||
|
||||
It("prepends BasePath to the URL", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
result := publicurl.AbsoluteURL(r, "/path/to/resource", nil)
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path/to/resource", nil)
|
||||
Expect(result).To(Equal("https://example.com/navidrome/path/to/resource"))
|
||||
})
|
||||
})
|
||||
|
||||
It("passes through absolute URLs unchanged", func() {
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
result := publicurl.AbsoluteURL(r, "https://other.example.com/path", nil)
|
||||
result := publicurl.AbsoluteURL(context.Background(), "https://other.example.com/path", nil)
|
||||
Expect(result).To(Equal("https://other.example.com/path"))
|
||||
})
|
||||
|
||||
It("includes query parameters", func() {
|
||||
conf.Server.BaseHost = "example.com"
|
||||
conf.Server.BaseScheme = "https"
|
||||
r, _ := http.NewRequest("GET", "http://localhost/test", nil)
|
||||
params := url.Values{"key": []string{"value"}}
|
||||
result := publicurl.AbsoluteURL(r, "/path", params)
|
||||
result := publicurl.AbsoluteURL(context.Background(), "/path", params)
|
||||
Expect(result).To(Equal("https://example.com/path?key=value"))
|
||||
})
|
||||
})
|
||||
|
|
@ -180,20 +182,51 @@ var _ = Describe("Public URL Utilities", func() {
|
|||
|
||||
It("generates a URL with the artwork token", func() {
|
||||
artID := model.NewArtworkID(model.KindAlbumArtwork, "album-123", nil)
|
||||
result := publicurl.ImageURL(nil, artID, 0)
|
||||
result := publicurl.ImageURL(context.Background(), artID, 0)
|
||||
Expect(result).To(HavePrefix("https://share.example.com/share/img/"))
|
||||
})
|
||||
|
||||
It("includes size parameter when provided", func() {
|
||||
artID := model.NewArtworkID(model.KindArtistArtwork, "artist-1", nil)
|
||||
result := publicurl.ImageURL(nil, artID, 300)
|
||||
result := publicurl.ImageURL(context.Background(), artID, 300)
|
||||
Expect(result).To(ContainSubstring("size=300"))
|
||||
})
|
||||
|
||||
It("omits size parameter when zero", func() {
|
||||
artID := model.NewArtworkID(model.KindMediaFileArtwork, "track-1", nil)
|
||||
result := publicurl.ImageURL(nil, artID, 0)
|
||||
result := publicurl.ImageURL(context.Background(), artID, 0)
|
||||
Expect(result).ToNot(ContainSubstring("size="))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("ImageURL address precedence", func() {
|
||||
var artID model.ArtworkID
|
||||
|
||||
BeforeEach(func() {
|
||||
auth.PublicTokenAuth = jwtauth.New("HS256", []byte("test secret"), nil)
|
||||
artID = model.NewArtworkID(model.KindMediaFileArtwork, "track-1", nil)
|
||||
})
|
||||
|
||||
It("uses the address of the request that triggered the call", func() {
|
||||
ctx := request.WithServerAddress(context.Background(), "https", "music.example.com")
|
||||
|
||||
result := publicurl.ImageURL(ctx, artID, 300)
|
||||
Expect(result).To(HavePrefix("https://music.example.com/share/img/"))
|
||||
Expect(result).To(ContainSubstring("size=300"))
|
||||
})
|
||||
|
||||
It("prefers ShareURL over the address in the context", func() {
|
||||
conf.Server.ShareURL = "https://share.example.com"
|
||||
ctx := request.WithServerAddress(context.Background(), "https", "music.example.com")
|
||||
|
||||
result := publicurl.ImageURL(ctx, artID, 0)
|
||||
Expect(result).To(HavePrefix("https://share.example.com/share/img/"))
|
||||
})
|
||||
|
||||
It("falls back to localhost on the configured port when no address is available", func() {
|
||||
conf.Server.Port = 4533
|
||||
result := publicurl.ImageURL(context.Background(), artID, 0)
|
||||
Expect(result).To(HavePrefix("http://localhost:4533/share/img/"))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ const (
|
|||
ReverseProxyIp = contextKey("reverseProxyIp")
|
||||
InternalAuth = contextKey("internalAuth") // Used for internal API calls, e.g., from the plugins
|
||||
TokenEpochHolder = contextKey("tokenEpochHolder")
|
||||
ServerAddress = contextKey("serverAddress")
|
||||
)
|
||||
|
||||
var allKeys = []contextKey{
|
||||
|
|
@ -32,6 +33,7 @@ var allKeys = []contextKey{
|
|||
ClientUniqueId,
|
||||
ReverseProxyIp,
|
||||
InternalAuth,
|
||||
ServerAddress,
|
||||
}
|
||||
|
||||
func WithUser(ctx context.Context, u model.User) context.Context {
|
||||
|
|
@ -70,6 +72,25 @@ func WithInternalAuth(ctx context.Context, username string) context.Context {
|
|||
return context.WithValue(ctx, InternalAuth, username)
|
||||
}
|
||||
|
||||
// serverAddress is the public scheme and host the client used to reach this server,
|
||||
// so code running without an http.Request can still build absolute URLs.
|
||||
type serverAddress struct {
|
||||
scheme string
|
||||
host string
|
||||
}
|
||||
|
||||
func WithServerAddress(ctx context.Context, scheme, host string) context.Context {
|
||||
return context.WithValue(ctx, ServerAddress, serverAddress{scheme: scheme, host: host})
|
||||
}
|
||||
|
||||
func ServerAddressFrom(ctx context.Context) (scheme, host string, ok bool) {
|
||||
a, ok := ctx.Value(ServerAddress).(serverAddress)
|
||||
if !ok || a.host == "" {
|
||||
return "", "", false
|
||||
}
|
||||
return a.scheme, a.host, true
|
||||
}
|
||||
|
||||
func UserFrom(ctx context.Context) (model.User, bool) {
|
||||
v, ok := ctx.Value(User).(model.User)
|
||||
return v, ok
|
||||
|
|
|
|||
|
|
@ -38,3 +38,34 @@ var _ = Describe("Token epoch holder", func() {
|
|||
Expect(ok).To(BeFalse())
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("Server address", func() {
|
||||
It("reports nothing when unset", func() {
|
||||
_, _, ok := ServerAddressFrom(context.TODO())
|
||||
Expect(ok).To(BeFalse())
|
||||
})
|
||||
|
||||
It("round-trips the scheme and host", func() {
|
||||
ctx := WithServerAddress(context.TODO(), "https", "music.example.com")
|
||||
|
||||
scheme, host, ok := ServerAddressFrom(ctx)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(scheme).To(Equal("https"))
|
||||
Expect(host).To(Equal("music.example.com"))
|
||||
})
|
||||
|
||||
It("reports nothing when the host is empty", func() {
|
||||
ctx := WithServerAddress(context.TODO(), "https", "")
|
||||
|
||||
_, _, ok := ServerAddressFrom(ctx)
|
||||
Expect(ok).To(BeFalse())
|
||||
})
|
||||
|
||||
It("is carried over to a background context by AddValues", func() {
|
||||
reqCtx := WithServerAddress(context.TODO(), "https", "music.example.com")
|
||||
|
||||
_, host, ok := ServerAddressFrom(AddValues(context.Background(), reqCtx))
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(host).To(Equal("music.example.com"))
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -14,24 +14,24 @@ func newArtworkService() host.ArtworkService {
|
|||
return &artworkServiceImpl{}
|
||||
}
|
||||
|
||||
func (a *artworkServiceImpl) GetArtistUrl(_ context.Context, id string, size int32) (string, error) {
|
||||
func (a *artworkServiceImpl) GetArtistUrl(ctx context.Context, id string, size int32) (string, error) {
|
||||
artID := model.ArtworkID{Kind: model.KindArtistArtwork, ID: id}
|
||||
return publicurl.ImageURL(nil, artID, int(size)), nil
|
||||
return publicurl.ImageURL(ctx, artID, int(size)), nil
|
||||
}
|
||||
|
||||
func (a *artworkServiceImpl) GetAlbumUrl(_ context.Context, id string, size int32) (string, error) {
|
||||
func (a *artworkServiceImpl) GetAlbumUrl(ctx context.Context, id string, size int32) (string, error) {
|
||||
artID := model.ArtworkID{Kind: model.KindAlbumArtwork, ID: id}
|
||||
return publicurl.ImageURL(nil, artID, int(size)), nil
|
||||
return publicurl.ImageURL(ctx, artID, int(size)), nil
|
||||
}
|
||||
|
||||
func (a *artworkServiceImpl) GetTrackUrl(_ context.Context, id string, size int32) (string, error) {
|
||||
func (a *artworkServiceImpl) GetTrackUrl(ctx context.Context, id string, size int32) (string, error) {
|
||||
artID := model.ArtworkID{Kind: model.KindMediaFileArtwork, ID: id}
|
||||
return publicurl.ImageURL(nil, artID, int(size)), nil
|
||||
return publicurl.ImageURL(ctx, artID, int(size)), nil
|
||||
}
|
||||
|
||||
func (a *artworkServiceImpl) GetPlaylistUrl(_ context.Context, id string, size int32) (string, error) {
|
||||
func (a *artworkServiceImpl) GetPlaylistUrl(ctx context.Context, id string, size int32) (string, error) {
|
||||
artID := model.ArtworkID{Kind: model.KindPlaylistArtwork, ID: id}
|
||||
return publicurl.ImageURL(nil, artID, int(size)), nil
|
||||
return publicurl.ImageURL(ctx, artID, int(size)), nil
|
||||
}
|
||||
|
||||
var _ host.ArtworkService = (*artworkServiceImpl)(nil)
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import (
|
|||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/core/auth"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
|
|
@ -122,7 +123,7 @@ var _ = Describe("ArtworkService", Ordered, func() {
|
|||
Size: size,
|
||||
}
|
||||
inputBytes, _ := json.Marshal(input)
|
||||
_, outputBytes, err := instance.Call("nd_test_artwork", inputBytes)
|
||||
_, outputBytes, err := instance.CallWithContext(ctx, "nd_test_artwork", inputBytes)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
|
@ -189,6 +190,14 @@ var _ = Describe("ArtworkService", Ordered, func() {
|
|||
Expect(artID.ID).To(Equal("al-456"))
|
||||
})
|
||||
|
||||
It("uses the address of the request that triggered the plugin", func() {
|
||||
ctx := request.WithServerAddress(GinkgoT().Context(), "https", "music.example.com")
|
||||
|
||||
url, err := callTestArtwork(ctx, "track", "mf-789", 300)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(url).To(HavePrefix("https://music.example.com/share/img/"))
|
||||
})
|
||||
|
||||
It("should handle unknown artwork type", func() {
|
||||
_, err := callTestArtwork(GinkgoT().Context(), "unknown", "id-123", 0)
|
||||
Expect(err).To(HaveOccurred())
|
||||
|
|
|
|||
|
|
@ -208,6 +208,8 @@ func serverAddressMiddleware(h http.Handler) http.Handler {
|
|||
if rScheme, rHost := ServerAddress(r); rHost != "" {
|
||||
r.Host = rHost
|
||||
r.URL.Scheme = rScheme
|
||||
// Recorded so code running without the request (e.g. plugins) can build public URLs.
|
||||
r = r.WithContext(request.WithServerAddress(r.Context(), rScheme, rHost))
|
||||
}
|
||||
|
||||
// Call the next handler in the chain with the modified request and response.
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import (
|
|||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/core/publicurl"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
|
|
@ -69,10 +70,15 @@ var _ = Describe("middlewares", func() {
|
|||
middleware http.Handler
|
||||
recorder *httptest.ResponseRecorder
|
||||
req *http.Request
|
||||
gotScheme string
|
||||
gotHost string
|
||||
gotOK bool
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
gotScheme, gotHost, gotOK = "", "", false
|
||||
nextHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
gotScheme, gotHost, gotOK = request.ServerAddressFrom(r.Context())
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
middleware = serverAddressMiddleware(nextHandler)
|
||||
|
|
@ -89,6 +95,13 @@ var _ = Describe("middlewares", func() {
|
|||
Expect(req.Host).To(Equal("example.com"))
|
||||
Expect(req.URL.Scheme).To(Equal("http"))
|
||||
})
|
||||
|
||||
It("should record the address in the context", func() {
|
||||
middleware.ServeHTTP(recorder, req)
|
||||
Expect(gotOK).To(BeTrue())
|
||||
Expect(gotScheme).To(Equal("http"))
|
||||
Expect(gotHost).To(Equal("example.com"))
|
||||
})
|
||||
})
|
||||
|
||||
Context("with X-Forwarded-Host header", func() {
|
||||
|
|
@ -143,6 +156,22 @@ var _ = Describe("middlewares", func() {
|
|||
Expect(req.Host).To(Equal("forwarded.example.com"))
|
||||
Expect(req.URL.Scheme).To(Equal("https"))
|
||||
})
|
||||
|
||||
It("should record the forwarded address in the context", func() {
|
||||
middleware.ServeHTTP(recorder, req)
|
||||
Expect(gotOK).To(BeTrue())
|
||||
Expect(gotScheme).To(Equal("https"))
|
||||
Expect(gotHost).To(Equal("forwarded.example.com"))
|
||||
})
|
||||
|
||||
It("lets a handler build a public URL on the forwarded address", func() {
|
||||
var got string
|
||||
serverAddressMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
got = publicurl.AbsoluteURL(r.Context(), "/share/img/token", nil)
|
||||
})).ServeHTTP(recorder, req)
|
||||
|
||||
Expect(got).To(Equal("https://forwarded.example.com/share/img/token"))
|
||||
})
|
||||
})
|
||||
|
||||
Context("with multiple values in X-Forwarded-Host header", func() {
|
||||
|
|
|
|||
|
|
@ -81,8 +81,8 @@ func checkShareError(ctx context.Context, w http.ResponseWriter, err error, id s
|
|||
}
|
||||
|
||||
func (pub *Router) mapShareInfo(r *http.Request, s model.Share) *model.Share {
|
||||
s.URL = ShareURL(r, s.ID)
|
||||
s.ImageURL = publicurl.ImageURL(r, s.CoverArtID(), conf.Server.UICoverArtSize)
|
||||
s.URL = ShareURL(r.Context(), s.ID)
|
||||
s.ImageURL = publicurl.ImageURL(r.Context(), s.CoverArtID(), conf.Server.UICoverArtSize)
|
||||
for i := range s.Tracks {
|
||||
s.Tracks[i].ID = encodeMediafileShare(s, s.Tracks[i].ID)
|
||||
}
|
||||
|
|
@ -92,7 +92,7 @@ func (pub *Router) mapShareInfo(r *http.Request, s model.Share) *model.Share {
|
|||
func (pub *Router) mapShareToM3U(r *http.Request, s model.Share) *model.Share {
|
||||
for i := range s.Tracks {
|
||||
id := encodeMediafileShare(s, s.Tracks[i].ID)
|
||||
s.Tracks[i].Path = publicurl.PublicURL(r, path.Join(consts.URLPathPublic, "s", id), nil)
|
||||
s.Tracks[i].Path = publicurl.PublicURL(r.Context(), path.Join(consts.URLPathPublic, "s", id), nil)
|
||||
}
|
||||
return &s
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package public
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"path"
|
||||
|
||||
|
|
@ -59,7 +60,7 @@ func (pub *Router) routes() http.Handler {
|
|||
return r
|
||||
}
|
||||
|
||||
func ShareURL(r *http.Request, id string) string {
|
||||
func ShareURL(ctx context.Context, id string) string {
|
||||
uri := path.Join(consts.URLPathPublic, id)
|
||||
return publicurl.PublicURL(r, uri, nil)
|
||||
return publicurl.PublicURL(ctx, uri, nil)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -231,9 +231,9 @@ func (api *Router) GetAlbumInfo(r *http.Request) (*responses.Subsonic, error) {
|
|||
response.AlbumInfo = &responses.AlbumInfo{}
|
||||
response.AlbumInfo.Notes = album.Description
|
||||
if !album.ImageAbsent {
|
||||
response.AlbumInfo.SmallImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 300)
|
||||
response.AlbumInfo.MediumImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 600)
|
||||
response.AlbumInfo.LargeImageUrl = publicurl.ImageURL(r, album.CoverArtID(), 1200)
|
||||
response.AlbumInfo.SmallImageUrl = publicurl.ImageURL(r.Context(), album.CoverArtID(), 300)
|
||||
response.AlbumInfo.MediumImageUrl = publicurl.ImageURL(r.Context(), album.CoverArtID(), 600)
|
||||
response.AlbumInfo.LargeImageUrl = publicurl.ImageURL(r.Context(), album.CoverArtID(), 1200)
|
||||
}
|
||||
|
||||
response.AlbumInfo.LastFmUrl = album.ExternalUrl
|
||||
|
|
@ -298,9 +298,9 @@ func (api *Router) getArtistInfo(r *http.Request) (*responses.ArtistInfoBase, *m
|
|||
base := responses.ArtistInfoBase{}
|
||||
base.Biography = artist.Biography
|
||||
if !artist.ImageAbsent {
|
||||
base.SmallImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 300)
|
||||
base.MediumImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 600)
|
||||
base.LargeImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 1200)
|
||||
base.SmallImageUrl = publicurl.ImageURL(r.Context(), artist.CoverArtID(), 300)
|
||||
base.MediumImageUrl = publicurl.ImageURL(r.Context(), artist.CoverArtID(), 600)
|
||||
base.LargeImageUrl = publicurl.ImageURL(r.Context(), artist.CoverArtID(), 1200)
|
||||
}
|
||||
base.LastFmUrl = artist.ExternalUrl
|
||||
base.MusicBrainzID = artist.MbzArtistID
|
||||
|
|
|
|||
|
|
@ -112,7 +112,7 @@ func toArtist(r *http.Request, a model.Artist) responses.Artist {
|
|||
CoverArt: coverArtOrEmpty(a.CoverArtID(), a.ImageAbsent),
|
||||
}
|
||||
if !a.ImageAbsent {
|
||||
artist.ArtistImageUrl = publicurl.ImageURL(r, a.CoverArtID(), 600)
|
||||
artist.ArtistImageUrl = publicurl.ImageURL(r.Context(), a.CoverArtID(), 600)
|
||||
}
|
||||
if conf.Server.Subsonic.EnableAverageRating {
|
||||
artist.AverageRating = a.AverageRating
|
||||
|
|
@ -132,7 +132,7 @@ func toArtistID3(r *http.Request, a model.Artist) responses.ArtistID3 {
|
|||
UserRating: int32(a.Rating),
|
||||
}
|
||||
if !a.ImageAbsent {
|
||||
artist.ArtistImageUrl = publicurl.ImageURL(r, a.CoverArtID(), 600)
|
||||
artist.ArtistImageUrl = publicurl.ImageURL(r.Context(), a.CoverArtID(), 600)
|
||||
}
|
||||
if conf.Server.Subsonic.EnableAverageRating {
|
||||
artist.AverageRating = a.AverageRating
|
||||
|
|
|
|||
|
|
@ -116,7 +116,7 @@ func (api *Router) Search2(r *http.Request) (*responses.Subsonic, error) {
|
|||
CoverArt: coverArtOrEmpty(artist.CoverArtID(), artist.ImageAbsent),
|
||||
}
|
||||
if !artist.ImageAbsent {
|
||||
a.ArtistImageUrl = publicurl.ImageURL(r, artist.CoverArtID(), 600)
|
||||
a.ArtistImageUrl = publicurl.ImageURL(r.Context(), artist.CoverArtID(), 600)
|
||||
}
|
||||
if artist.Starred {
|
||||
a.Starred = artist.StarredAt
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ func (api *Router) GetShares(r *http.Request) (*responses.Subsonic, error) {
|
|||
func (api *Router) buildShare(r *http.Request, share model.Share) responses.Share {
|
||||
resp := responses.Share{
|
||||
ID: share.ID,
|
||||
Url: public.ShareURL(r, share.ID),
|
||||
Url: public.ShareURL(r.Context(), share.ID),
|
||||
Description: share.Description,
|
||||
Username: share.Username,
|
||||
Created: share.CreatedAt,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue