silverstripe / silverstripe-asset-admin

Silverstripe assets gallery for asset management
BSD 3-Clause "New" or "Revised" License
20 stars 79 forks source link

UploadField does not respect max_upload_size configurations #1351

Open chrispenny opened 1 year ago

chrispenny commented 1 year ago

Preamble

Uploads made through the Asset admin area can be restricted with configuration. EG:

SilverStripe\AssetAdmin\Controller\AssetAdmin:
  max_upload_size:
    '[image]': 2M
    '[*]': 6M

(You can also set file size limits by specific file extensions)

The result is that any images being uploaded must be 2MB or smaller, and any other file type (documents, videos, etc) must be 6MB or smaller.

There is also an Upload_Validator that supports the same configuration (though I didn't really dig into where this class is used). EG:

SilverStripe\Assets\Upload_Validator:
  default_max_file_size:
    '[image]': 2M
    '[*]': 6M

Problem statement

UploadFields that have been added to (say) edit forms do not appear to respect either of these configurations, and I am able to upload images and documents that are larger than both of these limits.

We were able to determine that the UploadField goes through the UploadReceiver::constructUploadReceiver(), and this is setting the max upload size from PHP ini, rather than from our configurations:

$maxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
$maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
$this->getValidator()->setAllowedMaxFileSize(min($maxUpload, $maxPost));

Also: Even if this was being fetched from configuration, it currently wouldn't support the categorisation that the AssetAdmin does.

Feature request

Have the UploadField respect the configuration defined for AssetAdmin::$max_upload_size or Upload_Validator::$default_max_file_size (including the ability to specify limits for different categories and/or extensions).

GuySartorelli commented 1 year ago

You say feature request, but I'm going to label this a bug - I see no reason why the Upload_Validator::$default_max_file_size configuration shouldn't be respected, given that's the validator used by UploadField.

michalkleiner commented 1 year ago

Yep, that'd be a bug. The PHP values should only be used as fallbacks if there's no other config provided.