ACP2E-1353: allow image/x-icon favicon

Author: alzota

app/code/Magento/MediaStorage/Model/File/Validator/Image.php

@@ -87,20 +87,17 @@ public function __construct(
     public function isValid($filePath): bool
     {
         $fileMimeType = $this->fileMime->getMimeType($filePath);
-        $isValid = false;
+        $isValid = true;
 
         if (stripos(json_encode($this->imageMimeTypes), json_encode($fileMimeType)) !== false) {
-            $defaultAdapter = $this->config->getAdapterAlias();
             try {
-                $image = $this->imageFactory->create($filePath, $defaultAdapter);
+                $image = $this->imageFactory->create($filePath);
                 $image->open();
-                $isValid = true;
             } catch (\InvalidArgumentException $e) {
-                $adapters = $this->config->getAdapters();
-                unset($adapters[$defaultAdapter]);
-                $image = $this->imageFactory->create($filePath, array_key_first($adapters) ?? null);
-                $image->open();
-                $isValid = true;
+                if (stripos($fileMimeType, 'icon') !== false) {
+                    $image = $this->imageFactory->create($filePath, $this->getNonDefaultAdapter());
+                    $image->open();
+                }
             } catch (\Exception $e) {
                 $isValid = false;
                 $this->logger->critical($e, ['exception' => $e]);
@@ -109,4 +106,17 @@ public function isValid($filePath): bool
 
         return $isValid;
     }
+
+    /**
+     * Get non default image adapter
+     *
+     * @return string|null
+     */
+    private function getNonDefaultAdapter(): ?string
+    {
+        $defaultAdapter = $this->config->getAdapterAlias();
+        $adapters = $this->config->getAdapters();
+        unset($adapters[$defaultAdapter]);
+        return array_key_first($adapters) ?? null;
+    }
 }

app/code/Magento/MediaStorage/Test/Unit/Model/File/Validator/ImageTest.php

@@ -83,9 +83,12 @@ public function testIsValid($filePath, $mimeType, $result): void
     public function dataProviderForIsValid()
     {
         return [
-            'x-icon' => [dirname(__FILE__) . '/_files/favicon-x-icon.ico', 'image/x-icon', true],
-            'vnd-microsoft-icon' => [dirname(__FILE__) . '/_files/favicon-vnd-microsoft.ico', 'image/vnd.microsoft.icon', true],
-            'not-valid-ico' => [dirname(__FILE__) . '/_files/not-valid-file.ico', 'text/plain', false]
+            'x-icon' => [dirname(__FILE__) . '/_files/favicon-x-icon.ico',
+                'image/x-icon', true],
+            'vnd-microsoft-icon' => [dirname(__FILE__) . '/_files/favicon-vnd-microsoft.ico',
+                'image/vnd.microsoft.icon', true],
+            'not-valid-ico' => [dirname(__FILE__) . '/_files/not-valid-file.ico',
+                'text/plain', false]
         ];
     }
 }

dev/tests/integration/testsuite/Magento/Theme/Controller/Adminhtml/System/Design/ThemeControllerTest.php

@@ -5,7 +5,7 @@
  */
 namespace Magento\Theme\Controller\Adminhtml\System\Design;
 
-use Magento\Framework\Filesystem;
+use Magento\Framework\Exception\FileSystemException;
 use Magento\Framework\Filesystem\DirectoryList;
 
 /**
@@ -16,7 +16,7 @@ class ThemeControllerTest extends \Magento\TestFramework\TestCase\AbstractBacken
     public function testUploadJsAction()
     {
         $name = 'simple-js-file.js';
-        $this->createUploadFixture($name);
+        $this->createUploadFixture($name, 'application/x-javascript', 'js_files_uploader');
         $theme = $this->_objectManager->create(\Magento\Framework\View\Design\ThemeInterface::class)
             ->getCollection()
             ->getFirstItem();
@@ -28,13 +28,31 @@ public function testUploadJsAction()
         $this->assertStringContainsString($name, $output);
     }
 
+    public function testUploadFaviconAction()
+    {
+        $names = ['favicon-x-icon.ico', 'favicon-vnd-microsoft.ico'];
+        foreach ($names as $name) {
+            $this->createUploadFixture($name, 'image/vnd.microsoft.icon', 'head_shortcut_icon');
+            $theme = $this->_objectManager->create(\Magento\Framework\View\Design\ThemeInterface::class)
+                ->getCollection()
+                ->getFirstItem();
+            $this->getRequest()->setPostValue('id', $theme->getId());
+            $this->dispatch('backend/admin/design_config_fileUploader/save');
+            $output = $this->getResponse()->getBody();
+            $this->assertStringContainsString('"error":"false"', $output);
+            $this->assertStringContainsString($name, $output);
+        }
+    }
+
     /**
      * Creates a fixture for testing uploaded file
      *
      * @param string $name
+     * @params string $mimeType
      * @return void
+     * @throws FileSystemException
      */
-    private function createUploadFixture($name)
+    private function createUploadFixture($name, $mimeType, $model)
     {
         /** @var \Magento\TestFramework\App\Filesystem $filesystem */
         $filesystem = $this->_objectManager->get(\Magento\Framework\Filesystem::class);
@@ -44,11 +62,11 @@ private function createUploadFixture($name)
         $target = $tmpDir->getAbsolutePath("{$subDir}/{$name}");
         copy(__DIR__ . "/_files/{$name}", $target);
         $_FILES = [
-            'js_files_uploader' => [
-                'name' => 'simple-js-file.js',
-                'type' => 'application/x-javascript',
+            $model => [
+                'name' => $name,
+                'type' => $mimeType,
                 'tmp_name' => $target,
-                'error' => '0',
+                'error' => 'false',
                 'size' => '28',
             ],
         ];