Return null if MaxFileSizeBytes is 0 (#2409)

* Return "null" if MaxFileSizeBytes is 0

* Add comment and nil check (better save than sorry)

* Simplify config
This commit is contained in:
Till 2022-05-02 10:47:16 +02:00 committed by GitHub
parent bfa344e831
commit 979a551f1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 21 additions and 18 deletions

View File

@ -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 // 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( err := r.fetchRemoteFileAndStoreMetadata(
ctx, client, ctx, client,
cfg.AbsBasePath, *cfg.MaxFileSizeBytes, db, cfg.AbsBasePath, cfg.MaxFileSizeBytes, db,
cfg.ThumbnailSizes, activeThumbnailGeneration, cfg.ThumbnailSizes, activeThumbnailGeneration,
cfg.MaxThumbnailGenerators, cfg.MaxThumbnailGenerators,
) )

View File

@ -35,7 +35,7 @@ import (
// configResponse is the response to GET /_matrix/media/r0/config // configResponse is the response to GET /_matrix/media/r0/config
// https://matrix.org/docs/spec/client_server/latest#get-matrix-media-r0-config // https://matrix.org/docs/spec/client_server/latest#get-matrix-media-r0-config
type configResponse struct { type configResponse struct {
UploadSize config.FileSizeBytes `json:"m.upload.size"` UploadSize *config.FileSizeBytes `json:"m.upload.size"`
} }
// Setup registers the media API HTTP handlers // Setup registers the media API HTTP handlers
@ -73,9 +73,13 @@ func Setup(
if r := rateLimits.Limit(req); r != nil { if r := rateLimits.Limit(req); r != nil {
return *r return *r
} }
respondSize := &cfg.MaxFileSizeBytes
if cfg.MaxFileSizeBytes == 0 {
respondSize = nil
}
return util.JSONResponse{ return util.JSONResponse{
Code: http.StatusOK, Code: http.StatusOK,
JSON: configResponse{UploadSize: *cfg.MaxFileSizeBytes}, JSON: configResponse{UploadSize: respondSize},
} }
}) })

View File

@ -90,7 +90,7 @@ func parseAndValidateRequest(req *http.Request, cfg *config.MediaAPI, dev *usera
Logger: util.GetLogger(req.Context()).WithField("Origin", cfg.Matrix.ServerName), 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 return nil, resErr
} }
@ -148,20 +148,20 @@ func (r *uploadRequest) doUpload(
// r.storeFileAndMetadata(ctx, tmpDir, ...) // 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 // 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. // nested function to guarantee either storage or cleanup.
if *cfg.MaxFileSizeBytes > 0 { if cfg.MaxFileSizeBytes > 0 {
if *cfg.MaxFileSizeBytes+1 <= 0 { if cfg.MaxFileSizeBytes+1 <= 0 {
r.Logger.WithFields(log.Fields{ r.Logger.WithFields(log.Fields{
"MaxFileSizeBytes": *cfg.MaxFileSizeBytes, "MaxFileSizeBytes": cfg.MaxFileSizeBytes,
}).Warnf("Configured MaxFileSizeBytes overflows int64, defaulting to %d bytes", config.DefaultMaxFileSizeBytes) }).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) hash, bytesWritten, tmpDir, err := fileutils.WriteTempFile(ctx, reqReader, cfg.AbsBasePath)
if err != nil { if err != nil {
r.Logger.WithError(err).WithFields(log.Fields{ r.Logger.WithError(err).WithFields(log.Fields{
"MaxFileSizeBytes": *cfg.MaxFileSizeBytes, "MaxFileSizeBytes": cfg.MaxFileSizeBytes,
}).Warn("Error while transferring file") }).Warn("Error while transferring file")
return &util.JSONResponse{ return &util.JSONResponse{
Code: http.StatusBadRequest, Code: http.StatusBadRequest,
@ -170,9 +170,9 @@ func (r *uploadRequest) doUpload(
} }
// Check if temp file size exceeds max file size configuration // 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 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 // Look up the media by the file hash. If we already have the file but under a

View File

@ -36,12 +36,11 @@ func Test_uploadRequest_doUpload(t *testing.T) {
} }
maxSize := config.FileSizeBytes(8) maxSize := config.FileSizeBytes(8)
unlimitedSize := config.FileSizeBytes(0)
logger := log.New().WithField("mediaapi", "test") logger := log.New().WithField("mediaapi", "test")
testdataPath := filepath.Join(wd, "./testdata") testdataPath := filepath.Join(wd, "./testdata")
cfg := &config.MediaAPI{ cfg := &config.MediaAPI{
MaxFileSizeBytes: &maxSize, MaxFileSizeBytes: maxSize,
BasePath: config.Path(testdataPath), BasePath: config.Path(testdataPath),
AbsBasePath: config.Path(testdataPath), AbsBasePath: config.Path(testdataPath),
DynamicThumbnails: false, DynamicThumbnails: false,
@ -124,7 +123,7 @@ func Test_uploadRequest_doUpload(t *testing.T) {
ctx: context.Background(), ctx: context.Background(),
reqReader: strings.NewReader("test test test"), reqReader: strings.NewReader("test test test"),
cfg: &config.MediaAPI{ cfg: &config.MediaAPI{
MaxFileSizeBytes: &unlimitedSize, MaxFileSizeBytes: config.FileSizeBytes(0),
BasePath: config.Path(testdataPath), BasePath: config.Path(testdataPath),
AbsBasePath: config.Path(testdataPath), AbsBasePath: config.Path(testdataPath),
DynamicThumbnails: false, DynamicThumbnails: false,

View File

@ -23,7 +23,7 @@ type MediaAPI struct {
// The maximum file size in bytes that is allowed to be stored on this server. // 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 set to 0, the size is unlimited.
// Note: if max_file_size_bytes is not set, it will default to 10485760 (10MB) // 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 // Whether to dynamically generate thumbnails on-the-fly if the requested resolution is not already generated
DynamicThumbnails bool `yaml:"dynamic_thumbnails"` DynamicThumbnails bool `yaml:"dynamic_thumbnails"`
@ -48,7 +48,7 @@ func (c *MediaAPI) Defaults(generate bool) {
c.BasePath = "./media_store" c.BasePath = "./media_store"
} }
c.MaxFileSizeBytes = &DefaultMaxFileSizeBytes c.MaxFileSizeBytes = DefaultMaxFileSizeBytes
c.MaxThumbnailGenerators = 10 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.database.connection_string", string(c.Database.ConnectionString))
checkNotEmpty(configErrs, "media_api.base_path", string(c.BasePath)) 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)) checkPositive(configErrs, "media_api.max_thumbnail_generators", int64(c.MaxThumbnailGenerators))
for i, size := range c.ThumbnailSizes { for i, size := range c.ThumbnailSizes {