From b789f5a39603593d675a253714a26089762617c4 Mon Sep 17 00:00:00 2001
From: Kevin Appelt <kevin.appelt@icloud.com>
Date: Fri, 12 Aug 2022 10:03:35 +0200
Subject: [PATCH] [BUGFIX] Make sure social media images gets processed if
 needed

The check if the image has to be processed has been moved to
dedicated methods to enhance readability and testability

The condition still checks if the image is wider than the defined
maximum width. The check if the entire image is used should now be
easier to understand. No offset and 100% of the width and height
of the image correspond to the full image and negated in its entirety.

Resolves: #98118
Releases: main, 11.5
Change-Id: I8283e2f74310654455fb247e84c9e4204f3e1dd4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75422
Tested-by: Sascha Egerer <sascha@sascha-egerer.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: Sascha Egerer <sascha@sascha-egerer.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
---
 .../seo/Classes/MetaTag/MetaTagGenerator.php  |  64 ++---
 .../MetaTag/MetaTagGeneratorTest.php          | 230 ++++++++++++++++++
 2 files changed, 266 insertions(+), 28 deletions(-)
 create mode 100644 typo3/sysext/seo/Tests/Functional/MetaTag/MetaTagGeneratorTest.php

diff --git a/typo3/sysext/seo/Classes/MetaTag/MetaTagGenerator.php b/typo3/sysext/seo/Classes/MetaTag/MetaTagGenerator.php
index 4093928c1726..bd1821e30f69 100644
--- a/typo3/sysext/seo/Classes/MetaTag/MetaTagGenerator.php
+++ b/typo3/sysext/seo/Classes/MetaTag/MetaTagGenerator.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Seo\MetaTag;
 
 use TYPO3\CMS\Core\Imaging\ImageManipulation\CropVariantCollection;
 use TYPO3\CMS\Core\MetaTag\MetaTagManagerRegistry;
+use TYPO3\CMS\Core\Resource\FileInterface;
 use TYPO3\CMS\Core\Resource\FileReference;
 use TYPO3\CMS\Core\Resource\ProcessedFile;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -128,45 +129,52 @@ class MetaTagGenerator
     }
 
     /**
-     * @param array $fileReferences
+     * @param list<FileReference> $fileReferences
      * @return array
      */
     protected function generateSocialImages(array $fileReferences): array
     {
         $socialImages = [];
 
-        /** @var FileReference $file */
-        foreach ($fileReferences as $file) {
-            $arguments = $file->getProperties();
-            $cropVariantCollection = CropVariantCollection::create((string)$arguments['crop']);
-            $cropVariant = ($arguments['cropVariant'] ?? false) ?: 'social';
-            $cropArea = $cropVariantCollection->getCropArea($cropVariant);
-            $crop = $cropArea->makeAbsoluteBasedOnFile($file);
-
-            $processingConfiguration = [
-                'crop' => $crop,
-                'maxWidth' => 2000,
-            ];
-
-            if ($file->getProperty('width') > $processingConfiguration['maxWidth'] || ($cropArea->getHeight() !== 1.0 && $cropArea->getWidth() !== 1.0)) {
-                $image = $file->getOriginalFile()->process(
-                    ProcessedFile::CONTEXT_IMAGECROPSCALEMASK,
-                    $processingConfiguration
-                );
-            } else {
-                $image = $file->getOriginalFile();
-            }
-
-            $imageUri = $this->imageService->getImageUri($image, true);
-
+        foreach ($fileReferences as $fileReference) {
+            $arguments = $fileReference->getProperties();
+            $image = $this->processSocialImage($fileReference);
             $socialImages[] = [
-                'url' => $imageUri,
-                'width' => floor($image->getProperty('width')),
-                'height' => floor($image->getProperty('height')),
+                'url' => $this->imageService->getImageUri($image, true),
+                'width' => floor((float)$image->getProperty('width')),
+                'height' => floor((float)$image->getProperty('height')),
                 'alternative' => $arguments['alternative'],
             ];
         }
 
         return $socialImages;
     }
+
+    protected function processSocialImage(FileReference $fileReference): FileInterface
+    {
+        $arguments = $fileReference->getProperties();
+        $cropVariantCollection = CropVariantCollection::create((string)($arguments['crop'] ?? ''));
+        $cropVariantName = ($arguments['cropVariant'] ?? false) ?: 'social';
+        $cropArea = $cropVariantCollection->getCropArea($cropVariantName);
+        $crop = $cropArea->makeAbsoluteBasedOnFile($fileReference);
+
+        $processingConfiguration = [
+            'crop' => $crop,
+            'maxWidth' => 2000,
+        ];
+
+        // The image needs to be processed if:
+        //  - the image width is greater than the defined maximum width, or
+        //  - there is a cropping other than the full image (starts at 0,0 and has a width and height of 100%) defined
+        $needsProcessing = $fileReference->getProperty('width') > $processingConfiguration['maxWidth']
+            || !$cropArea->isEmpty();
+        if (!$needsProcessing) {
+            return $fileReference->getOriginalFile();
+        }
+
+        return $fileReference->getOriginalFile()->process(
+            ProcessedFile::CONTEXT_IMAGECROPSCALEMASK,
+            $processingConfiguration
+        );
+    }
 }
diff --git a/typo3/sysext/seo/Tests/Functional/MetaTag/MetaTagGeneratorTest.php b/typo3/sysext/seo/Tests/Functional/MetaTag/MetaTagGeneratorTest.php
new file mode 100644
index 000000000000..3bd71940ec86
--- /dev/null
+++ b/typo3/sysext/seo/Tests/Functional/MetaTag/MetaTagGeneratorTest.php
@@ -0,0 +1,230 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Seo\Tests\Functional\MetaTag;
+
+use org\bovigo\vfs\vfsStream;
+use org\bovigo\vfs\vfsStreamDirectory;
+use TYPO3\CMS\Core\Imaging\ImageManipulation\Area;
+use TYPO3\CMS\Core\Imaging\ImageManipulation\CropVariantCollection;
+use TYPO3\CMS\Core\Resource\File;
+use TYPO3\CMS\Core\Resource\FileInterface;
+use TYPO3\CMS\Core\Resource\ProcessedFile;
+use TYPO3\CMS\Core\Resource\ResourceFactory;
+use TYPO3\CMS\Core\Resource\ResourceStorage;
+use TYPO3\CMS\Core\Resource\StorageRepository;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Install\Configuration\Image\GraphicsMagickPreset;
+use TYPO3\CMS\Seo\MetaTag\MetaTagGenerator;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
+
+class MetaTagGeneratorTest extends FunctionalTestCase
+{
+    private MetaTagGenerator $subject;
+    private ResourceStorage $defaultStorage;
+    private vfsStreamDirectory $temporaryFileSystem;
+    protected array $coreExtensionsToLoad = ['seo'];
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        // functional tests use GraphicMagick per default, resolve the corresponding path in current OS
+        $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_path'] = $this->determineGraphicMagickBinaryPath();
+        $this->subject = GeneralUtility::makeInstance(MetaTagGenerator::class);
+        $this->defaultStorage = GeneralUtility::makeInstance(StorageRepository::class)->getDefaultStorage();
+        $this->temporaryFileSystem = vfsStream::setup();
+    }
+
+    public static function socialImageIsProcessedDataProvider(): \Generator
+    {
+        // having a valid `crop` definition, images are only process if there's a necessity
+        yield 'social: 600x600 enforced ratio' => [
+            true,
+            ['width' => 600, 'height' => 600],
+            ['width' => 600, 'height' => 315],
+            ProcessedFile::class,
+        ];
+        yield 'social: 600x315 kept as is' => [
+            true,
+            ['width' => 600, 'height' => 315],
+            ['width' => 600, 'height' => 315],
+            File::class,
+        ];
+        yield 'social: 1200x630 kept as is' => [
+            true,
+            ['width' => 1200, 'height' => 630],
+            ['width' => 1200, 'height' => 630],
+            File::class,
+        ];
+        yield 'social: 2400x1260 limited to maxWidth, kept ratio' => [
+            true,
+            ['width' => 2400, 'height' => 1260],
+            ['width' => 2000, 'height' => 1050],
+            ProcessedFile::class,
+        ];
+        yield 'social: 3000x3000 limited to maxWidth, enforced ratio' => [
+            true,
+            ['width' => 3000, 'height' => 3000],
+            ['width' => 2000, 'height' => 1050],
+            ProcessedFile::class,
+        ];
+        yield 'social: 600x300 enforced ratio (no up-scaling)' => [
+            true,
+            ['width' => 600, 'height' => 3000],
+            ['width' => 600, 'height' => 315],
+            ProcessedFile::class,
+        ];
+        yield 'social: 3000x600 enforced ratio (no up-scaling)' => [
+            true,
+            ['width' => 3000, 'height' => 600],
+            ['width' => 1142, 'height' => 600],
+            ProcessedFile::class,
+        ];
+
+        // in case `crop` is not defined, no target ratio is defined for these images
+        // (data created prior to https://review.typo3.org/c/Packages/TYPO3.CMS/+/58774/ in v9.5.1 behaves like this)
+        yield 'empty crop: 600x600 kept as is' => [
+            false,
+            ['width' => 600, 'height' => 600],
+            ['width' => 600, 'height' => 600],
+            File::class,
+        ];
+        yield 'empty crop: 600x315 kept as is' => [
+            false,
+            ['width' => 600, 'height' => 315],
+            ['width' => 600, 'height' => 315],
+            File::class,
+        ];
+        yield 'empty crop: 1200x630 kept as is' => [
+            false,
+            ['width' => 1200, 'height' => 630],
+            ['width' => 1200, 'height' => 630],
+            File::class,
+        ];
+        yield 'empty crop: 2400x1260 limited to maxWidth' => [
+            false,
+            ['width' => 2400, 'height' => 1260],
+            ['width' => 2000, 'height' => 1050],
+            ProcessedFile::class,
+        ];
+        yield 'empty crop: 3000x3000 limited to maxWidth' => [
+            false,
+            ['width' => 3000, 'height' => 3000],
+            ['width' => 2000, 'height' => 2000],
+            ProcessedFile::class,
+        ];
+        yield 'empty crop: 600x300 kept as is' => [
+            false,
+            ['width' => 600, 'height' => 3000],
+            ['width' => 600, 'height' => 3000],
+            File::class,
+        ];
+        yield 'empty crop: 3000x600 limited to maxWidth' => [
+            false,
+            ['width' => 3000, 'height' => 600],
+            ['width' => 2000, 'height' => 400],
+            ProcessedFile::class,
+        ];
+    }
+
+    /**
+     * @param array{width: int, height: int} $imageDimension
+     * @param array{width: int, height: int} $expectedDimension
+     * @test
+     * @dataProvider socialImageIsProcessedDataProvider
+     */
+    public function socialImageIsProcessed(bool $hasCrop, array $imageDimension, array $expectedDimension, string $expectedClassName): void
+    {
+        $fileName = sprintf('test_%dx%d.png', $imageDimension['width'], $imageDimension['height']);
+        $folder = $this->defaultStorage->getFolder('/');
+        // drop file if it exists
+        $file = $folder->getFile($fileName);
+        if ($file !== null) {
+            $file->delete();
+        }
+        // create new file, fill it dummy PNG data for given dimension
+        /** @var File $file */
+        $file = $this->defaultStorage->createFile($fileName, $folder);
+        $file->setContents($this->createImagePngContent($imageDimension['width'], $imageDimension['height']));
+        // temporary file reference to an actual existing file
+        $fileReferenceProperties = [
+            'uid_local' => $file->getUid(),
+            'uid_foreign' => 0,
+            'uid' => 0,
+            'crop' => '',
+        ];
+        if ($hasCrop) {
+            $cropVariantCollection = CropVariantCollection::create('', $this->resolveCropVariantsConfiguration());
+            $cropVariantCollection = $cropVariantCollection->applyRatioRestrictionToSelectedCropArea($file);
+            $fileReferenceProperties['crop'] = (string)$cropVariantCollection;
+        }
+        $fileReference = GeneralUtility::makeInstance(ResourceFactory::class)
+            ->createFileReferenceObject($fileReferenceProperties);
+        // invoke processing of social image
+        $reflectionSubject = new \ReflectionObject($this->subject);
+        $reflectionMethod = $reflectionSubject->getMethod('processSocialImage');
+        $reflectionMethod->setAccessible(true); // no-op in PHP 8.1
+        /** @var FileInterface $processedSocialImage */
+        $processedSocialImage = $reflectionMethod->invoke($this->subject, $fileReference);
+
+        self::assertSame($expectedDimension, [
+            'width' => (int)$processedSocialImage->getProperty('width'),
+            'height' => (int)$processedSocialImage->getProperty('height'),
+        ]);
+        self::assertInstanceOf($expectedClassName, $processedSocialImage);
+    }
+
+    private function createImagePngContent(int $width, int $height): string
+    {
+        $filePath = $this->temporaryFileSystem->url() . '/image.png';
+        $gdImage = imagecreatetruecolor($width, $height);
+        imagepng($gdImage, $filePath);
+        return file_get_contents($filePath);
+    }
+
+    private function determineGraphicMagickBinaryPath(): string
+    {
+        $values = GeneralUtility::makeInstance(GraphicsMagickPreset::class)->getConfigurationValues();
+        return $values['GFX/processor_path'] ?? $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_path'];
+    }
+
+    /**
+     * A little bit like `\TYPO3\CMS\Backend\Form\Element\ImageManipulationElement::populateConfiguration`...
+     */
+    private function resolveCropVariantsConfiguration(): array
+    {
+        $config = $GLOBALS['TCA']['pages']['columns']['og_image']['config']['overrideChildTca']['columns']['crop']['config'];
+        $cropVariants = [];
+        foreach ($config['cropVariants'] as $id => $cropVariant) {
+            // Filter allowed aspect ratios
+            $cropVariant['allowedAspectRatios'] = array_filter(
+                $cropVariant['allowedAspectRatios'] ?? [],
+                static fn ($aspectRatio) => empty($aspectRatio['disabled'])
+            );
+            // Ignore disabled crop variants
+            if (!empty($cropVariant['disabled'])) {
+                continue;
+            }
+            // Enforce a crop area (default is full image)
+            if (empty($cropVariant['cropArea'])) {
+                $cropVariant['cropArea'] = Area::createEmpty()->asArray();
+            }
+            $cropVariants[$id] = $cropVariant;
+        }
+        return $cropVariants;
+    }
+}
-- 
GitLab