From 979a551f1e2aeb9f3417df5e52a7279230b7a3ba Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Mon, 2 May 2022 10:47:16 +0200 Subject: [PATCH] Return `null` if MaxFileSizeBytes is 0 (#2409) * Return "null" if MaxFileSizeBytes is 0 * Add comment and nil check (better save than sorry) * Simplify config --- mediaapi/routing/download.go | 2 +- mediaapi/routing/routing.go | 8 ++++++-- mediaapi/routing/upload.go | 18 +++++++++--------- mediaapi/routing/upload_test.go | 5 ++--- setup/config/config_mediaapi.go | 6 +++--- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/mediaapi/routing/download.go b/mediaapi/routing/download.go index 5f22a9461..10b25a5cd 100644 --- a/mediaapi/routing/download.go +++ b/mediaapi/routing/download.go @@ -551,7 +551,7 @@ func (r *downloadRequest) getRemoteFile( // If we do not have a record, we need to fetch the remote file first and then respond from the local file err := r.fetchRemoteFileAndStoreMetadata( ctx, client, - cfg.AbsBasePath, *cfg.MaxFileSizeBytes, db, + cfg.AbsBasePath, cfg.MaxFileSizeBytes, db, cfg.ThumbnailSizes, activeThumbnailGeneration, cfg.MaxThumbnailGenerators, ) diff --git a/mediaapi/routing/routing.go b/mediaapi/routing/routing.go index 0e1583991..97dfd3341 100644 --- a/mediaapi/routing/routing.go +++ b/mediaapi/routing/routing.go @@ -35,7 +35,7 @@ import ( // configResponse is the response to GET /_matrix/media/r0/config // https://matrix.org/docs/spec/client_server/latest#get-matrix-media-r0-config type configResponse struct { - UploadSize config.FileSizeBytes `json:"m.upload.size"` + UploadSize *config.FileSizeBytes `json:"m.upload.size"` } // Setup registers the media API HTTP handlers @@ -73,9 +73,13 @@ func Setup( if r := rateLimits.Limit(req); r != nil { return *r } + respondSize := &cfg.MaxFileSizeBytes + if cfg.MaxFileSizeBytes == 0 { + respondSize = nil + } return util.JSONResponse{ Code: http.StatusOK, - JSON: configResponse{UploadSize: *cfg.MaxFileSizeBytes}, + JSON: configResponse{UploadSize: respondSize}, } }) diff --git a/mediaapi/routing/upload.go b/mediaapi/routing/upload.go index 972c52af0..2175648ea 100644 --- a/mediaapi/routing/upload.go +++ b/mediaapi/routing/upload.go @@ -90,7 +90,7 @@ func parseAndValidateRequest(req *http.Request, cfg *config.MediaAPI, dev *usera Logger: util.GetLogger(req.Context()).WithField("Origin", cfg.Matrix.ServerName), } - if resErr := r.Validate(*cfg.MaxFileSizeBytes); resErr != nil { + if resErr := r.Validate(cfg.MaxFileSizeBytes); resErr != nil { return nil, resErr } @@ -148,20 +148,20 @@ func (r *uploadRequest) doUpload( // r.storeFileAndMetadata(ctx, tmpDir, ...) // before you return from doUpload else we will leak a temp file. We could make this nicer with a `WithTransaction` style of // nested function to guarantee either storage or cleanup. - if *cfg.MaxFileSizeBytes > 0 { - if *cfg.MaxFileSizeBytes+1 <= 0 { + if cfg.MaxFileSizeBytes > 0 { + if cfg.MaxFileSizeBytes+1 <= 0 { r.Logger.WithFields(log.Fields{ - "MaxFileSizeBytes": *cfg.MaxFileSizeBytes, + "MaxFileSizeBytes": cfg.MaxFileSizeBytes, }).Warnf("Configured MaxFileSizeBytes overflows int64, defaulting to %d bytes", config.DefaultMaxFileSizeBytes) - cfg.MaxFileSizeBytes = &config.DefaultMaxFileSizeBytes + cfg.MaxFileSizeBytes = config.DefaultMaxFileSizeBytes } - reqReader = io.LimitReader(reqReader, int64(*cfg.MaxFileSizeBytes)+1) + reqReader = io.LimitReader(reqReader, int64(cfg.MaxFileSizeBytes)+1) } hash, bytesWritten, tmpDir, err := fileutils.WriteTempFile(ctx, reqReader, cfg.AbsBasePath) if err != nil { r.Logger.WithError(err).WithFields(log.Fields{ - "MaxFileSizeBytes": *cfg.MaxFileSizeBytes, + "MaxFileSizeBytes": cfg.MaxFileSizeBytes, }).Warn("Error while transferring file") return &util.JSONResponse{ Code: http.StatusBadRequest, @@ -170,9 +170,9 @@ func (r *uploadRequest) doUpload( } // Check if temp file size exceeds max file size configuration - if *cfg.MaxFileSizeBytes > 0 && bytesWritten > types.FileSizeBytes(*cfg.MaxFileSizeBytes) { + if cfg.MaxFileSizeBytes > 0 && bytesWritten > types.FileSizeBytes(cfg.MaxFileSizeBytes) { fileutils.RemoveDir(tmpDir, r.Logger) // delete temp file - return requestEntityTooLargeJSONResponse(*cfg.MaxFileSizeBytes) + return requestEntityTooLargeJSONResponse(cfg.MaxFileSizeBytes) } // Look up the media by the file hash. If we already have the file but under a diff --git a/mediaapi/routing/upload_test.go b/mediaapi/routing/upload_test.go index b2c2f5a44..e04c010f8 100644 --- a/mediaapi/routing/upload_test.go +++ b/mediaapi/routing/upload_test.go @@ -36,12 +36,11 @@ func Test_uploadRequest_doUpload(t *testing.T) { } maxSize := config.FileSizeBytes(8) - unlimitedSize := config.FileSizeBytes(0) logger := log.New().WithField("mediaapi", "test") testdataPath := filepath.Join(wd, "./testdata") cfg := &config.MediaAPI{ - MaxFileSizeBytes: &maxSize, + MaxFileSizeBytes: maxSize, BasePath: config.Path(testdataPath), AbsBasePath: config.Path(testdataPath), DynamicThumbnails: false, @@ -124,7 +123,7 @@ func Test_uploadRequest_doUpload(t *testing.T) { ctx: context.Background(), reqReader: strings.NewReader("test test test"), cfg: &config.MediaAPI{ - MaxFileSizeBytes: &unlimitedSize, + MaxFileSizeBytes: config.FileSizeBytes(0), BasePath: config.Path(testdataPath), AbsBasePath: config.Path(testdataPath), DynamicThumbnails: false, diff --git a/setup/config/config_mediaapi.go b/setup/config/config_mediaapi.go index 9a7d84969..c85020d2a 100644 --- a/setup/config/config_mediaapi.go +++ b/setup/config/config_mediaapi.go @@ -23,7 +23,7 @@ type MediaAPI struct { // The maximum file size in bytes that is allowed to be stored on this server. // Note: if max_file_size_bytes is set to 0, the size is unlimited. // Note: if max_file_size_bytes is not set, it will default to 10485760 (10MB) - MaxFileSizeBytes *FileSizeBytes `yaml:"max_file_size_bytes,omitempty"` + MaxFileSizeBytes FileSizeBytes `yaml:"max_file_size_bytes,omitempty"` // Whether to dynamically generate thumbnails on-the-fly if the requested resolution is not already generated DynamicThumbnails bool `yaml:"dynamic_thumbnails"` @@ -48,7 +48,7 @@ func (c *MediaAPI) Defaults(generate bool) { c.BasePath = "./media_store" } - c.MaxFileSizeBytes = &DefaultMaxFileSizeBytes + c.MaxFileSizeBytes = DefaultMaxFileSizeBytes c.MaxThumbnailGenerators = 10 } @@ -61,7 +61,7 @@ func (c *MediaAPI) Verify(configErrs *ConfigErrors, isMonolith bool) { checkNotEmpty(configErrs, "media_api.database.connection_string", string(c.Database.ConnectionString)) checkNotEmpty(configErrs, "media_api.base_path", string(c.BasePath)) - checkPositive(configErrs, "media_api.max_file_size_bytes", int64(*c.MaxFileSizeBytes)) + checkPositive(configErrs, "media_api.max_file_size_bytes", int64(c.MaxFileSizeBytes)) checkPositive(configErrs, "media_api.max_thumbnail_generators", int64(c.MaxThumbnailGenerators)) for i, size := range c.ThumbnailSizes {