mirror of
https://github.com/navidrome/navidrome.git
synced 2026-04-26 10:30:46 +00:00
feat(artwork): make max image upload size configurable (#5335)
* feat(config): make max image upload size configurable Let max image upload size be set from config or environment instead of a fixed 10 MB cap. The upload handler still falls back to 10 MB when MaxImageUploadSize is not set. Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com> * feat(config): support human-readable MaxImageUploadSize values Max image upload size can now be configured as a readable string like 10MB or 1GB instead of raw bytes. The config load validates it at startup, and the upload handler parses it before applying request limits (10MB fallback if it fails). + MaxImageUploadSize as human-readable string + removed redundant max(1, ...) to address code review + cap memory usage of ParseMultipartForm to 10MB (address code review) Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com> * refactor(config): consolidate MaxImageUploadSize default and add tests Move the "10MB" default constant to consts.DefaultMaxImageUploadSize so both the viper default and the runtime fallback share a single source of truth. Improve the validator error message with fmt.Errorf wrapping to match the project convention (e.g. validatePurgeMissingOption). Add unit tests for validateMaxImageUploadSize (valid/invalid inputs) and maxImageUploadSize (configured, empty, invalid, raw bytes). Compute maxImageSize once at handler creation rather than per request. --------- Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com> Co-authored-by: Deluan Quintão <deluan@navidrome.org>
This commit is contained in:
parent
85e9982b43
commit
c49e5855b9
6 changed files with 93 additions and 3 deletions
|
|
@ -12,6 +12,7 @@ import (
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/bmatcuk/doublestar/v4"
|
"github.com/bmatcuk/doublestar/v4"
|
||||||
|
"github.com/dustin/go-humanize"
|
||||||
"github.com/go-viper/encoding/ini"
|
"github.com/go-viper/encoding/ini"
|
||||||
"github.com/kr/pretty"
|
"github.com/kr/pretty"
|
||||||
"github.com/navidrome/navidrome/consts"
|
"github.com/navidrome/navidrome/consts"
|
||||||
|
|
@ -80,6 +81,7 @@ type configOptions struct {
|
||||||
EnableStarRating bool
|
EnableStarRating bool
|
||||||
EnableUserEditing bool
|
EnableUserEditing bool
|
||||||
EnableArtworkUpload bool
|
EnableArtworkUpload bool
|
||||||
|
MaxImageUploadSize string
|
||||||
EnableSharing bool
|
EnableSharing bool
|
||||||
ShareURL string
|
ShareURL string
|
||||||
DefaultShareExpiration time.Duration
|
DefaultShareExpiration time.Duration
|
||||||
|
|
@ -360,6 +362,7 @@ func Load(noConfigDump bool) {
|
||||||
validateBackupSchedule,
|
validateBackupSchedule,
|
||||||
validatePlaylistsPath,
|
validatePlaylistsPath,
|
||||||
validatePurgeMissingOption,
|
validatePurgeMissingOption,
|
||||||
|
validateMaxImageUploadSize,
|
||||||
validateURL("ExtAuth.LogoutURL", Server.ExtAuth.LogoutURL),
|
validateURL("ExtAuth.LogoutURL", Server.ExtAuth.LogoutURL),
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -584,6 +587,15 @@ func validatePurgeMissingOption() error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func validateMaxImageUploadSize() error {
|
||||||
|
if _, err := humanize.ParseBytes(Server.MaxImageUploadSize); err != nil {
|
||||||
|
err = fmt.Errorf("invalid MaxImageUploadSize %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", Server.MaxImageUploadSize, err)
|
||||||
|
log.Error(err.Error())
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func validateScanSchedule() error {
|
func validateScanSchedule() error {
|
||||||
if Server.Scanner.Schedule == "0" || Server.Scanner.Schedule == "" {
|
if Server.Scanner.Schedule == "0" || Server.Scanner.Schedule == "" {
|
||||||
Server.Scanner.Schedule = ""
|
Server.Scanner.Schedule = ""
|
||||||
|
|
@ -742,6 +754,7 @@ func setViperDefaults() {
|
||||||
viper.SetDefault("enablecoveranimation", true)
|
viper.SetDefault("enablecoveranimation", true)
|
||||||
viper.SetDefault("enablenowplaying", true)
|
viper.SetDefault("enablenowplaying", true)
|
||||||
viper.SetDefault("enableartworkupload", true)
|
viper.SetDefault("enableartworkupload", true)
|
||||||
|
viper.SetDefault("maximageuploadsize", consts.DefaultMaxImageUploadSize)
|
||||||
viper.SetDefault("enablesharing", false)
|
viper.SetDefault("enablesharing", false)
|
||||||
viper.SetDefault("shareurl", "")
|
viper.SetDefault("shareurl", "")
|
||||||
viper.SetDefault("defaultshareexpiration", 8760*time.Hour)
|
viper.SetDefault("defaultshareexpiration", 8760*time.Hour)
|
||||||
|
|
|
||||||
|
|
@ -219,6 +219,37 @@ var _ = Describe("Configuration", func() {
|
||||||
|
|
||||||
})
|
})
|
||||||
|
|
||||||
|
Describe("ValidateMaxImageUploadSize", 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.ValidateMaxImageUploadSize()).To(Succeed())
|
||||||
|
},
|
||||||
|
Entry("megabytes", "10MB"),
|
||||||
|
Entry("gigabytes", "1GB"),
|
||||||
|
Entry("raw bytes", "10485760"),
|
||||||
|
Entry("mebibytes", "10MiB"),
|
||||||
|
Entry("lower case", "50mb"),
|
||||||
|
)
|
||||||
|
|
||||||
|
DescribeTable("rejects invalid size values",
|
||||||
|
func(input string) {
|
||||||
|
conf.Server.MaxImageUploadSize = input
|
||||||
|
Expect(conf.ValidateMaxImageUploadSize()).To(MatchError(ContainSubstring("invalid MaxImageUploadSize")))
|
||||||
|
},
|
||||||
|
Entry("garbage string", "not-a-size"),
|
||||||
|
Entry("negative-looking", "-10MB"),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
DescribeTable("should load configuration from",
|
DescribeTable("should load configuration from",
|
||||||
func(format string) {
|
func(format string) {
|
||||||
filename := filepath.Join("testdata", "cfg."+format)
|
filename := filepath.Join("testdata", "cfg."+format)
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,8 @@ var NormalizeSearchBackend = normalizeSearchBackend
|
||||||
|
|
||||||
var ToPascalCase = toPascalCase
|
var ToPascalCase = toPascalCase
|
||||||
|
|
||||||
|
var ValidateMaxImageUploadSize = validateMaxImageUploadSize
|
||||||
|
|
||||||
func SetLogFatal(f func(...any)) func() {
|
func SetLogFatal(f func(...any)) func() {
|
||||||
old := logFatal
|
old := logFatal
|
||||||
logFatal = f
|
logFatal = f
|
||||||
|
|
|
||||||
|
|
@ -85,7 +85,8 @@ const (
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
DefaultUICoverArtSize = 300
|
DefaultUICoverArtSize = 300
|
||||||
|
DefaultMaxImageUploadSize = "10MB"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Prometheus options
|
// Prometheus options
|
||||||
|
|
|
||||||
|
|
@ -13,14 +13,22 @@ import (
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
"github.com/dustin/go-humanize"
|
||||||
"github.com/navidrome/navidrome/conf"
|
"github.com/navidrome/navidrome/conf"
|
||||||
|
"github.com/navidrome/navidrome/consts"
|
||||||
"github.com/navidrome/navidrome/log"
|
"github.com/navidrome/navidrome/log"
|
||||||
"github.com/navidrome/navidrome/model"
|
"github.com/navidrome/navidrome/model"
|
||||||
"github.com/navidrome/navidrome/model/request"
|
"github.com/navidrome/navidrome/model/request"
|
||||||
_ "golang.org/x/image/webp"
|
_ "golang.org/x/image/webp"
|
||||||
)
|
)
|
||||||
|
|
||||||
const maxImageSize = 10 << 20 // 10MB
|
func maxImageUploadSize() int64 {
|
||||||
|
if size, err := humanize.ParseBytes(conf.Server.MaxImageUploadSize); err == nil && size > 0 {
|
||||||
|
return int64(size)
|
||||||
|
}
|
||||||
|
size, _ := humanize.ParseBytes(consts.DefaultMaxImageUploadSize)
|
||||||
|
return int64(size)
|
||||||
|
}
|
||||||
|
|
||||||
func checkImageUploadPermission(w http.ResponseWriter, r *http.Request) bool {
|
func checkImageUploadPermission(w http.ResponseWriter, r *http.Request) bool {
|
||||||
user, _ := request.UserFrom(r.Context())
|
user, _ := request.UserFrom(r.Context())
|
||||||
|
|
@ -32,13 +40,14 @@ func checkImageUploadPermission(w http.ResponseWriter, r *http.Request) bool {
|
||||||
}
|
}
|
||||||
|
|
||||||
func handleImageUpload(saveFn func(ctx context.Context, reader io.Reader, ext string) error) http.HandlerFunc {
|
func handleImageUpload(saveFn func(ctx context.Context, reader io.Reader, ext string) error) http.HandlerFunc {
|
||||||
|
maxImageSize := maxImageUploadSize()
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
ctx := r.Context()
|
ctx := r.Context()
|
||||||
if !checkImageUploadPermission(w, r) {
|
if !checkImageUploadPermission(w, r) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
r.Body = http.MaxBytesReader(w, r.Body, maxImageSize)
|
r.Body = http.MaxBytesReader(w, r.Body, maxImageSize)
|
||||||
if err := r.ParseMultipartForm(maxImageSize / 2); err != nil {
|
if err := r.ParseMultipartForm(min(maxImageSize, 10<<20)); err != nil {
|
||||||
log.Error(ctx, "Error parsing multipart form", err)
|
log.Error(ctx, "Error parsing multipart form", err)
|
||||||
http.Error(w, "file too large or invalid form", http.StatusBadRequest)
|
http.Error(w, "file too large or invalid form", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
|
|
|
||||||
34
server/nativeapi/image_upload_test.go
Normal file
34
server/nativeapi/image_upload_test.go
Normal file
|
|
@ -0,0 +1,34 @@
|
||||||
|
package nativeapi
|
||||||
|
|
||||||
|
import (
|
||||||
|
"github.com/navidrome/navidrome/conf"
|
||||||
|
"github.com/navidrome/navidrome/conf/configtest"
|
||||||
|
. "github.com/onsi/ginkgo/v2"
|
||||||
|
. "github.com/onsi/gomega"
|
||||||
|
)
|
||||||
|
|
||||||
|
var _ = Describe("maxImageUploadSize", func() {
|
||||||
|
BeforeEach(func() {
|
||||||
|
DeferCleanup(configtest.SetupConfig())
|
||||||
|
})
|
||||||
|
|
||||||
|
It("returns the configured size when valid", func() {
|
||||||
|
conf.Server.MaxImageUploadSize = "20MB"
|
||||||
|
Expect(maxImageUploadSize()).To(Equal(int64(20_000_000)))
|
||||||
|
})
|
||||||
|
|
||||||
|
It("returns the default size when config is empty", func() {
|
||||||
|
conf.Server.MaxImageUploadSize = ""
|
||||||
|
Expect(maxImageUploadSize()).To(Equal(int64(10_000_000)))
|
||||||
|
})
|
||||||
|
|
||||||
|
It("returns the default size when config is invalid", func() {
|
||||||
|
conf.Server.MaxImageUploadSize = "not-a-size"
|
||||||
|
Expect(maxImageUploadSize()).To(Equal(int64(10_000_000)))
|
||||||
|
})
|
||||||
|
|
||||||
|
It("parses raw byte values", func() {
|
||||||
|
conf.Server.MaxImageUploadSize = "52428800"
|
||||||
|
Expect(maxImageUploadSize()).To(Equal(int64(52_428_800)))
|
||||||
|
})
|
||||||
|
})
|
||||||
Loading…
Add table
Add a link
Reference in a new issue