diff --git a/core/artwork/agent_images.go b/core/artwork/agent_images.go index 029235830..95991629f 100644 --- a/core/artwork/agent_images.go +++ b/core/artwork/agent_images.go @@ -25,25 +25,24 @@ func denyGate(_ string, _ func() (io.ReadCloser, string, error)) (io.ReadCloser, } // bestImageURL returns the largest-Size image URL, skipping empty or unparseable -// URLs; nil when none qualifies. +// URLs; nil when none qualifies. Parsing happens per candidate so a malformed largest +// URL never shadows a valid smaller one. func bestImageURL(imgs []agents.ExternalImage) *url.URL { - var best *agents.ExternalImage + var best *url.URL + var bestSize int for i := range imgs { if imgs[i].URL == "" { continue } - if best == nil || imgs[i].Size > best.Size { - best = &imgs[i] + u, err := url.Parse(imgs[i].URL) + if err != nil { + continue + } + if best == nil || imgs[i].Size > bestSize { + best, bestSize = u, imgs[i].Size } } - if best == nil { - return nil - } - u, err := url.Parse(best.URL) - if err != nil { - return nil - } - return u + return best } // fetchArtistImage tries each enabled artist-image agent in order, each under its own gate. diff --git a/core/artwork/agent_images_test.go b/core/artwork/agent_images_test.go index f90863aed..0bcd7667a 100644 --- a/core/artwork/agent_images_test.go +++ b/core/artwork/agent_images_test.go @@ -82,6 +82,15 @@ var _ = Describe("agent images", func() { Expect(u.String()).To(Equal("http://x/big")) }) + It("skips a malformed largest URL and falls back to a valid smaller one", func() { + u := bestImageURL([]agents.ExternalImage{ + {URL: "http://x/valid", Size: 10}, + {URL: "http://x/%zz", Size: 100}, // invalid percent-escape, largest + }) + Expect(u).ToNot(BeNil()) + Expect(u.String()).To(Equal("http://x/valid")) + }) + It("returns nil when there is no non-empty URL", func() { Expect(bestImageURL(nil)).To(BeNil()) Expect(bestImageURL([]agents.ExternalImage{{URL: "", Size: 5}})).To(BeNil())