feat(artwork): make the artwork image size cap configurable (#5931)

* feat(artwork): make the artwork image size cap configurable

Replace the hardcoded 20MB cap on resolved image reads with a new
MaxImageSize config option. Load floors it at MaxImageUploadSize so an
accepted upload can never be too large for the resolver to read back.

* fix(conf): reject zero-valued byte-size options at startup

ParseBytes accepts "0", but parseSize silently substitutes the default
for it, so the accepted config would differ from the effective limit.

* fix(conf): reject byte-size options that overflow int64

A raw value above math.MaxInt64 parses as a valid uint64 but wraps to a
negative int64 in parseSize, giving readCapped a non-positive LimitReader
bound so every artwork read comes back empty.

conf/configuration.go

@@ -5,6 +5,7 @@ import (
 	"encoding"
 	"encoding/json"
 	"fmt"
+	"math"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -90,6 +91,7 @@ type configOptions struct {
 	EnableUserEditing               bool
 	EnableArtworkUpload             bool
 	MaxImageUploadSize              string
+	MaxImageSize                    string
 	EnableSharing                   bool
 	ShareURL                        string
 	DefaultShareExpiration          time.Duration
@@ -421,7 +423,8 @@ func Load(noConfigDump bool) {
 		validateBackupSchedule,
 		validatePlaylistsPath,
 		validatePurgeMissingOption,
-		validateMaxImageUploadSize,
+		validateByteSize("MaxImageUploadSize", Server.MaxImageUploadSize),
+		validateByteSize("MaxImageSize", Server.MaxImageSize),
 		validateURL("ExtAuth.LogoutURL", Server.ExtAuth.LogoutURL),
 	)
 	if err != nil {
@@ -481,6 +484,14 @@ func Load(noConfigDump bool) {
 		Server.UICoverArtSize = newValue
 	}
 
+	// Floor MaxImageSize at MaxImageUploadSize so accepted uploads can always be read back.
+	imgSize, _ := humanize.ParseBytes(Server.MaxImageSize)
+	uploadSize, _ := humanize.ParseBytes(Server.MaxImageUploadSize)
+	if imgSize < uploadSize {
+		log.Warn("MaxImageSize must be at least MaxImageUploadSize, raising", "value", Server.MaxImageSize, "newValue", Server.MaxImageUploadSize)
+		Server.MaxImageSize = Server.MaxImageUploadSize
+	}
+
 	// Call init hooks
 	for _, hook := range hooks {
 		hook()
@@ -806,11 +817,20 @@ func validatePurgeMissingOption() error {
 	return nil
 }
 
-func validateMaxImageUploadSize() error {
+func validateByteSize(name, value string) func() error {
-	if _, err := humanize.ParseBytes(Server.MaxImageUploadSize); err != nil {
+	return func() error {
-		return fmt.Errorf("invalid MaxImageUploadSize %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", Server.MaxImageUploadSize, err)
+		size, err := humanize.ParseBytes(value)
+		if err != nil {
+			return fmt.Errorf("invalid %s %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", name, value, err)
+		}
+		if size == 0 {
+			return fmt.Errorf("invalid %s %q: must be greater than zero", name, value)
+		}
+		if size > math.MaxInt64 {
+			return fmt.Errorf("invalid %s %q: value is too large", name, value)
+		}
+		return nil
 	}
-	return nil
 }
 
 func validateEnforceNonRootUser() error {
@@ -980,6 +1000,7 @@ func setViperDefaults() {
 	viper.SetDefault("uiplaybackreportinterval", consts.DefaultUIPlaybackReportInterval)
 	viper.SetDefault("enableartworkupload", true)
 	viper.SetDefault("maximageuploadsize", consts.DefaultMaxImageUploadSize)
+	viper.SetDefault("maximagesize", consts.DefaultMaxImageSize)
 	viper.SetDefault("enablesharing", true)
 	viper.SetDefault("shareurl", "")
 	viper.SetDefault("defaultshareexpiration", 8760*time.Hour)

conf/configuration_test.go

@@ -336,19 +336,10 @@ var _ = Describe("Configuration", func() {
 
 	})
 
-	Describe("ValidateMaxImageUploadSize", func() {
+	Describe("ValidateByteSize", func() {
-		BeforeEach(func() {
-			viper.Reset()
-			conf.SetViperDefaults()
-			viper.SetDefault("datafolder", GinkgoT().TempDir())
-			viper.SetDefault("loglevel", "error")
-			conf.ResetConf()
-		})
-
 		DescribeTable("accepts valid size values",
 			func(input string) {
-				conf.Server.MaxImageUploadSize = input
+				Expect(conf.ValidateByteSize("MaxImageSize", input)()).To(Succeed())
-				Expect(conf.ValidateMaxImageUploadSize()).To(Succeed())
 			},
 			Entry("megabytes", "10MB"),
 			Entry("gigabytes", "1GB"),
@@ -359,14 +350,39 @@ var _ = Describe("Configuration", func() {
 
 		DescribeTable("rejects invalid size values",
 			func(input string) {
-				conf.Server.MaxImageUploadSize = input
+				Expect(conf.ValidateByteSize("MaxImageSize", input)()).To(MatchError(ContainSubstring("invalid MaxImageSize")))
-				Expect(conf.ValidateMaxImageUploadSize()).To(MatchError(ContainSubstring("invalid MaxImageUploadSize")))
 			},
 			Entry("garbage string", "not-a-size"),
 			Entry("negative-looking", "-10MB"),
+			Entry("zero", "0"),
+			Entry("zero with unit", "0MB"),
+			Entry("overflows int64", "9223372036854775808"),
 		)
 	})
 
+	Describe("MaxImageSize floor", func() {
+		BeforeEach(func() {
+			viper.Reset()
+			conf.SetViperDefaults()
+			viper.SetDefault("datafolder", GinkgoT().TempDir())
+			viper.SetDefault("loglevel", "error")
+			conf.ResetConf()
+		})
+
+		It("is raised to MaxImageUploadSize when configured lower", func() {
+			viper.SetDefault("maximagesize", "5MB")
+			viper.SetDefault("maximageuploadsize", "50MB")
+			conf.Load(true)
+			Expect(conf.Server.MaxImageSize).To(Equal("50MB"))
+		})
+
+		It("keeps a larger MaxImageSize unchanged", func() {
+			viper.SetDefault("maximagesize", "30MB")
+			conf.Load(true)
+			Expect(conf.Server.MaxImageSize).To(Equal("30MB"))
+		})
+	})
+
 	Describe("EnforceNonRootUser", func() {
 		It("defaults to false", func() {
 			conf.Load(true)

conf/export_test.go

@@ -14,7 +14,7 @@ var NormalizeSearchBackend = normalizeSearchBackend
 
 var ToPascalCase = toPascalCase
 
-var ValidateMaxImageUploadSize = validateMaxImageUploadSize
+var ValidateByteSize = validateByteSize
 
 func SetRuntimeInfoForTest(goos string, euid int) func() {
 	oldGOOS := currentGOOS

consts/consts.go

@@ -112,6 +112,7 @@ const (
 const (
 	DefaultUICoverArtSize     = 300
 	DefaultMaxImageUploadSize = "10MB"
+	DefaultMaxImageSize       = "20MB"
 )
 
 // Prometheus options

core/artwork/processor.go

@@ -13,6 +13,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/core/artwork/blurhash"
 	"github.com/navidrome/navidrome/core/artwork/dominant"
 	"github.com/navidrome/navidrome/core/artwork/thumbhash"
@@ -51,7 +53,9 @@ const thumbnailSize = 100
 
 // maxImageBytes caps a resolved image read: a user-editable ExternalImageURL could point at
 // an arbitrarily large endpoint.
-const maxImageBytes = 20 << 20
+func maxImageBytes() int64 {
+	return parseSize(conf.Server.MaxImageSize, consts.DefaultMaxImageSize)
+}
 
 // maxImagePixels guards against decompression bombs: a tiny file can declare a canvas that
 // image.Decode would expand into gigabytes.
@@ -202,12 +206,13 @@ func writeAbsent(ctx context.Context, repo model.ArtworkRepository, item model.A
 }
 
 func readCapped(r io.Reader) ([]byte, error) {
-	data, err := io.ReadAll(io.LimitReader(r, maxImageBytes+1))
+	limit := maxImageBytes()
+	data, err := io.ReadAll(io.LimitReader(r, limit+1))
 	if err != nil {
 		return nil, err
 	}
-	if len(data) > maxImageBytes {
+	if int64(len(data)) > limit {
-		return nil, fmt.Errorf("image exceeds size cap %d", maxImageBytes)
+		return nil, fmt.Errorf("image exceeds size cap %d", limit)
 	}
 	return data, nil
 }

core/artwork/processor_test.go

@@ -394,7 +394,7 @@ var _ = Describe("processor.acquire", func() {
 		imgPath := filepath.Join(tmpDir, "artwork", "radio", "big_test.jpg")
 		f, err := os.Create(imgPath)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(f.Truncate(maxImageBytes + 1)).To(Succeed())
+		Expect(f.Truncate(maxImageBytes() + 1)).To(Succeed())
 		Expect(f.Close()).To(Succeed())
 
 		radioRepo := tests.CreateMockedRadioRepo()
@@ -493,3 +493,14 @@ var _ = Describe("makeThumbnail", func() {
 		Expect(thumb.Pix[3]).To(BeNumerically("==", 128))
 	})
 })
+
+var _ = Describe("maxImageBytes", func() {
+	BeforeEach(func() {
+		DeferCleanup(configtest.SetupConfig())
+	})
+
+	It("reads MaxImageSize from config", func() {
+		conf.Server.MaxImageSize = "30MB"
+		Expect(maxImageBytes()).To(Equal(int64(30_000_000)))
+	})
+})

core/artwork/resolve_test.go

@@ -625,7 +625,7 @@ var _ = Describe("decodeTile", func() {
 	})
 
 	It("rejects a tile larger than the size cap", func() {
-		data := bytes.Repeat([]byte{0}, maxImageBytes+1)
+		data := bytes.Repeat([]byte{0}, int(maxImageBytes())+1)
 		_, err := decodeTile(io.NopCloser(bytes.NewReader(data)))
 		Expect(err).To(HaveOccurred())
 	})

core/artwork/uploader.go

@@ -17,10 +17,14 @@ import (
 
 // MaxImageUploadSize returns the configured max upload size in bytes, or the built-in default.
 func MaxImageUploadSize() int64 {
-	if size, err := humanize.ParseBytes(conf.Server.MaxImageUploadSize); err == nil && size > 0 {
+	return parseSize(conf.Server.MaxImageUploadSize, consts.DefaultMaxImageUploadSize)
+}
+
+func parseSize(value, fallback string) int64 {
+	if size, err := humanize.ParseBytes(value); err == nil && size > 0 {
 		return int64(size)
 	}
-	size, _ := humanize.ParseBytes(consts.DefaultMaxImageUploadSize)
+	size, _ := humanize.ParseBytes(fallback)
 	return int64(size)
 }