Posters: recognise a login page instead of an image; try the key in the query, then the provider

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

internal/arr/image.go

@@ -2,6 +2,7 @@ package arr
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -28,16 +29,10 @@ func (c *Client) Poster(images []Image, inLibrary bool) (key string, fetch func(
 			local, remote := img.URL, img.RemoteURL
 			base, _ := c.settings()
 			return base + "|" + local, func(ctx context.Context) ([]byte, error) {
-				// MediaCover has poster-250.jpg next to poster.jpg; fall back to the original.
-				if small := smallVariant(local); small != local {
-					if data, err := c.GetBytes(ctx, small); err == nil {
-						return data, nil
-					}
-				}
-				data, err := c.GetBytes(ctx, local)
+				data, err := c.localPoster(ctx, local)
 				if err != nil && remote != "" {
-					// The instance has not cached (or has lost) its copy: use the
-					// metadata provider's, as for titles that are not in the library.
+					// The instance has no usable copy (not cached, or behind a login):
+					// use the metadata provider's, as for titles not in the library.
 					if remoteData, remoteErr := FetchURL(ctx, remote); remoteErr == nil {
 						return remoteData, nil
 					}
@@ -53,6 +48,34 @@ func (c *Client) Poster(images []Image, inLibrary bool) (key string, fetch func(
 	return "", nil, false
 }
 
+// localPoster fetches a poster from the instance: the small variant, then the
+// original. When the instance answers with a page instead of an image (it
+// wants a login for /MediaCover) the API key is also tried as a query
+// parameter, which is the other way Radarr and Sonarr accept it.
+func (c *Client) localPoster(ctx context.Context, local string) ([]byte, error) {
+	try := func(ref string) ([]byte, error) {
+		data, err := c.GetBytes(ctx, ref)
+		var page *notImageError
+		if errors.As(err, &page) {
+			if data, err2 := c.getBytes(ctx, ref, true); err2 == nil {
+				return data, nil
+			}
+		}
+		return data, err
+	}
+	if small := smallVariant(local); small != local {
+		data, err := try(small)
+		if err == nil {
+			return data, nil
+		}
+		var page *notImageError
+		if errors.As(err, &page) {
+			return nil, err // the original would be a page as well
+		}
+	}
+	return try(local)
+}
+
 // smallVariant turns /MediaCover/1/poster.jpg?x into /MediaCover/1/poster-250.jpg?x.
 func smallVariant(p string) string {
 	path, query, hasQuery := strings.Cut(p, "?")
@@ -69,19 +92,45 @@ func smallVariant(p string) string {
 // GetBytes downloads a server-relative path such as /MediaCover/1/poster.jpg
 // from the instance, authenticated with the API key.
 func (c *Client) GetBytes(ctx context.Context, ref string) ([]byte, error) {
+	return c.getBytes(ctx, ref, false)
+}
+
+// getBytes sends the API key as a header, or with keyInQuery as the apikey
+// query parameter.
+func (c *Client) getBytes(ctx context.Context, ref string, keyInQuery bool) ([]byte, error) {
 	u, err := c.resolve(ref)
 	if err != nil {
 		return nil, err
 	}
+	_, apiKey := c.settings()
+	if keyInQuery {
+		parsed, err := url.Parse(u)
+		if err != nil {
+			return nil, err
+		}
+		q := parsed.Query()
+		q.Set("apikey", apiKey)
+		parsed.RawQuery = q.Encode()
+		u = parsed.String()
+	}
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
 	if err != nil {
 		return nil, err
 	}
-	_, apiKey := c.settings()
-	req.Header.Set("X-Api-Key", apiKey)
+	if !keyInQuery {
+		req.Header.Set("X-Api-Key", apiKey)
+	}
 	return doImage(c.http, req)
 }
 
+// notImageError means the server answered 200 with something that is not an
+// image, typically the login page of an instance that requires authentication.
+type notImageError struct{ contentType string }
+
+func (e *notImageError) Error() string {
+	return fmt.Sprintf("image: the server sent %q instead of an image (a login page?)", e.contentType)
+}
+
 // resolve joins a server-relative path to the base URL. Radarr/Sonarr include
 // their URL base in image paths, so it must not be added twice.
 func (c *Client) resolve(ref string) (string, error) {
@@ -120,6 +169,11 @@ func doImage(client *http.Client, req *http.Request) ([]byte, error) {
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("image: HTTP %d", resp.StatusCode)
 	}
+	// Pages and error documents. (Not all of text/*: a plain server may label
+	// image bytes it cannot identify as text/plain.)
+	if ct := resp.Header.Get("Content-Type"); strings.HasPrefix(ct, "text/html") || strings.HasPrefix(ct, "application/xhtml") || strings.HasPrefix(ct, "application/json") {
+		return nil, &notImageError{contentType: ct}
+	}
 	data, err := io.ReadAll(io.LimitReader(resp.Body, maxImageBytes+1))
 	if err != nil {
 		return nil, err

internal/arr/image_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
@@ -124,3 +125,69 @@ func TestPosterFallsBackToTheProviderWhenTheInstanceHasNoCopy(t *testing.T) {
 		t.Error("expected the local error")
 	}
 }
+
+// loginWall answers like an instance that requires a login for /MediaCover:
+// a 200 with an HTML page, unless the request carries the key as a query parameter.
+func loginWall(acceptQuery bool, hits *[]string) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		*hits = append(*hits, r.URL.RequestURI()+" key-header="+r.Header.Get("X-Api-Key"))
+		if acceptQuery && r.URL.Query().Get("apikey") == "secret" {
+			w.Header().Set("Content-Type", "image/jpeg")
+			w.Write([]byte("IMAGE"))
+			return
+		}
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		w.Write([]byte("<!DOCTYPE html><html><head><title>Login</title>"))
+	}
+}
+
+func TestPosterTriesTheKeyInTheQueryWhenGivenALoginPage(t *testing.T) {
+	var hits []string
+	srv := httptest.NewServer(loginWall(true, &hits))
+	defer srv.Close()
+
+	c := NewClient(srv.URL, "secret")
+	_, fetch, _ := c.Poster([]Image{{CoverType: "poster", URL: "/MediaCover/89/poster.jpg"}}, true)
+	data, err := fetch(context.Background())
+	if err != nil || string(data) != "IMAGE" {
+		t.Fatalf("data=%q err=%v, hits=%v", data, err, hits)
+	}
+	// Small variant with the header (a page), then the same file with the key in the query.
+	if len(hits) != 2 || !strings.Contains(hits[0], "poster-250.jpg") || !strings.Contains(hits[0], "key-header=secret") ||
+		!strings.Contains(hits[1], "poster-250.jpg?apikey=secret") {
+		t.Errorf("requests = %v", hits)
+	}
+}
+
+func TestPosterUsesTheProviderWhenTheInstanceOnlyShowsALogin(t *testing.T) {
+	var hits []string
+	instance := httptest.NewServer(loginWall(false, &hits))
+	defer instance.Close()
+	var remoteKey, remoteQuery string
+	remote := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		remoteKey, remoteQuery = r.Header.Get("X-Api-Key"), r.URL.RawQuery
+		w.Header().Set("Content-Type", "image/webp")
+		w.Write([]byte("WEBP"))
+	}))
+	defer remote.Close()
+
+	c := NewClient(instance.URL, "secret")
+	_, fetch, _ := c.Poster([]Image{{CoverType: "poster", URL: "/MediaCover/89/poster.jpg", RemoteURL: remote.URL + "/p.jpg"}}, true)
+	data, err := fetch(context.Background())
+	if err != nil || string(data) != "WEBP" {
+		t.Fatalf("data=%q err=%v", data, err)
+	}
+	if remoteKey != "" || remoteQuery != "" {
+		t.Errorf("credentials reached the provider: header %q query %q", remoteKey, remoteQuery)
+	}
+	if len(hits) != 2 { // the small variant with the header and with the query; the original is skipped
+		t.Errorf("instance requests = %v", hits)
+	}
+
+	// Without a provider address the failure says what came back.
+	_, fetch, _ = c.Poster([]Image{{CoverType: "poster", URL: "/MediaCover/89/poster.jpg"}}, true)
+	_, err = fetch(context.Background())
+	if err == nil || !strings.Contains(err.Error(), "text/html") || !strings.Contains(err.Error(), "login") {
+		t.Errorf("error = %v", err)
+	}
+}