[BUGFIX] Do not depend on global TSFE for link generation 36/59036/6
authorBenni Mack <benni@typo3.org>
Wed, 5 Dec 2018 10:41:42 +0000 (11:41 +0100)
committerWouter Wolters <typo3@wouterwolters.nl>
Mon, 17 Dec 2018 12:22:06 +0000 (13:22 +0100)
Handing in the dependency of TSFE into
AbstractLinkBuilder allows to use a custom TSFE
object, when working within middlewares (e.g. Redirects)
thus, not depending on global state directly.

As a drive-by-fix, using AbstractTypolinkBuilder now does not
generate a global TSFE anymore, which can have ugly side-effects.

Tests are now simplified because the original constructor can be
called directly instead of relying on $GLOBALS[TSFE].

Resolves: #87143
Releases: master, 9.5
Change-Id: I77f2da501d1a78f0579626ebb50ef47a0026f1f7
Reviewed-on: https://review.typo3.org/59036
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Andreas Wolf <andreas.wolf@typo3.org>
Reviewed-by: Jörg Bösche <typo3@joergboesche.de>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/frontend/Classes/Typolink/AbstractTypolinkBuilder.php
typo3/sysext/frontend/Tests/Unit/Typolink/AbstractTypolinkBuilderTest.php
typo3/sysext/redirects/Classes/Service/RedirectService.php

index 766b45c..96029d5 100644 (file)
@@ -5399,7 +5399,8 @@ class ContentObjectRenderer implements LoggerAwareInterface
             /** @var AbstractTypolinkBuilder $linkBuilder */
             $linkBuilder = GeneralUtility::makeInstance(
                 $GLOBALS['TYPO3_CONF_VARS']['FE']['typolinkBuilder'][$linkDetails['type']],
-                $this
+                $this,
+                $tsfe
             );
             try {
                 list($this->lastTypoLinkUrl, $linkText, $target) = $linkBuilder->build($linkDetails, $linkText, $target, $conf);
index a7bbf60..1b83a47 100644 (file)
@@ -25,7 +25,7 @@ use TYPO3\CMS\Frontend\Page\PageRepository;
 
 /**
  * Abstract class to provide proper helper for most types necessary
- * Hands in the contentobject which is needed here for all the stdWrap magic.
+ * Hands in the ContentObject and TSFE which are needed here for all the stdWrap magic.
  */
 abstract class AbstractTypolinkBuilder
 {
@@ -35,13 +35,20 @@ abstract class AbstractTypolinkBuilder
     protected $contentObjectRenderer;
 
     /**
+     * @var TypoScriptFrontendController
+     */
+    protected $typoScriptFrontendController;
+
+    /**
      * AbstractTypolinkBuilder constructor.
      *
      * @param ContentObjectRenderer $contentObjectRenderer
+     * @param TypoScriptFrontendController $typoScriptFrontendController
      */
-    public function __construct(ContentObjectRenderer $contentObjectRenderer)
+    public function __construct(ContentObjectRenderer $contentObjectRenderer, TypoScriptFrontendController $typoScriptFrontendController = null)
     {
         $this->contentObjectRenderer = $contentObjectRenderer;
+        $this->typoScriptFrontendController = $typoScriptFrontendController ?? $GLOBALS['TSFE'];
     }
 
     /**
@@ -190,20 +197,22 @@ abstract class AbstractTypolinkBuilder
      */
     public function getTypoScriptFrontendController(): TypoScriptFrontendController
     {
-        if (!$GLOBALS['TSFE']) {
-            // This usually happens when typolink is created by the TYPO3 Backend, where no TSFE object
-            // is there. This functionality is currently completely internal, as these links cannot be
-            // created properly from the Backend.
-            // However, this is added to avoid any exceptions when trying to create a link
-            $GLOBALS['TSFE'] = GeneralUtility::makeInstance(
-                TypoScriptFrontendController::class,
-                    null,
-                    GeneralUtility::_GP('id'),
-                    (int)GeneralUtility::_GP('type')
-            );
-            $GLOBALS['TSFE']->sys_page = GeneralUtility::makeInstance(PageRepository::class);
-            $GLOBALS['TSFE']->tmpl = GeneralUtility::makeInstance(TemplateService::class);
+        if ($this->typoScriptFrontendController instanceof TypoScriptFrontendController) {
+            return $this->typoScriptFrontendController;
         }
-        return $GLOBALS['TSFE'];
+
+        // This usually happens when typolink is created by the TYPO3 Backend, where no TSFE object
+        // is there. This functionality is currently completely internal, as these links cannot be
+        // created properly from the Backend.
+        // However, this is added to avoid any exceptions when trying to create a link
+        $this->typoScriptFrontendController = GeneralUtility::makeInstance(
+            TypoScriptFrontendController::class,
+            null,
+            GeneralUtility::_GP('id'),
+            (int)GeneralUtility::_GP('type')
+        );
+        $this->typoScriptFrontendController->sys_page = GeneralUtility::makeInstance(PageRepository::class);
+        $this->typoScriptFrontendController->tmpl = GeneralUtility::makeInstance(TemplateService::class);
+        return $this->typoScriptFrontendController;
     }
 }
index d79ad71..5d78cba 100644 (file)
@@ -52,7 +52,6 @@ class AbstractTypolinkBuilderTest extends UnitTestCase
             '',
             false
         );
-        $GLOBALS['TSFE'] = $this->frontendControllerMock;
     }
 
     //////////////////////
@@ -167,19 +166,16 @@ class AbstractTypolinkBuilderTest extends UnitTestCase
      */
     public function forceAbsoluteUrlReturnsCorrectAbsoluteUrl($expected, $url, array $configuration)
     {
+        $this->frontendControllerMock->absRefPrefix = '';
         $contentObjectRendererProphecy = $this->prophesize(ContentObjectRenderer::class);
         $subject = $this->getAccessibleMock(
             AbstractTypolinkBuilder::class,
             ['build'],
-            [$contentObjectRendererProphecy->reveal()],
-            '',
-            false
+            [$contentObjectRendererProphecy->reveal(), $this->frontendControllerMock]
         );
         // Force hostname
         $_SERVER['HTTP_HOST'] = 'localhost';
         $_SERVER['SCRIPT_NAME'] = '/typo3/index.php';
-        $GLOBALS['TSFE']->absRefPrefix = '';
-
         $this->assertEquals($expected, $subject->_call('forceAbsoluteUrl', $url, $configuration));
     }
 
@@ -192,9 +188,7 @@ class AbstractTypolinkBuilderTest extends UnitTestCase
         $subject = $this->getAccessibleMock(
             AbstractTypolinkBuilder::class,
             ['build'],
-            [$contentObjectRendererProphecy->reveal()],
-            '',
-            false
+            [$contentObjectRendererProphecy->reveal(), $this->frontendControllerMock]
         );
         // Force hostname
         $_SERVER['HTTP_HOST'] = 'localhost';
@@ -319,7 +313,7 @@ class AbstractTypolinkBuilderTest extends UnitTestCase
             ['config' => [ 'doctype' => $doctype]];
         $renderer = GeneralUtility::makeInstance(ContentObjectRenderer::class);
         $subject = $this->getMockBuilder(AbstractTypolinkBuilder::class)
-            ->setConstructorArgs([$renderer])
+            ->setConstructorArgs([$renderer, $this->frontendControllerMock])
             ->setMethods(['build'])
             ->getMock();
         $actual = $this->callInaccessibleMethod(
index 57dd3c4..91420e6 100644 (file)
@@ -223,7 +223,8 @@ class RedirectService implements LoggerAwareInterface
         /** @var AbstractTypolinkBuilder $linkBuilder */
         $linkBuilder = GeneralUtility::makeInstance(
             $GLOBALS['TYPO3_CONF_VARS']['FE']['typolinkBuilder'][$linkDetails['type']],
-            $GLOBALS['TSFE']->cObj
+            $GLOBALS['TSFE']->cObj,
+            $GLOBALS['TSFE']
         );
         try {
             $configuration = [