[TASK] Inject PSR-7 data in TSFE methods 93/58093/5
authorBenni Mack <benni@typo3.org>
Thu, 30 Aug 2018 20:39:00 +0000 (22:39 +0200)
committerGeorg Ringer <georg.ringer@gmail.com>
Fri, 31 Aug 2018 07:34:02 +0000 (09:34 +0200)
The following public methods within TSFE now get a PSR-7 request object
handed in to fetch the GET / POST variables.

- $TSFE->preparePageContentGeneration(ServerRequestInterface $request)
- $TSFE->calculateLinkVars(array $queryParams)
- $TSFE->makeCacheHash()

Note: If a hook modified _GET and preparePageContentGeneration
is called from Core, the state of _GET and $request->getQueryParams()
will differ. This COULD be overcome by using GeneralUtility::_GETset()
which TYPO3 Core and extensions like realurl already use.

Resolves: #86046
Releases: master
Change-Id: Iecb333d1b501c0d14abf4d16d930e715e9bd8d2e
Reviewed-on: https://review.typo3.org/58093
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Frans Saris <franssaris@gmail.com>
Tested-by: Frans Saris <franssaris@gmail.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
typo3/sysext/core/Classes/Utility/GeneralUtility.php
typo3/sysext/core/Documentation/Changelog/master/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst [new file with mode: 0644]
typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
typo3/sysext/frontend/Classes/Http/RequestHandler.php
typo3/sysext/frontend/Classes/Middleware/PageResolver.php
typo3/sysext/frontend/Classes/Middleware/ShortcutAndMountPointRedirect.php
typo3/sysext/frontend/Tests/Unit/Controller/TypoScriptFrontendControllerTest.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentRequiredMatcher.php

index f940fdc..df62d83 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Core\Utility;
  */
 
 use GuzzleHttp\Exception\RequestException;
+use Psr\Http\Message\ServerRequestInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
 use TYPO3\CMS\Core\Cache\CacheManager;
@@ -245,6 +246,9 @@ class GeneralUtility
         } elseif (is_array($inputGet)) {
             $_GET = $inputGet;
             $GLOBALS['HTTP_GET_VARS'] = $inputGet;
+            if (isset($GLOBALS['TYPO3_REQUEST']) && $GLOBALS['TYPO3_REQUEST'] instanceof ServerRequestInterface) {
+                $GLOBALS['TYPO3_REQUEST'] = $GLOBALS['TYPO3_REQUEST']->withQueryParams($inputGet);
+            }
         }
     }
 
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst
new file mode 100644 (file)
index 0000000..0461863
--- /dev/null
@@ -0,0 +1,45 @@
+.. include:: ../../Includes.txt
+
+==========================================================================================
+Deprecation: #86046 - Additional arguments in several TypoScriptFrontendController methods
+==========================================================================================
+
+See :issue:`86046`
+
+Description
+===========
+
+The following public methods within :php:`TypoScriptFrontendController` now expect an argument:
+- :php:`makeCacheHash(ServerRequestInterface $request)`
+- :php:`calculateLinkVars(array $queryParams)`
+- :php:`preparePageContentGeneration(ServerRequestInterface $request)`
+
+This is necessary to avoid using the PHP global variables $_GET/$_POST.
+
+In addition, to be backwards-compatible with extensions previously using
+:php:`GeneralUtility::_GETset()`, this method now also updates the global PSR-7 request
+for the time being, although this method will vanish in the future.
+
+TYPO3 aims to not access global state in the future, in order to do proper "sub requests".
+
+
+Impact
+======
+
+Calling any of the methods above without a first method argument will trigger an according
+deprecation message.
+
+
+Affected Installations
+======================
+
+TYPO3 installations with extensions using these methods previously.
+
+
+Migration
+=========
+
+Inject either QueryParameters from a given PSR-7 request object or the object itself,
+by looking at the according method signature.
+
+.. index:: Frontend, FullyScanned, ext:frontend
\ No newline at end of file
index 0f2da15..da57ef4 100644 (file)
@@ -42,6 +42,7 @@ use TYPO3\CMS\Core\Error\Http\ServiceUnavailableException;
 use TYPO3\CMS\Core\Error\Http\ShortcutTargetPageNotFoundException;
 use TYPO3\CMS\Core\Exception\Page\RootLineException;
 use TYPO3\CMS\Core\Http\ImmediateResponseException;
+use TYPO3\CMS\Core\Http\ServerRequestFactory;
 use TYPO3\CMS\Core\Localization\LanguageService;
 use TYPO3\CMS\Core\Locking\Exception\LockAcquireWouldBlockException;
 use TYPO3\CMS\Core\Locking\LockFactory;
@@ -2172,14 +2173,22 @@ class TypoScriptFrontendController implements LoggerAwareInterface
      * This is used to cache pages with more parameters than just id and type.
      *
      * @see reqCHash()
+     * @param ServerRequestInterface $request
      */
-    public function makeCacheHash()
+    public function makeCacheHash(ServerRequestInterface $request = null)
     {
+        if ($request === null) {
+            trigger_error('TSFE->makeCacheHash() requires a ServerRequestInterface as first argument, add this argument in order to avoid this deprecation error.', E_USER_DEPRECATED);
+        }
         // No need to test anything if caching was already disabled.
         if ($this->no_cache && !$GLOBALS['TYPO3_CONF_VARS']['FE']['pageNotFoundOnCHashError']) {
             return;
         }
-        $GET = GeneralUtility::_GET();
+        if ($request === null) {
+            $GET = GeneralUtility::_GET();
+        } else {
+            $GET = $request->getQueryParams();
+        }
         if ($this->cHash && is_array($GET)) {
             // Make sure we use the page uid and not the page alias
             $GET['id'] = $this->id;
@@ -2935,11 +2944,17 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     }
 
     /**
-     * Calculates and sets the internal linkVars based upon the current
-     * $_GET parameters and the setting "config.linkVars".
+     * Calculates and sets the internal linkVars based upon the current request parameters
+     * and the setting "config.linkVars".
+     *
+     * @param array $queryParams $_GET (usually called with a PSR-7 $request->getQueryParams())
      */
-    public function calculateLinkVars()
+    public function calculateLinkVars(array $queryParams = null)
     {
+        if ($queryParams === null) {
+            trigger_error('Calling TSFE->calculateLinkVars() without first argument is deprecated, and needs to be an array.', E_USER_DEPRECATED);
+            $queryParams = GeneralUtility::_GET();
+        }
         $this->linkVars = '';
         if (empty($this->config['config']['linkVars'])) {
             return;
@@ -2950,7 +2965,6 @@ class TypoScriptFrontendController implements LoggerAwareInterface
         if (empty($linkVars)) {
             return;
         }
-        $getData = GeneralUtility::_GET();
         foreach ($linkVars as $linkVar) {
             $test = $value = '';
             if (preg_match('/^(.*)\\((.+)\\)$/', $linkVar, $match)) {
@@ -2961,10 +2975,10 @@ class TypoScriptFrontendController implements LoggerAwareInterface
             $keys = explode('|', $linkVar);
             $numberOfLevels = count($keys);
             $rootKey = trim($keys[0]);
-            if (!isset($getData[$rootKey])) {
+            if (!isset($queryParams[$rootKey])) {
                 continue;
             }
-            $value = $getData[$rootKey];
+            $value = $queryParams[$rootKey];
             for ($i = 1; $i < $numberOfLevels; $i++) {
                 $currentKey = trim($keys[$i]);
                 if (isset($value[$currentKey])) {
@@ -3069,11 +3083,13 @@ class TypoScriptFrontendController implements LoggerAwareInterface
      * If the current page is of type mountpoint and should be overlaid with the contents of the mountpoint page
      * and is accessed directly, the user will be redirected to the mountpoint context.
      * @internal
+     * @param ServerRequestInterface $request
+     * @return string|null
      */
-    public function getRedirectUriForMountPoint(): ?string
+    public function getRedirectUriForMountPoint(ServerRequestInterface $request): ?string
     {
         if (!empty($this->originalMountPointPage) && (int)$this->originalMountPointPage['doktype'] === PageRepository::DOKTYPE_MOUNTPOINT) {
-            return $this->getUriToCurrentPageForRedirect();
+            return $this->getUriToCurrentPageForRedirect($request);
         }
 
         return null;
@@ -3099,12 +3115,15 @@ class TypoScriptFrontendController implements LoggerAwareInterface
      *
      * If the current page is of type shortcut and accessed directly via its URL,
      * the user will be redirected to shortcut target.
+     *
      * @internal
+     * @param ServerRequestInterface $request
+     * @return string|null
      */
-    public function getRedirectUriForShortcut(): ?string
+    public function getRedirectUriForShortcut(ServerRequestInterface $request): ?string
     {
         if (!empty($this->originalShortcutPage) && $this->originalShortcutPage['doktype'] == PageRepository::DOKTYPE_SHORTCUT) {
-            return $this->getUriToCurrentPageForRedirect();
+            return $this->getUriToCurrentPageForRedirect($request);
         }
 
         return null;
@@ -3133,7 +3152,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     protected function redirectToCurrentPage()
     {
         trigger_error('Method ' . __FUNCTION__ . 'is deprecated.', E_USER_DEPRECATED);
-        $redirectUrl = $this->getUriToCurrentPageForRedirect();
+        $redirectUrl = $this->getUriToCurrentPageForRedirect($GLOBALS['TYPO3_REQUEST']);
         // Prevent redirection loop
         if (!empty($redirectUrl) && GeneralUtility::getIndpEnv('REQUEST_URI') !== '/' . $redirectUrl) {
             // redirect and exit
@@ -3144,11 +3163,12 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     /**
      * Instantiate \TYPO3\CMS\Frontend\ContentObject to generate the correct target URL
      *
+     * @param ServerRequestInterface $request
      * @return string
      */
-    protected function getUriToCurrentPageForRedirect(): string
+    protected function getUriToCurrentPageForRedirect(ServerRequestInterface $request): string
     {
-        $this->calculateLinkVars();
+        $this->calculateLinkVars($request->getQueryParams());
         $parameter = $this->page['uid'];
         if ($this->type && MathUtility::canBeInterpretedAsInteger($this->type)) {
             $parameter .= ',' . $this->type;
@@ -3402,9 +3422,15 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     /**
      * Previously located in static method in PageGenerator::init. Is solely used to set up TypoScript
      * config. options and set properties in $TSFE for that.
+     *
+     * @param ServerRequestInterface $request
      */
-    public function preparePageContentGeneration()
+    public function preparePageContentGeneration(ServerRequestInterface $request = null)
     {
+        if ($request === null) {
+            trigger_error('TSFE->preparePageContentGeneration() requires a ServerRequestInterface as first argument, add this argument in order to avoid this deprecation error.', E_USER_DEPRECATED);
+            $request = ServerRequestFactory::fromGlobals();
+        }
         $this->getTimeTracker()->push('Prepare page content generation');
         if (isset($this->page['content_from_pid']) && $this->page['content_from_pid'] > 0) {
             // make REAL copy of TSFE object - not reference!
@@ -3453,9 +3479,9 @@ class TypoScriptFrontendController implements LoggerAwareInterface
             $this->absRefPrefix = '';
         }
         $this->ATagParams = trim($this->config['config']['ATagParams'] ?? '') ? ' ' . trim($this->config['config']['ATagParams']) : '';
-        $this->initializeSearchWordData(GeneralUtility::_GP('sword_list'));
+        $this->initializeSearchWordData($request->getParsedBody()['sword_list'] ?? $request->getQueryParams()['sword_list'] ?? null);
         // linkVars
-        $this->calculateLinkVars();
+        $this->calculateLinkVars($request->getQueryParams());
         // Setting XHTML-doctype from doctype
         if (!isset($this->config['config']['xhtmlDoctype']) || !$this->config['config']['xhtmlDoctype']) {
             $this->config['config']['xhtmlDoctype'] = $this->config['config']['doctype'] ?? '';
index f00faf9..39ef14d 100644 (file)
@@ -91,7 +91,7 @@ class RequestHandler implements RequestHandlerInterface, PsrRequestHandlerInterf
         if ($controller->isGeneratePage()) {
             $this->timeTracker->push('Page generation');
             $controller->generatePage_preProcessing();
-            $controller->preparePageContentGeneration();
+            $controller->preparePageContentGeneration($request);
 
             // Content generation
             $this->timeTracker->incStackPointer();
@@ -117,7 +117,7 @@ class RequestHandler implements RequestHandlerInterface, PsrRequestHandlerInterf
         if ($controller->isINTincScript()) {
             if (!$controller->isGeneratePage()) {
                 // When page was generated, this was already called. Avoid calling this twice.
-                $controller->preparePageContentGeneration();
+                $controller->preparePageContentGeneration($request);
             }
             $this->timeTracker->push('Non-cached objects');
             $controller->INTincScript();
index e0df66b..5befaa1 100644 (file)
@@ -132,7 +132,7 @@ class PageResolver implements MiddlewareInterface
         }
 
         // Evaluate the cache hash parameter
-        $this->controller->makeCacheHash();
+        $this->controller->makeCacheHash($request);
 
         return $handler->handle($request);
     }
index 691e125..91dd092 100644 (file)
@@ -43,7 +43,7 @@ class ShortcutAndMountPointRedirect implements MiddlewareInterface
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         // Check for shortcut page and mount point redirect
-        $redirectToUri = $this->getRedirectUri();
+        $redirectToUri = $this->getRedirectUri($request);
         if ($redirectToUri !== null && $redirectToUri !== (string)$request->getUri()) {
             return new RedirectResponse($redirectToUri, 307);
         }
@@ -63,13 +63,13 @@ class ShortcutAndMountPointRedirect implements MiddlewareInterface
         return $handler->handle($request);
     }
 
-    protected function getRedirectUri(): ?string
+    protected function getRedirectUri(ServerRequestInterface $request): ?string
     {
-        $redirectToUri = $this->controller->getRedirectUriForShortcut();
+        $redirectToUri = $this->controller->getRedirectUriForShortcut($request);
         if ($redirectToUri !== null) {
             return $redirectToUri;
         }
-        return $this->controller->getRedirectUriForMountPoint();
+        return $this->controller->getRedirectUriForMountPoint($request);
     }
 
     /**
index 7e9865c..0fdfa1c 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Frontend\Tests\Unit\Controller;
  */
 
 use TYPO3\CMS\Core\Context\Context;
+use TYPO3\CMS\Core\Http\ServerRequestFactory;
 use TYPO3\CMS\Core\Page\PageRenderer;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
@@ -189,7 +190,7 @@ class TypoScriptFrontendControllerTest extends UnitTestCase
     /**
      * @return array
      */
-    public function initializeSearchWordDataInTsfeBuildsCorrectRegexDataProvider()
+    public function initializeSearchWordDataBuildsCorrectRegexDataProvider()
     {
         return [
             'one simple search word' => [
@@ -222,22 +223,26 @@ class TypoScriptFrontendControllerTest extends UnitTestCase
 
     /**
      * @test
-     * @dataProvider initializeSearchWordDataInTsfeBuildsCorrectRegexDataProvider
+     * @dataProvider initializeSearchWordDataBuildsCorrectRegexDataProvider
      *
      * @param array $searchWordGetParameters The values that should be loaded in the sword_list GET parameter.
      * @param bool $enableStandaloneSearchWords If TRUE the sword_standAlone option will be enabled.
      * @param string $expectedRegex The expected regex after processing the search words.
      */
-    public function initializeSearchWordDataInTsfeBuildsCorrectRegex(array $searchWordGetParameters, $enableStandaloneSearchWords, $expectedRegex)
+    public function initializeSearchWordDataBuildsCorrectRegex(array $searchWordGetParameters, $enableStandaloneSearchWords, $expectedRegex)
     {
         $_GET['sword_list'] = $searchWordGetParameters;
+        $_SERVER['HTTP_HOST'] = 'localhost';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
 
         $this->subject->page = [];
         if ($enableStandaloneSearchWords) {
             $this->subject->config = ['config' => ['sword_standAlone' => 1]];
         }
 
-        $this->subject->preparePageContentGeneration();
+        GeneralUtility::flushInternalRuntimeCaches();
+        $request = ServerRequestFactory::fromGlobals();
+        $this->subject->preparePageContentGeneration($request);
         $this->assertEquals($this->subject->sWordRegEx, $expectedRegex);
     }
 
@@ -369,9 +374,8 @@ class TypoScriptFrontendControllerTest extends UnitTestCase
      */
     public function calculateLinkVarsConsidersCorrectVariables(string $linkVars, array $getVars, string $expected)
     {
-        $_GET = $getVars;
         $this->subject->config['config']['linkVars'] = $linkVars;
-        $this->subject->calculateLinkVars();
+        $this->subject->calculateLinkVars($getVars);
         $this->assertEquals($expected, $this->subject->linkVars);
     }
 
index 7eb997f..4bfe58e 100644 (file)
@@ -14,4 +14,25 @@ return [
             'Deprecation-84109-DeprecateDependencyResolver.rst',
         ],
     ],
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->calculateLinkVars' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst'
+        ],
+    ],
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->makeCacheHash' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst'
+        ],
+    ],
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->preparePageContentGeneration' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst'
+        ],
+    ],
 ];