From 2a0218b667230c6736f3b6c772afc8fcb7d433d0 Mon Sep 17 00:00:00 2001
From: Helmut Hummel <typo3@helhum.io>
Date: Fri, 28 Feb 2020 15:15:10 +0100
Subject: [PATCH] [BUGFIX] Only allow scalar values in AssetCollector

Since AssetCollector needs to be serialized,
or to be more precise its state need to be cached as string,
we must avoid that state is stored that isn't serializable.

Therefore we adapt the API to reflect this and change
the API calls to not add non serializable objects.

With this change we also revert the previous USER_INT fix
that bound the AssetCollector unnecessarily to PageRenderer
and store it serialized in cache (the deserialization was
already part of the code of the commit that introduced
the AssetCollector).

Reverts: #90537
Resolves: #90565
Releases: master
Change-Id: I59f10b608e5f270aaacde63bcd347116910aac7a
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63488
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Susanne Moog <look@susi.dev>
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Susanne Moog <look@susi.dev>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
---
 typo3/sysext/core/Classes/Page/AssetCollector.php  | 13 ++++++++++++-
 typo3/sysext/core/Classes/Page/PageRenderer.php    | 11 ++---------
 .../extbase/Classes/Service/ImageService.php       | 14 ++++++++------
 .../ContentObject/ContentObjectRenderer.php        |  4 +++-
 .../Controller/TypoScriptFrontendController.php    |  2 +-
 .../frontend/Classes/Http/RequestHandler.php       |  3 +++
 6 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/typo3/sysext/core/Classes/Page/AssetCollector.php b/typo3/sysext/core/Classes/Page/AssetCollector.php
index d46b23eee7f9..19581c8d4512 100644
--- a/typo3/sysext/core/Classes/Page/AssetCollector.php
+++ b/typo3/sysext/core/Classes/Page/AssetCollector.php
@@ -121,14 +121,25 @@ class AssetCollector implements SingletonInterface
         return $this;
     }
 
+    /**
+     * @param string $fileName
+     * @param array $additionalInformation One dimensional hash map (array with non numerical keys) with scalar values
+     * @return AssetCollector
+     */
     public function addMedia(string $fileName, array $additionalInformation): self
     {
         $existingAdditionalInformation = $this->media[$fileName] ?? [];
-        ArrayUtility::mergeRecursiveWithOverrule($existingAdditionalInformation, $additionalInformation);
+        ArrayUtility::mergeRecursiveWithOverrule($existingAdditionalInformation, $this->ensureAllValuesAreSerializable($additionalInformation));
         $this->media[$fileName] = $existingAdditionalInformation;
         return $this;
     }
 
+    private function ensureAllValuesAreSerializable(array $additionalInformation): array
+    {
+        // Currently just filtering all non scalar values
+        return array_filter($additionalInformation, 'is_scalar');
+    }
+
     public function removeJavaScript(string $identifier): self
     {
         unset($this->javaScripts[$identifier]);
diff --git a/typo3/sysext/core/Classes/Page/PageRenderer.php b/typo3/sysext/core/Classes/Page/PageRenderer.php
index dc6110724e87..60851ddc33f2 100644
--- a/typo3/sysext/core/Classes/Page/PageRenderer.php
+++ b/typo3/sysext/core/Classes/Page/PageRenderer.php
@@ -323,11 +323,6 @@ class PageRenderer implements SingletonInterface
      */
     protected $metaTagRegistry;
 
-    /**
-     * @var AssetCollector
-     */
-    protected $assetCollector;
-
     /**
      * @var FrontendInterface
      */
@@ -353,18 +348,16 @@ class PageRenderer implements SingletonInterface
         ];
 
         $this->metaTagRegistry = GeneralUtility::makeInstance(MetaTagManagerRegistry::class);
-        $this->assetCollector = GeneralUtility::makeInstance(AssetCollector::class);
         $this->setMetaTag('name', 'generator', 'TYPO3 CMS');
     }
 
     /**
-     * Set restored meta tag managers as singletons and asset collector
+     * Set restored meta tag managers as singletons
      * so that uncached plugins can use them to add or remove meta tags
      */
     public function __wakeup()
     {
         GeneralUtility::setSingletonInstance(get_class($this->metaTagRegistry), $this->metaTagRegistry);
-        GeneralUtility::setSingletonInstance(get_class($this->assetCollector), $this->assetCollector);
     }
 
     /**
@@ -1795,7 +1788,7 @@ class PageRenderer implements SingletonInterface
             $jsInline = '';
         }
         // Use AssetRenderer to inject all JavaScripts and CSS files
-        $assetRenderer = GeneralUtility::makeInstance(AssetRenderer::class, $this->assetCollector);
+        $assetRenderer = GeneralUtility::makeInstance(AssetRenderer::class);
         $jsInline .= $assetRenderer->renderInlineJavaScript(true);
         $jsFooterInline .= $assetRenderer->renderInlineJavaScript();
         $jsFiles .= $assetRenderer->renderJavaScript(true);
diff --git a/typo3/sysext/extbase/Classes/Service/ImageService.php b/typo3/sysext/extbase/Classes/Service/ImageService.php
index db04869669fc..aff9de167de5 100644
--- a/typo3/sysext/extbase/Classes/Service/ImageService.php
+++ b/typo3/sysext/extbase/Classes/Service/ImageService.php
@@ -177,16 +177,22 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
      */
     protected function setCompatibilityValues(ProcessedFile $processedImage): void
     {
+        $imageInfoValues = $this->getCompatibilityImageResourceValues($processedImage);
         if (
             $this->environmentService->isEnvironmentInFrontendMode()
             && is_object($GLOBALS['TSFE'])
         ) {
-            $GLOBALS['TSFE']->lastImageInfo = $this->getCompatibilityImageResourceValues($processedImage);
+            // This is needed by \TYPO3\CMS\Frontend\Imaging\GifBuilder,
+            // but was never needed to be set in lastImageInfo.
+            // We set it for BC here anyway, as this TSFE property is deprecated anyway.
+            $imageInfoValues['originalFile'] = $processedImage->getOriginalFile();
+            $imageInfoValues['processedFile'] = $processedImage;
+            $GLOBALS['TSFE']->lastImageInfo = $imageInfoValues;
             $GLOBALS['TSFE']->imagesOnPage[] = $processedImage->getPublicUrl();
         }
         GeneralUtility::makeInstance(AssetCollector::class)->addMedia(
             $processedImage->getPublicUrl(),
-            $this->getCompatibilityImageResourceValues($processedImage)
+            $imageInfoValues
         );
     }
 
@@ -208,10 +214,6 @@ class ImageService implements \TYPO3\CMS\Core\SingletonInterface
             3 => $processedImage->getPublicUrl(),
             'origFile' => $originalFile->getPublicUrl(),
             'origFile_mtime' => $originalFile->getModificationTime(),
-            // This is needed by \TYPO3\CMS\Frontend\Imaging\GifBuilder,
-            // in order for the setup-array to create a unique filename hash.
-            'originalFile' => $originalFile,
-            'processedFile' => $processedImage
         ];
     }
 }
diff --git a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
index 99e24378a83d..0e3dd3a1c5ab 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
@@ -1056,9 +1056,11 @@ class ContentObjectRenderer implements LoggerAwareInterface
         } else {
             $source = $info[3];
         }
+        // Remove file objects for AssetCollector, as it only allows to store scalar values
+        unset($info['originalFile'], $info['processedFile']);
         GeneralUtility::makeInstance(AssetCollector::class)->addMedia(
             $source,
-            $info ?? []
+            $info
         );
 
         $layoutKey = $this->stdWrap($conf['layoutKey'], $conf['layoutKey.']);
diff --git a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
index f242dd4e517b..7c40e41f8d27 100644
--- a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
+++ b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
@@ -2992,7 +2992,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
         }
         if (!empty($this->config['INTincScript_ext']['assetCollector'])) {
             /** @var AssetCollector $assetCollectorr */
-            $assetCollector = unserialize($this->config['INTincScript_ext']['assetCollector'], ['allowed_classes' => false]);
+            $assetCollector = unserialize($this->config['INTincScript_ext']['assetCollector'], ['allowed_classes' => [AssetCollector::class]]);
             GeneralUtility::setSingletonInstance(AssetCollector::class, $assetCollector);
         }
 
diff --git a/typo3/sysext/frontend/Classes/Http/RequestHandler.php b/typo3/sysext/frontend/Classes/Http/RequestHandler.php
index 783d62fa3d23..83e7d7689829 100644
--- a/typo3/sysext/frontend/Classes/Http/RequestHandler.php
+++ b/typo3/sysext/frontend/Classes/Http/RequestHandler.php
@@ -24,6 +24,7 @@ use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Http\NullResponse;
 use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Core\Information\Typo3Information;
+use TYPO3\CMS\Core\Page\AssetCollector;
 use TYPO3\CMS\Core\Page\PageRenderer;
 use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
@@ -197,6 +198,8 @@ class RequestHandler implements RequestHandlerInterface
         if ($controller->isINTincScript()) {
             // Store the serialized pageRenderer in configuration
             $controller->config['INTincScript_ext']['pageRenderer'] = serialize($pageRenderer);
+            // Store the serialized AssetCollector in configuration
+            $controller->config['INTincScript_ext']['assetCollector'] = serialize(GeneralUtility::makeInstance(AssetCollector::class));
             // Render complete page, keep placeholders for JavaScript and CSS
             return $pageRenderer->renderPageWithUncachedObjects($controller->config['INTincScript_ext']['divKey']);
         }
-- 
GitLab