[Validator] fix showing wrong max file size for upload errors

this was because the maxSize option wasn't parsed correctly and simply string comparision could lead to wrong results, e.g. 200 > 1000M
symfony · Mar 13, 2013 · 7216cb0 · 7216cb0
1 parent ee495f8
commit 7216cb0
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 8 deletions.
diff --git a/src/Symfony/Component/Validator/Constraints/FileValidator.php b/src/Symfony/Component/Validator/Constraints/FileValidator.php
@@ -37,8 +37,21 @@ public function validate($value, Constraint $constraint)
         if ($value instanceof UploadedFile && !$value->isValid()) {
             switch ($value->getError()) {
                 case UPLOAD_ERR_INI_SIZE:
-                    $maxSize = UploadedFile::getMaxFilesize();
-                    $maxSize = $constraint->maxSize ? min($maxSize, $constraint->maxSize) : $maxSize;
+                    if ($constraint->maxSize) {
+                        if (ctype_digit((string) $constraint->maxSize)) {
+                            $maxSize = (int) $constraint->maxSize;
+                        } elseif (preg_match('/^\d++k$/', $constraint->maxSize)) {
+                            $maxSize = $constraint->maxSize * 1024;
+                        } elseif (preg_match('/^\d++M$/', $constraint->maxSize)) {
+                            $maxSize = $constraint->maxSize * 1048576;
+                        } else {
+                            throw new ConstraintDefinitionException(sprintf('"%s" is not a valid maximum size', $constraint->maxSize));
+                        }
+                        $maxSize = min(UploadedFile::getMaxFilesize(), $maxSize);
+                    } else {
+                        $maxSize = UploadedFile::getMaxFilesize();
+                    }
+
                     $this->context->addViolation($constraint->uploadIniSizeErrorMessage, array(
                         '{{ limit }}' => $maxSize,
                         '{{ suffix }}' => 'bytes',
@@ -97,15 +110,15 @@ public function validate($value, Constraint $constraint)
         if ($constraint->maxSize) {
             if (ctype_digit((string) $constraint->maxSize)) {
                 $size = filesize($path);
-                $limit = $constraint->maxSize;
+                $limit = (int) $constraint->maxSize;
                 $suffix = 'bytes';
-            } elseif (preg_match('/^(\d+)k$/', $constraint->maxSize, $matches)) {
+            } elseif (preg_match('/^\d++k$/', $constraint->maxSize)) {
                 $size = round(filesize($path) / 1000, 2);
-                $limit = $matches[1];
+                $limit = (int) $constraint->maxSize;
                 $suffix = 'kB';
-            } elseif (preg_match('/^(\d+)M$/', $constraint->maxSize, $matches)) {
+            } elseif (preg_match('/^\d++M$/', $constraint->maxSize)) {
                 $size = round(filesize($path) / 1000000, 2);
-                $limit = $matches[1];
+                $limit = (int) $constraint->maxSize;
                 $suffix = 'MB';
             } else {
                 throw new ConstraintDefinitionException(sprintf('"%s" is not a valid maximum size', $constraint->maxSize));

diff --git a/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php
@@ -288,12 +288,13 @@ public function testInvalidWildcardMimeType()
     /**
      * @dataProvider uploadedFileErrorProvider
      */
-    public function testUploadedFileError($error, $message, array $params = array())
+    public function testUploadedFileError($error, $message, array $params = array(), $maxSize = null)
     {
         $file = new UploadedFile('/path/to/file', 'originalName', 'mime', 0, $error);
 
         $constraint = new File(array(
             $message => 'myMessage',
+            'maxSize' => $maxSize
         ));
 
         $this->context->expects($this->once())
@@ -316,10 +317,24 @@ public function uploadedFileErrorProvider()
         );
 
         if (class_exists('Symfony\Component\HttpFoundation\File\UploadedFile')) {
+            // when no maxSize is specified on constraint, it should use the ini value
             $tests[] = array(UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', array(
                 '{{ limit }}' => UploadedFile::getMaxFilesize(),
                 '{{ suffix }}' => 'bytes',
             ));
+
+            // it should use the smaller limitation (maxSize option in this case)
+            $tests[] = array(UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', array(
+                '{{ limit }}' => 1,
+                '{{ suffix }}' => 'bytes',
+            ), '1');
+
+            // it correctly parses the maxSize option and not only uses simple string comparison
+            // 1000M should be bigger than the ini value
+            $tests[] = array(UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', array(
+                '{{ limit }}' => UploadedFile::getMaxFilesize(),
+                '{{ suffix }}' => 'bytes',
+            ), '1000M');
         }
 
         return $tests;