fix(artwork): validate each agent image URL before picking the largest

bestImageURL selected the largest by size and only then parsed it, so a malformed
largest URL (e.g. a bad percent-escape) returned nil and shadowed a valid smaller
candidate, contradicting the documented skip-unparseable behavior. Parse per
candidate and compare sizes only among URLs that parse.
This commit is contained in:
Deluan 2026-07-23 00:43:03 -04:00
parent 0781c4a9b2
commit d15d85ad0c
2 changed files with 20 additions and 12 deletions

View File

@ -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.

View File

@ -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())