[Validator] Improved to-string conversion of the file size/size limit

symfony · May 22, 2014 · e4c6da5 · e4c6da5
1 parent bbe1045
commit e4c6da5
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 72 deletions.
diff --git a/src/Symfony/Component/Validator/Constraints/FileValidator.php b/src/Symfony/Component/Validator/Constraints/FileValidator.php
@@ -25,6 +25,16 @@
  */
 class FileValidator extends ConstraintValidator
 {
+    const KB_BYTES = 1000;
+
+    const MB_BYTES = 1000000;
+
+    private static $suffices = array(
+        1 => 'bytes',
+        self::KB_BYTES => 'kB',
+        self::MB_BYTES => 'MB',
+    );
+
     /**
      * {@inheritdoc}
      */
@@ -43,21 +53,21 @@ public function validate($value, Constraint $constraint)
                 case UPLOAD_ERR_INI_SIZE:
                     if ($constraint->maxSize) {
                         if (ctype_digit((string) $constraint->maxSize)) {
-                            $maxSize = (int) $constraint->maxSize;
+                            $limitInBytes = (int) $constraint->maxSize;
                         } elseif (preg_match('/^\d++k$/', $constraint->maxSize)) {
-                            $maxSize = $constraint->maxSize * 1000;
+                            $limitInBytes = $constraint->maxSize * self::KB_BYTES;
                         } elseif (preg_match('/^\d++M$/', $constraint->maxSize)) {
-                            $maxSize = $constraint->maxSize * 1000 * 1000;
+                            $limitInBytes = $constraint->maxSize * self::MB_BYTES;
                         } else {
                             throw new ConstraintDefinitionException(sprintf('"%s" is not a valid maximum size', $constraint->maxSize));
                         }
-                        $maxSize = min(UploadedFile::getMaxFilesize(), $maxSize);
+                        $limitInBytes = min(UploadedFile::getMaxFilesize(), $limitInBytes);
                     } else {
-                        $maxSize = UploadedFile::getMaxFilesize();
+                        $limitInBytes = UploadedFile::getMaxFilesize();
                     }
 
                     $this->context->addViolation($constraint->uploadIniSizeErrorMessage, array(
-                        '{{ limit }}' => $maxSize,
+                        '{{ limit }}' => $limitInBytes,
                         '{{ suffix }}' => 'bytes',
                     ));
 
@@ -112,27 +122,45 @@ public function validate($value, Constraint $constraint)
         }
 
         if ($constraint->maxSize) {
-            if (ctype_digit((string) $constraint->maxSize)) {
-                $size = filesize($path);
-                $limit = (int) $constraint->maxSize;
-                $suffix = 'bytes';
-            } elseif (preg_match('/^\d++k$/', $constraint->maxSize)) {
-                $size = round(filesize($path) / 1000, 2);
-                $limit = (int) $constraint->maxSize;
-                $suffix = 'KB';
+            $sizeInBytes = filesize($path);
+            $limitInBytes = (int) $constraint->maxSize;
+
+            if (preg_match('/^\d++k$/', $constraint->maxSize)) {
+                $limitInBytes *= self::KB_BYTES;
             } elseif (preg_match('/^\d++M$/', $constraint->maxSize)) {
-                $size = round(filesize($path) / (1000 * 1000), 2);
-                $limit = (int) $constraint->maxSize;
-                $suffix = 'MB';
-            } else {
+                $limitInBytes *= self::MB_BYTES;
+            } elseif (!ctype_digit((string) $constraint->maxSize)) {
                 throw new ConstraintDefinitionException(sprintf('"%s" is not a valid maximum size', $constraint->maxSize));
             }
 
-            if ($size > $limit) {
+            if ($sizeInBytes > $limitInBytes) {
+                // Convert the limit to the smallest possible number
+                // (i.e. try "MB", then "kB", then "bytes")
+                $coef = self::MB_BYTES;
+                $limitAsString = (string) ($limitInBytes / $coef);
+
+                // Restrict the limit to 2 decimals (without rounding! we
+                // need the precise value)
+                while (self::moreDecimalsThan($limitAsString, 2)) {
+                    $coef /= 1000;
+                    $limitAsString = (string) ($limitInBytes / $coef);
+                }
+
+                // Convert size to the same measure, but round to 2 decimals
+                $sizeAsString = (string) round($sizeInBytes / $coef, 2);
+
+                // If the size and limit produce the same string output
+                // (due to rounding), reduce the coefficient
+                while ($sizeAsString === $limitAsString) {
+                    $coef /= 1000;
+                    $limitAsString = (string) ($limitInBytes / $coef);
+                    $sizeAsString = (string) round($sizeInBytes / $coef, 2);
+                }
+
                 $this->context->addViolation($constraint->maxSizeMessage, array(
-                    '{{ size }}'    => $size,
-                    '{{ limit }}'   => $limit,
-                    '{{ suffix }}'  => $suffix,
+                    '{{ size }}'    => $sizeAsString,
+                    '{{ limit }}'   => $limitAsString,
+                    '{{ suffix }}'  => static::$suffices[$coef],
                     '{{ file }}'    => $path,
                 ));
 
@@ -172,4 +200,9 @@ public function validate($value, Constraint $constraint)
             }
         }
     }
+
+    private static function moreDecimalsThan($double, $numberOfDecimals)
+    {
+        return strlen((string) $double) > strlen(round($double, $numberOfDecimals));
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/FileValidatorTest.php
@@ -33,7 +33,13 @@ protected function setUp()
 
     protected function tearDown()
     {
-        fclose($this->file);
+        if (is_resource($this->file)) {
+            fclose($this->file);
+        }
+
+        if (file_exists($this->path)) {
+            unlink($this->path);
+        }
 
         $this->context = null;
         $this->validator = null;
@@ -82,90 +88,98 @@ public function testValidUploadedfile()
         $this->validator->validate($file, new File());
     }
 
-    public function testTooLargeBytes()
+    public function provideMaxSizeExceededTests()
     {
-        fwrite($this->file, str_repeat('0', 11));
+        return array(
+            array(11, 10, '11', '10', 'bytes'),
 
-        $constraint = new File(array(
-            'maxSize'           => 10,
-            'maxSizeMessage'    => 'myMessage',
-        ));
+            array(ceil(1.005*1000), ceil(1.005*1000) - 1, '1005', '1004', 'bytes'),
+            array(ceil(1.005*1000*1000), ceil(1.005*1000*1000) - 1, '1005000', '1004999', 'bytes'),
 
-        $this->context->expects($this->once())
-            ->method('addViolation')
-            ->with('myMessage', array(
-                '{{ limit }}'   => '10',
-                '{{ size }}'    => '11',
-                '{{ suffix }}'  => 'bytes',
-                '{{ file }}'    => $this->path,
-            ));
+            // round(size) == 1.01kB, limit == 1kB
+            array(ceil(1.005*1000), 1000, '1.01', '1', 'kB'),
+            array(ceil(1.005*1000), '1k', '1.01', '1', 'kB'),
 
-        $this->validator->validate($this->getFile($this->path), $constraint);
-    }
+            // round(size) == 1kB, limit == 1kB -> use bytes
+            array(ceil(1.004*1000), 1000, '1004', '1000', 'bytes'),
+            array(ceil(1.004*1000), '1k', '1004', '1000', 'bytes'),
 
-    public function testTooLargeKiloBytes()
-    {
-        fwrite($this->file, str_repeat('0', 1400));
+            array(1000 + 1, 1000, '1001', '1000', 'bytes'),
+            array(1000 + 1, '1k', '1001', '1000', 'bytes'),
 
-        $constraint = new File(array(
-            'maxSize'           => '1k',
-            'maxSizeMessage'    => 'myMessage',
-        ));
+            // round(size) == 1.01MB, limit == 1MB
+            array(ceil(1.005*1000*1000), 1000*1000, '1.01', '1', 'MB'),
+            array(ceil(1.005*1000*1000), '1000k', '1.01', '1', 'MB'),
+            array(ceil(1.005*1000*1000), '1M', '1.01', '1', 'MB'),
 
-        $this->context->expects($this->once())
-            ->method('addViolation')
-            ->with('myMessage', array(
-                '{{ limit }}'   => '1',
-                '{{ size }}'    => '1.37',
-                '{{ suffix }}'  => 'KiB',
-                '{{ file }}'    => $this->path,
-            ));
+            // round(size) == 1MB, limit == 1MB -> use kB
+            array(ceil(1.004*1000*1000), 1000*1000, '1004', '1000', 'kB'),
+            array(ceil(1.004*1000*1000), '1000k', '1004', '1000', 'kB'),
+            array(ceil(1.004*1000*1000), '1M', '1004', '1000', 'kB'),
 
-        $this->validator->validate($this->getFile($this->path), $constraint);
+            array(1000*1000 + 1, 1000*1000, '1000001', '1000000', 'bytes'),
+            array(1000*1000 + 1, '1000k', '1000001', '1000000', 'bytes'),
+            array(1000*1000 + 1, '1M', '1000001', '1000000', 'bytes'),
+        );
     }
 
-    public function testTooLargeMegaBytes()
+    /**
+     * @dataProvider provideMaxSizeExceededTests
+     */
+    public function testMaxSizeExceeded($bytesWritten, $limit, $sizeAsString, $limitAsString, $suffix)
     {
-        fwrite($this->file, str_repeat('0', 1400000));
+        fseek($this->file, $bytesWritten-1, SEEK_SET);
+        fwrite($this->file, '0');
+        fclose($this->file);
 
         $constraint = new File(array(
-            'maxSize'           => '1M',
+            'maxSize'           => $limit,
             'maxSizeMessage'    => 'myMessage',
         ));
 
         $this->context->expects($this->once())
             ->method('addViolation')
             ->with('myMessage', array(
-                '{{ limit }}'   => '1',
-                '{{ size }}'    => '1.34',
-                '{{ suffix }}'  => 'MiB',
+                '{{ limit }}'   => $limitAsString,
+                '{{ size }}'    => $sizeAsString,
+                '{{ suffix }}'  => $suffix,
                 '{{ file }}'    => $this->path,
             ));
 
         $this->validator->validate($this->getFile($this->path), $constraint);
     }
 
-    public function testMaxSizeKiloBytes()
+    public function provideMaxSizeNotExceededTests()
     {
-        fwrite($this->file, str_repeat('0', 1010));
+        return array(
+            array(10, 10),
+            array(9, 10),
 
-        $constraint = new File(array(
-            'maxSize' => '1k',
-        ));
+            array(1000, '1k'),
+            array(1000 - 1, '1k'),
 
-        $this->context->expects($this->never())->method('addViolation');
-        $this->validator->validate($this->getFile($this->path), $constraint);
+            array(1000*1000, '1M'),
+            array(1000*1000 - 1, '1M'),
+        );
     }
 
-    public function testMaxSizeMegaBytes()
+    /**
+     * @dataProvider provideMaxSizeNotExceededTests
+     */
+    public function testMaxSizeNotExceeded($bytesWritten, $limit)
     {
-        fwrite($this->file, str_repeat('0', (1024 * 1022)));
+        fseek($this->file, $bytesWritten-1, SEEK_SET);
+        fwrite($this->file, '0');
+        fclose($this->file);
 
         $constraint = new File(array(
-            'maxSize' => '1M',
+            'maxSize'           => $limit,
+            'maxSizeMessage'    => 'myMessage',
         ));
 
-        $this->context->expects($this->never())->method('addViolation');
+        $this->context->expects($this->never())
+            ->method('addViolation');
+
         $this->validator->validate($this->getFile($this->path), $constraint);
     }