Fix clobbering of the upload size validation (#10059)

* Fix clobbering of the upload size validation

When the validation is set here like this, it overrides validation which has already been setup with a simple '*' rule for the size based on PHP.

If you've defined in the sites yml config something like

    SilverStripe\Assets\Upload_Validator:
      default_max_file_size:
        '[image]': '2m'
        '*' : '1m'

then it will not be respected.

If you review SilverStripe\Assets\Upload_Validator and check the getAllowedMaxFileSize method, you'll see the sizing will be populated (if it hasn't been done before).

You can see it fail by;
- Setup a new SilverStripe site.
- Set your PHP to allow max post / max upload size of 10mb.
- Add the above config to your sites yml file and flush.
- In the CMS you'll be able to upload a 5MB file, when you shouldn't.

* Test that FileField will use size validation if defined

Couple of tests which prove a fix so the FileField and others will use the default_max_file_size setting

* Fix variable name in last commit

This is what happens when you refactor in the github window.
Fix the variable names. This will get squashed once merged.

* Updates the pr - white space and non deprecated method for byte conversion

Remove extra white space to appease the CS. Use the non deprecated method for memstring2bytes

* White space fixes for the phpcs

White space fixes for the phpcs

* Ensure that "memstring2bytes" can handle if an empty or value with no number is passed in

* DEP Bump assets constraint to ensure that change is also pulled in

---------

Co-authored-by: Guy Sartorelli <[email protected]>

composer.json

@@ -36,7 +36,7 @@
         "psr/http-message": "^1",
         "sebastian/diff": "^4.0",
         "silverstripe/config": "^2",
-        "silverstripe/assets": "^2",
+        "silverstripe/assets": "^2.2",
         "silverstripe/vendor-plugin": "^2",
         "sminnee/callbacklist": "^0.1.1",
         "symfony/cache": "^6.1",

src/Core/Convert.php

@@ -451,6 +451,10 @@ public static function memstring2bytes($memString)
         // Remove non-numeric characters from the size
         $size = preg_replace('/[^0-9\.\-]/', '', $memString ?? '');
 
+        if (empty($size)) {
+            return 0;
+        }
+
         if ($unit) {
             // Find the position of the unit in the ordered string which is the power
             // of magnitude to multiply a kilobyte by

src/Forms/UploadReceiver.php

@@ -46,11 +46,6 @@ protected function constructUploadReceiver()
         $this->getValidator()->setAllowedExtensions(
             array_filter(File::config()->allowed_extensions ?? [])
         );
-
-        // get the lower max size
-        $maxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
-        $maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
-        $this->getValidator()->setAllowedMaxFileSize(min($maxUpload, $maxPost));
     }
 
     /**

tests/php/Core/ConvertTest.php

@@ -437,7 +437,10 @@ public function memString2BytesProvider()
             'mbytes' => ['512 mbytes', 512 * 1024 * 1024],
             'megabytes' => ['512 megabytes', 512 * 1024 * 1024],
             'giga' => ['1024g', 1024 * 1024 * 1024 * 1024],
-            'G' => ['1024G', 1024 * 1024 * 1024 * 1024]
+            'G' => ['1024G', 1024 * 1024 * 1024 * 1024],
+            'blank' => ['', 0],
+            'null' => [null, 0],
+            'no_numbers' => ['k', 0],
         ];
     }
 
@@ -459,6 +462,7 @@ public function testBytes2MemString($bytes, $expected)
     public function bytes2MemStringProvider()
     {
         return [
+            [0, '0B'],
             [200, '200B'],
             [2 * 1024, '2K'],
             [512 * 1024 * 1024, '512M'],

tests/php/Forms/FileFieldTest.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\Forms\Tests;
 
 use ReflectionMethod;
+use SilverStripe\Core\Convert;
 use SilverStripe\Assets\Upload_Validator;
 use SilverStripe\Dev\FunctionalTest;
 use SilverStripe\Control\Controller;
@@ -162,4 +163,41 @@ public function testUploadMissingRequiredFile()
             'A null value was passed as parameter for an uploaded file, but the validator returned true'
         );
     }
+
+    /**
+     * Test the file size validation will use the PHP max size setting if
+     * no config for the Upload_Validator::default_max_file_size has been defined
+     */
+    public function testWeWillDefaultToPHPMaxUploadSizingForValidation()
+    {
+        // These 3 lines are how SilverStripe works out the default max upload size as defined in Upload_Validator
+        $phpMaxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
+        $maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
+        $defaultUploadSize = min($phpMaxUpload, $maxPost);
+
+        $fileField = new FileField('DemoField');
+
+        $this->assertEquals($defaultUploadSize, $fileField->getValidator()->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($defaultUploadSize, $fileField->getValidator()->getAllowedMaxFileSize('png'));
+    }
+
+    /**
+     * Test the file size validation will use the default_max_file_size validation config if defined
+     */
+    public function testWeUseConfigForSizingIfDefined()
+    {
+        $configMaxFileSizes = [
+            'jpg' => $jpgSize = '2m',
+            '*' => $defaultSize = '1m',
+        ];
+
+        Upload_Validator::config()->set('default_max_file_size', $configMaxFileSizes);
+
+        $fileField = new FileField('DemoField');
+
+        $this->assertEquals(Convert::memstring2bytes($jpgSize), $fileField->getValidator()->getAllowedMaxFileSize('jpg'));
+
+        // PNG is not explicitly defined in config, so would fall back to *
+        $this->assertEquals(Convert::memstring2bytes($defaultSize), $fileField->getValidator()->getAllowedMaxFileSize('png'));
+    }
 }