diff --git a/conf/configuration.go b/conf/configuration.go index 5122b15eb..fbbaaf252 100644 --- a/conf/configuration.go +++ b/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 { - if _, err := humanize.ParseBytes(Server.MaxImageUploadSize); err != nil { - return fmt.Errorf("invalid MaxImageUploadSize %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", Server.MaxImageUploadSize, err) +func validateByteSize(name, value string) func() error { + return func() error { + 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) diff --git a/conf/configuration_test.go b/conf/configuration_test.go index 4eaa8e3d8..ac6477572 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -336,19 +336,10 @@ var _ = Describe("Configuration", func() { }) - Describe("ValidateMaxImageUploadSize", func() { - BeforeEach(func() { - viper.Reset() - conf.SetViperDefaults() - viper.SetDefault("datafolder", GinkgoT().TempDir()) - viper.SetDefault("loglevel", "error") - conf.ResetConf() - }) - + Describe("ValidateByteSize", func() { DescribeTable("accepts valid size values", func(input string) { - conf.Server.MaxImageUploadSize = input - Expect(conf.ValidateMaxImageUploadSize()).To(Succeed()) + Expect(conf.ValidateByteSize("MaxImageSize", input)()).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.ValidateMaxImageUploadSize()).To(MatchError(ContainSubstring("invalid MaxImageUploadSize"))) + Expect(conf.ValidateByteSize("MaxImageSize", input)()).To(MatchError(ContainSubstring("invalid MaxImageSize"))) }, 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) diff --git a/conf/export_test.go b/conf/export_test.go index cbb64b3d0..d1e1a6f99 100644 --- a/conf/export_test.go +++ b/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 diff --git a/consts/consts.go b/consts/consts.go index 46906b8b5..aed8ecf66 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -112,6 +112,7 @@ const ( const ( DefaultUICoverArtSize = 300 DefaultMaxImageUploadSize = "10MB" + DefaultMaxImageSize = "20MB" ) // Prometheus options diff --git a/core/artwork/processor.go b/core/artwork/processor.go index c3cf15b71..6ce76bf45 100644 --- a/core/artwork/processor.go +++ b/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 { - return nil, fmt.Errorf("image exceeds size cap %d", maxImageBytes) + if int64(len(data)) > limit { + return nil, fmt.Errorf("image exceeds size cap %d", limit) } return data, nil } diff --git a/core/artwork/processor_test.go b/core/artwork/processor_test.go index d326dde09..8d30975cd 100644 --- a/core/artwork/processor_test.go +++ b/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))) + }) +}) diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index fc1a6a3ff..9a0962cf2 100644 --- a/core/artwork/resolve_test.go +++ b/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()) }) diff --git a/core/artwork/uploader.go b/core/artwork/uploader.go index 9c38ae2e5..76526af3c 100644 --- a/core/artwork/uploader.go +++ b/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) }