Ensure the setAllowedMaxFileSize wont allow config values to exceed PHP limits

src/Upload_Validator.php

@@ -122,7 +122,6 @@ public function getLargestAllowedMaxFileSize()
      */
     public function getAllowedMaxFileSize($ext = null)
     {
-
         // Check if there is any defined instance max file sizes
         if (empty($this->allowedMaxFileSize)) {
             // Set default max file sizes if there isn't
@@ -131,9 +130,7 @@ public function getAllowedMaxFileSize($ext = null)
                 $this->setAllowedMaxFileSize($fileSize);
             } else {
                 // When no default is present, use maximum set by PHP
-                $maxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
-                $maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
-                $this->setAllowedMaxFileSize(min($maxUpload, $maxPost));
+                $this->setAllowedMaxFileSize($this->maxUploadSizeFromPHPIni());
             }
         }
 
@@ -174,25 +171,49 @@ public function setAllowedMaxFileSize($rules)
             $finalRules = [];
 
             foreach ($rules as $rule => $value) {
-                if (is_numeric($value)) {
-                    $tmpSize = $value;
-                } else {
-                    $tmpSize = Convert::memstring2bytes($value);
-                }
-
-                $finalRules[$rule] = (int)$tmpSize;
+                $finalRules[$rule] = $this->parseAndVerifyAllowedSize($value);
             }
 
             $this->allowedMaxFileSize = $finalRules;
-        } elseif (is_string($rules)) {
-            $this->allowedMaxFileSize['*'] = Convert::memstring2bytes($rules);
-        } elseif ((int)$rules > 0) {
-            $this->allowedMaxFileSize['*'] = (int)$rules;
+        } else {
+            $this->allowedMaxFileSize['*'] = $this->parseAndVerifyAllowedSize($rules);
         }
 
         return $this;
     }
 
+    /**
+     * Converts values in bytes or INI format to bytes.
+     * Will fall back to the default PHP size if null|0|''
+     * Verify the parsed size will not exceed PHP's ini settings
+     */
+    private function parseAndVerifyAllowedSize($value): int
+    {
+        $maxPHPAllows = $this->maxUploadSizeFromPHPIni();
+
+        if (is_numeric($value)) {
+            $tmpSize = (int)$value;
+        } elseif (is_string($value)) {
+            $tmpSize = Convert::memstring2bytes($value);
+        }
+
+        if (empty($tmpSize)) {
+            $tmpSize = $maxPHPAllows;
+        }
+
+        return min($maxPHPAllows, $tmpSize);
+    }
+
+    /**
+     * Checks the two ini settings to work out the max upload size php would allow
+     */
+    private function maxUploadSizeFromPHPIni(): int
+    {
+        $maxUpload = Convert::memstring2bytes(ini_get('upload_max_filesize'));
+        $maxPost = Convert::memstring2bytes(ini_get('post_max_size'));
+        return min($maxUpload, $maxPost);
+    }
+
     /**
      * @return array
      */

tests/php/UploadTest.php

@@ -2,6 +2,8 @@
 
 namespace SilverStripe\Assets\Tests;
 
+use ReflectionMethod;
+use SilverStripe\Core\Convert;
 use Silverstripe\Assets\Dev\TestAssetStore;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Folder;
@@ -181,6 +183,65 @@ public function testAllowedFilesize()
         $this->assertFalse($result, 'Load failed because size was too big');
     }
 
+    public function testAllowedSizeWontExceedPHPConfig()
+    {
+        $v = new Upload_Validator();
+        $reflectionMethod = new ReflectionMethod($v, 'maxUploadSizeFromPHPIni');
+        $reflectionMethod->setAccessible(true);
+        $maxUploadSize = $reflectionMethod->invoke($v);
+
+        // Bump up the size by about 2MB.
+        // This would result in a hard PHP error if upload was attempted.
+        $exceedsMaxPHP = $maxUploadSize + 2000000;
+
+        $v->setAllowedMaxFileSize([
+            'jpg' => Convert::bytes2memstring($exceedsMaxPHP),
+            '*' => Convert::bytes2memstring($exceedsMaxPHP),
+        ]);
+
+        // Our config exceeded the ini settings for PHP, so fall back to that size
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('png'));
+    }
+
+    public function testWeFallBackToMaxPHPConfigIfEmptyArrayPassedForConfig()
+    {
+        $v = new Upload_Validator();
+        $reflectionMethod = new ReflectionMethod($v, 'maxUploadSizeFromPHPIni');
+        $reflectionMethod->setAccessible(true);
+        $maxUploadSize = $reflectionMethod->invoke($v);
+
+        $v->setAllowedMaxFileSize([]);
+
+        // We just fall back to default, which is the max php upload allowed
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('png'));
+    }
+
+    public function testWeHandleEmptyOrNullFileSizeValues()
+    {
+        $v = new Upload_Validator();
+        $reflectionMethod = new ReflectionMethod($v, 'maxUploadSizeFromPHPIni');
+        $reflectionMethod->setAccessible(true);
+        $maxUploadSize = $reflectionMethod->invoke($v);
+
+        /*
+         The test for the zip extension should be enabled once
+         the Convert::memstring2bytes if fixed to handle empty strings.
+         */
+        $v->setAllowedMaxFileSize([
+            'jpg' => '50',
+            'png' => 0,
+            'txt' => null,
+            // 'zip' => '',
+        ]);
+
+        $this->assertEquals(50, $v->getAllowedMaxFileSize('jpg'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('png'));
+        $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('txt'));
+        // $this->assertEquals($maxUploadSize, $v->getAllowedMaxFileSize('zip'));
+    }
+
     public function testPHPUploadErrors()
     {
         $configMaxFileSizes = ['*' => '1k'];