Merge pull request #2988 from getkirby/fix/media-performance

Media::publish(): No longer check MIME type
getkirby · Dec 10, 2020 · 3397ee6 · 3397ee6
2 parents 660085d + 7e11bf8
commit 3397ee6
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 7 deletions.
diff --git a/src/Cms/FileRules.php b/src/Cms/FileRules.php
@@ -202,15 +202,23 @@ public static function validExtension(File $file, string $extension): bool
      * Validates the extension, MIME type and filename
      *
      * @param \Kirby\Cms\File $file
-     * @param string|null $mime If not passed, the MIME type is detected from the file
+     * @param string|null|false $mime If not passed, the MIME type is detected from the file,
+     *                                if `false`, the MIME type is not validated for performance reasons
      * @return bool
      * @throws \Kirby\Exception\InvalidArgumentException If the extension, MIME type or filename is missing or forbidden
      */
-    public static function validFile(File $file, ?string $mime = null): bool
+    public static function validFile(File $file, $mime = null): bool
     {
+        if ($mime === false) {
+            // request to skip the MIME check for performance reasons
+            $validMime = true;
+        } else {
+            $validMime = static::validMime($file, $mime ?? $file->mime());
+        }
+
         return
+            $validMime &&
             static::validExtension($file, $file->extension()) &&
-            static::validMime($file, $mime ?? $file->mime()) &&
             static::validFilename($file, $file->filename());
     }
 

diff --git a/src/Cms/Media.php b/src/Cms/Media.php
@@ -71,7 +71,7 @@ public static function link(Model $model = null, string $hash, string $filename)
     public static function publish(File $file, string $dest): bool
     {
         // never publish risky files (e.g. HTML, PHP or Apache config files)
-        FileRules::validFile($file);
+        FileRules::validFile($file, false);
 
         $src       = $file->root();
         $version   = dirname($dest);

diff --git a/tests/Cms/Files/FileRulesTest.php b/tests/Cms/Files/FileRulesTest.php
@@ -245,9 +245,9 @@ public function fileProvider()
             ['.gitignore', 'gitignore', 'application/x-git', false, 'You are not allowed to upload invisible files'],
 
             // rule order
-            ['.test.htm', 'htm', 'application/php', false, 'The extension "htm" is not allowed'],
-            ['.test.htm', 'jpg', 'application/php', false, 'You are not allowed to upload PHP files'],
-            ['.test.htm', 'jpg', 'text/plain', false, 'You are not allowed to upload invisible files'],
+            ['.test.jpg', 'jpg', 'application/php', false, 'You are not allowed to upload PHP files'],
+            ['.test.htm', 'htm', 'text/plain', false, 'The extension "htm" is not allowed'],
+            ['.test.jpg', 'jpg', 'text/plain', false, 'You are not allowed to upload invisible files'],
         ];
     }
 
@@ -275,6 +275,24 @@ public function testValidFile($filename, $extension, $mime, $expected, $message
         $this->assertTrue($result);
     }
 
+    public function testValidFileSkipMime()
+    {
+        $file = $this->getMockBuilder(File::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['filename', 'extension'])
+            ->addMethods(['mime'])
+            ->getMock();
+        $file->method('filename')->willReturn('test.jpg');
+        $file->method('extension')->willReturn('jpg');
+        $file->method('mime')->willReturn('text/html');
+
+        $this->assertTrue(FileRules::validFile($file, false));
+
+        $this->expectException('Kirby\Exception\InvalidArgumentException');
+        $this->expectExceptionMessage('The media type "text/html" is not allowed');
+        $this->assertTrue(FileRules::validFile($file));
+    }
+
     public function filenameProvider()
     {
         return [