From 3da2b590e780722df6c2a35e10fd38dfaf83098a Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Mon, 24 Aug 2026 16:22:32 +0100 Subject: [PATCH] fix: add Navidrome UserAgent in all outgoing requests (#6020) * There has been a report about navidrome hitting listenbrainz hard and the listenbrainz guys wanting to be able to distinguish navidrome * feat: apply Navidrome User-Agent to all outgoing HTTP requests Add utils/httpclient, a shared http.Client factory whose transport sets the User-Agent header (Navidrome/{version} - https://github.com/navidrome) on any request that does not already have one, and use it at every place the server builds an HTTP client: Last.fm, ListenBrainz and Deezer agents and auth routers, insights collector, backgrounds handler, and the plugin host HTTP service. Plugin-set User-Agent values are preserved. The per-request header lines from the previous commit are superseded by the transport. --------- Co-authored-by: Deluan --- adapters/deezer/deezer.go | 6 +- adapters/lastfm/agent.go | 5 +- adapters/lastfm/auth_router.go | 5 +- adapters/listenbrainz/agent.go | 6 +- adapters/listenbrainz/auth_router.go | 5 +- consts/consts.go | 2 +- core/metrics/insights.go | 5 +- plugins/host_httpclient.go | 3 +- server/backgrounds/handler.go | 5 +- utils/httpclient/httpclient.go | 35 +++++++++++ utils/httpclient/httpclient_suite_test.go | 17 +++++ utils/httpclient/httpclient_test.go | 76 +++++++++++++++++++++++ 12 files changed, 146 insertions(+), 24 deletions(-) create mode 100644 utils/httpclient/httpclient.go create mode 100644 utils/httpclient/httpclient_suite_test.go create mode 100644 utils/httpclient/httpclient_test.go diff --git a/adapters/deezer/deezer.go b/adapters/deezer/deezer.go index 742b8b1a5..1fa10e25c 100644 --- a/adapters/deezer/deezer.go +++ b/adapters/deezer/deezer.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "net/http" "slices" "strings" @@ -15,6 +14,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/cache" + "github.com/navidrome/navidrome/utils/httpclient" "github.com/navidrome/navidrome/utils/slice" ) @@ -36,9 +36,7 @@ func deezerConstructor(dataStore model.DataStore) agents.Interface { dataStore: dataStore, languages: conf.Server.Deezer.Languages, } - httpClient := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } + httpClient := httpclient.New(consts.DefaultHttpClientTimeOut) cachedHttpClient := cache.NewHTTPClient(httpClient, consts.DefaultHttpClientTimeOut) agent.client = newClient(cachedHttpClient) return agent diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index f967595e3..863868b5a 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -18,6 +18,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/cache" + "github.com/navidrome/navidrome/utils/httpclient" "golang.org/x/net/html" ) @@ -59,9 +60,7 @@ func lastFMConstructor(ds model.DataStore) *lastfmAgent { secret: conf.Server.LastFM.Secret, sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty}, } - hc := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } + hc := httpclient.New(consts.DefaultHttpClientTimeOut) chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut) l.httpClient = chc l.client = newClient(l.apiKey, l.secret, chc) diff --git a/adapters/lastfm/auth_router.go b/adapters/lastfm/auth_router.go index 499863e28..411bf069a 100644 --- a/adapters/lastfm/auth_router.go +++ b/adapters/lastfm/auth_router.go @@ -18,6 +18,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/utils/httpclient" "github.com/navidrome/navidrome/utils/req" ) @@ -41,9 +42,7 @@ func NewRouter(ds model.DataStore) *Router { sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty}, } r.Handler = r.routes() - hc := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } + hc := httpclient.New(consts.DefaultHttpClientTimeOut) r.client = newClient(r.apiKey, r.secret, hc) return r } diff --git a/adapters/listenbrainz/agent.go b/adapters/listenbrainz/agent.go index 76beed921..a59a5393f 100644 --- a/adapters/listenbrainz/agent.go +++ b/adapters/listenbrainz/agent.go @@ -3,7 +3,6 @@ package listenbrainz import ( "context" "errors" - "net/http" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" @@ -12,6 +11,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/cache" + "github.com/navidrome/navidrome/utils/httpclient" "github.com/navidrome/navidrome/utils/slice" ) @@ -33,9 +33,7 @@ func listenBrainzConstructor(ds model.DataStore) *listenBrainzAgent { sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty}, baseURL: conf.Server.ListenBrainz.BaseURL, } - hc := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } + hc := httpclient.New(consts.DefaultHttpClientTimeOut) chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut) l.client = newClient(l.baseURL, chc) return l diff --git a/adapters/listenbrainz/auth_router.go b/adapters/listenbrainz/auth_router.go index 7cb9eb16a..1ff1a1495 100644 --- a/adapters/listenbrainz/auth_router.go +++ b/adapters/listenbrainz/auth_router.go @@ -16,6 +16,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/utils/httpclient" ) type sessionKeysRepo interface { @@ -37,9 +38,7 @@ func NewRouter(ds model.DataStore) *Router { sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty}, } r.Handler = r.routes() - hc := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } + hc := httpclient.New(consts.DefaultHttpClientTimeOut) r.client = newClient(conf.Server.ListenBrainz.BaseURL, hc) return r } diff --git a/consts/consts.go b/consts/consts.go index aed8ecf66..2934cd968 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -201,7 +201,7 @@ var ( } ) -var HTTPUserAgent = "Navidrome" + "/" + Version +var HTTPUserAgent = "Navidrome/" + Version + " - https://github.com/navidrome" var ( VariousArtists = "Various Artists" diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 66d0b89bd..706df6559 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -26,6 +26,7 @@ import ( "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/plugins" "github.com/navidrome/navidrome/server/events" + "github.com/navidrome/navidrome/utils/httpclient" "github.com/navidrome/navidrome/utils/singleton" ) @@ -95,9 +96,7 @@ func (c *insightsCollector) sendInsights(ctx context.Context) { log.Trace(ctx, "No users found, skipping Insights data collection") return } - hc := &http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } + hc := httpclient.New(consts.DefaultHttpClientTimeOut) data := c.collect(ctx) if data == nil { return diff --git a/plugins/host_httpclient.go b/plugins/host_httpclient.go index f1d64deb7..4c8f85acd 100644 --- a/plugins/host_httpclient.go +++ b/plugins/host_httpclient.go @@ -14,6 +14,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/plugins/host" + "github.com/navidrome/navidrome/utils/httpclient" ) const ( @@ -46,7 +47,7 @@ func newHTTPService(pluginName string, permission *HTTPPermission) *httpServiceI requiredHosts: requiredHosts, } svc.client = &http.Client{ - Transport: http.DefaultTransport, + Transport: httpclient.NewTransport(nil), // Timeout is set per-request via context deadline, not here. // CheckRedirect validates hosts and enforces redirect limits. CheckRedirect: func(req *http.Request, via []*http.Request) error { diff --git a/server/backgrounds/handler.go b/server/backgrounds/handler.go index b00a51696..f6e159b4b 100644 --- a/server/backgrounds/handler.go +++ b/server/backgrounds/handler.go @@ -13,6 +13,7 @@ import ( "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/utils/cache" + "github.com/navidrome/navidrome/utils/httpclient" "github.com/navidrome/navidrome/utils/random" "gopkg.in/yaml.v3" ) @@ -35,7 +36,7 @@ type Handler struct { func NewHandler() *Handler { h := &Handler{} - h.httpClient = cache.NewHTTPClient(&http.Client{Timeout: 5 * time.Second}, imageListTTL) + h.httpClient = cache.NewHTTPClient(httpclient.New(5*time.Second), imageListTTL) h.cache = cache.NewFileCache(imageCacheDir, imageCacheSize, imageCacheDir, imageCacheMaxItems, h.serveImage) go func() { _, _ = h.getImageList(log.NewContext(context.Background())) @@ -78,7 +79,7 @@ func (h *Handler) serveImage(ctx context.Context, item cache.Item) (io.Reader, e if image == "" { return nil, errors.New("empty image name") } - c := http.Client{Timeout: imageRequestTimeout} + c := httpclient.New(imageRequestTimeout) req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageURL(image), nil) resp, err := c.Do(req) //nolint:bodyclose,gosec // No need to close resp.Body, it will be closed via the CachedStream wrapper if errors.Is(err, context.DeadlineExceeded) { diff --git a/utils/httpclient/httpclient.go b/utils/httpclient/httpclient.go new file mode 100644 index 000000000..7fb48f36d --- /dev/null +++ b/utils/httpclient/httpclient.go @@ -0,0 +1,35 @@ +// Package httpclient provides a shared http.Client factory that identifies +// Navidrome via the User-Agent header on all outgoing requests. +package httpclient + +import ( + "net/http" + "time" + + "github.com/navidrome/navidrome/consts" +) + +type uaTransport struct { + base http.RoundTripper +} + +func (t *uaTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if _, ok := req.Header["User-Agent"]; !ok { + req = req.Clone(req.Context()) + req.Header.Set("User-Agent", consts.HTTPUserAgent) + } + return t.base.RoundTrip(req) +} + +// NewTransport wraps base (or http.DefaultTransport if nil) to set the +// Navidrome User-Agent on requests that don't have one. +func NewTransport(base http.RoundTripper) http.RoundTripper { + if base == nil { + base = http.DefaultTransport + } + return &uaTransport{base: base} +} + +func New(timeout time.Duration) *http.Client { + return &http.Client{Timeout: timeout, Transport: NewTransport(nil)} +} diff --git a/utils/httpclient/httpclient_suite_test.go b/utils/httpclient/httpclient_suite_test.go new file mode 100644 index 000000000..e18a9d0ad --- /dev/null +++ b/utils/httpclient/httpclient_suite_test.go @@ -0,0 +1,17 @@ +package httpclient_test + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestHTTPClient(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "HTTPClient Suite") +} diff --git a/utils/httpclient/httpclient_test.go b/utils/httpclient/httpclient_test.go new file mode 100644 index 000000000..c86b51165 --- /dev/null +++ b/utils/httpclient/httpclient_test.go @@ -0,0 +1,76 @@ +package httpclient_test + +import ( + "net/http" + "net/http/httptest" + "time" + + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/utils/httpclient" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("httpclient", func() { + var server *httptest.Server + var receivedUA string + + BeforeEach(func() { + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedUA = r.Header.Get("User-Agent") + })) + DeferCleanup(server.Close) + }) + + Describe("New", func() { + It("sets the Navidrome User-Agent when the request has none", func() { + c := httpclient.New(time.Second) + resp, err := c.Get(server.URL) + Expect(err).ToNot(HaveOccurred()) + resp.Body.Close() + Expect(receivedUA).To(Equal(consts.HTTPUserAgent)) + }) + + It("keeps a User-Agent already set by the caller", func() { + c := httpclient.New(time.Second) + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + Expect(err).ToNot(HaveOccurred()) + req.Header.Set("User-Agent", "CustomAgent/1.0") + resp, err := c.Do(req) + Expect(err).ToNot(HaveOccurred()) + resp.Body.Close() + Expect(receivedUA).To(Equal("CustomAgent/1.0")) + }) + + It("applies the given timeout", func() { + c := httpclient.New(5 * time.Second) + Expect(c.Timeout).To(Equal(5 * time.Second)) + }) + }) + + Describe("NewTransport", func() { + It("uses the default transport when base is nil", func() { + c := &http.Client{Transport: httpclient.NewTransport(nil)} + resp, err := c.Get(server.URL) + Expect(err).ToNot(HaveOccurred()) + resp.Body.Close() + Expect(receivedUA).To(Equal(consts.HTTPUserAgent)) + }) + + It("does not modify the original request", func() { + c := &http.Client{Transport: httpclient.NewTransport(nil)} + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + Expect(err).ToNot(HaveOccurred()) + resp, err := c.Do(req) + Expect(err).ToNot(HaveOccurred()) + resp.Body.Close() + Expect(req.Header).ToNot(HaveKey("User-Agent")) + }) + }) + + Describe("HTTPUserAgent", func() { + It("identifies Navidrome with version and project URL", func() { + Expect(consts.HTTPUserAgent).To(Equal("Navidrome/" + consts.Version + " - https://github.com/navidrome")) + }) + }) +})