[TASK] Ensure HTTP RequestHandlers always return a PSR-7 Repsonse 98/55498/10
authorBenjamin Franzke <bfr@qbus.de>
Wed, 24 Jan 2018 21:25:52 +0000 (22:25 +0100)
committerSusanne Moog <susanne.moog@typo3.org>
Thu, 1 Feb 2018 18:13:09 +0000 (19:13 +0100)
This is in preparation for PSR-15 middleware support which
will require PSR-7 RespnseInterface return type declarations
for request handlers.

As TSFE powers a concept of outputting nothing [see isOutputting()]
we need to add NullResponse (which implements the PSR-7 ResponseInterface)
which Core/Bootstrap can detect, to stop invoking header() and echo.

Change-Id: Ie3169a4365a85d0472523138cc73bb47cbbcb70f
Releases: master
Resolves: #83724
Reviewed-on: https://review.typo3.org/55498
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Mathias Brodala <mbrodala@pagemachine.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
12 files changed:
typo3/sysext/backend/Classes/Http/AjaxRequestHandler.php
typo3/sysext/backend/Classes/Http/RequestHandler.php
typo3/sysext/core/Classes/Console/CommandRequestHandler.php
typo3/sysext/core/Classes/Console/RequestHandlerInterface.php
typo3/sysext/core/Classes/Core/Bootstrap.php
typo3/sysext/core/Classes/Http/NullResponse.php [new file with mode: 0644]
typo3/sysext/core/Classes/Http/RequestHandlerInterface.php
typo3/sysext/core/Documentation/Changelog/master/Important-83724-APIAndBehaviorChangeInRequestHandlerClasses.rst [new file with mode: 0644]
typo3/sysext/frontend/Classes/Http/EidRequestHandler.php
typo3/sysext/frontend/Classes/Http/RequestHandler.php
typo3/sysext/install/Classes/Http/InstallerRequestHandler.php
typo3/sysext/install/Classes/Http/RequestHandler.php

index c138d2c..27dfd1b 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Http;
 
 /*
@@ -66,12 +67,12 @@ class AjaxRequestHandler implements RequestHandlerInterface
      * Handles any AJAX request in the TYPO3 Backend
      *
      * @param ServerRequestInterface $request
-     * @return \Psr\Http\Message\ResponseInterface|null
+     * @return ResponseInterface
      */
-    public function handleRequest(ServerRequestInterface $request)
+    public function handleRequest(ServerRequestInterface $request): ResponseInterface
     {
         // First get the name of the route
-        $routePath = $request->getParsedBody()['route'] ?? $request->getQueryParams()['route'];
+        $routePath = $request->getParsedBody()['route'] ?? $request->getQueryParams()['route'] ?? '';
         $request = $request->withAttribute('routePath', $routePath);
 
         $proceedIfNoUserIsLoggedIn = $this->isLoggedInBackendUserRequired($routePath);
@@ -88,9 +89,9 @@ class AjaxRequestHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request
      * @return bool If the request is an AJAX backend request, TRUE otherwise FALSE
      */
-    public function canHandleRequest(ServerRequestInterface $request)
+    public function canHandleRequest(ServerRequestInterface $request): bool
     {
-        $routePath = $request->getParsedBody()['route'] ?? $request->getQueryParams()['route'];
+        $routePath = $request->getParsedBody()['route'] ?? $request->getQueryParams()['route'] ?? '';
         return strpos($routePath, '/ajax/') === 0;
     }
 
@@ -99,7 +100,7 @@ class AjaxRequestHandler implements RequestHandlerInterface
      *
      * @return int The priority of the request handler.
      */
-    public function getPriority()
+    public function getPriority(): int
     {
         return 80;
     }
@@ -111,7 +112,7 @@ class AjaxRequestHandler implements RequestHandlerInterface
      * @param string $routePath the Route path to check against, something like '
      * @return bool whether the request can proceed without a login required
      */
-    protected function isLoggedInBackendUserRequired($routePath)
+    protected function isLoggedInBackendUserRequired(string $routePath): bool
     {
         return in_array($routePath, $this->publicAjaxRoutes, true);
     }
@@ -121,7 +122,7 @@ class AjaxRequestHandler implements RequestHandlerInterface
      *
      * @param bool $proceedIfNoUserIsLoggedIn a flag if a backend user is required
      */
-    protected function boot($proceedIfNoUserIsLoggedIn)
+    protected function boot(bool $proceedIfNoUserIsLoggedIn)
     {
         $this->bootstrap
             ->checkLockedBackendAndRedirectOrDie($proceedIfNoUserIsLoggedIn)
@@ -146,7 +147,7 @@ class AjaxRequestHandler implements RequestHandlerInterface
      * @throws ResourceNotFoundException if no valid route was found
      * @throws InvalidRequestTokenException if the request could not be verified
      */
-    protected function dispatch(ServerRequestInterface $request)
+    protected function dispatch(ServerRequestInterface $request): ResponseInterface
     {
         /** @var Response $response */
         $response = GeneralUtility::makeInstance(Response::class, 'php://temp', 200, [
index a24905f..0b7daf6 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Http;
 
 /*
@@ -18,6 +19,7 @@ use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Routing\Exception\InvalidRequestTokenException;
 use TYPO3\CMS\Core\Core\Bootstrap;
+use TYPO3\CMS\Core\Http\RedirectResponse;
 use TYPO3\CMS\Core\Http\RequestHandlerInterface;
 use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -57,7 +59,7 @@ class RequestHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request
      * @return ResponseInterface
      */
-    public function handleRequest(ServerRequestInterface $request)
+    public function handleRequest(ServerRequestInterface $request): ResponseInterface
     {
         // Check if a module URL is requested and deprecate this call
         $moduleName = $request->getQueryParams()['M'] ?? $request->getParsedBody()['M'] ?? null;
@@ -81,7 +83,7 @@ class RequestHandler implements RequestHandlerInterface
         } catch (InvalidRequestTokenException $e) {
             // When token was invalid redirect to login
             $url = GeneralUtility::getIndpEnv('TYPO3_SITE_URL') . TYPO3_mainDir;
-            \TYPO3\CMS\Core\Utility\HttpUtility::redirect($url);
+            return new RedirectResponse($url);
         }
     }
 
@@ -90,7 +92,7 @@ class RequestHandler implements RequestHandlerInterface
      *
      * @param bool $proceedIfNoUserIsLoggedIn option to allow to render the request even if no user is logged in
      */
-    protected function boot($proceedIfNoUserIsLoggedIn)
+    protected function boot(bool $proceedIfNoUserIsLoggedIn)
     {
         $this->bootstrap
             ->checkLockedBackendAndRedirectOrDie()
@@ -113,7 +115,7 @@ class RequestHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request
      * @return bool If the request is not a CLI script, TRUE otherwise FALSE
      */
-    public function canHandleRequest(ServerRequestInterface $request)
+    public function canHandleRequest(ServerRequestInterface $request): bool
     {
         return TYPO3_REQUESTTYPE & TYPO3_REQUESTTYPE_BE && !(TYPO3_REQUESTTYPE & TYPO3_REQUESTTYPE_CLI);
     }
@@ -124,7 +126,7 @@ class RequestHandler implements RequestHandlerInterface
      *
      * @return int The priority of the request handler.
      */
-    public function getPriority()
+    public function getPriority(): int
     {
         return 50;
     }
@@ -137,7 +139,7 @@ class RequestHandler implements RequestHandlerInterface
      * @throws InvalidRequestTokenException if the request could not be verified
      * @throws \InvalidArgumentException when a route is found but the target of the route cannot be called
      */
-    protected function dispatch($request)
+    protected function dispatch(ServerRequestInterface $request): ResponseInterface
     {
         /** @var Response $response */
         $response = GeneralUtility::makeInstance(Response::class);
index 0655cea..8d4ccc9 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Core\Console;
 
 /*
@@ -79,7 +80,7 @@ class CommandRequestHandler implements RequestHandlerInterface
      * @param InputInterface $input
      * @return bool Always TRUE
      */
-    public function canHandleRequest(InputInterface $input)
+    public function canHandleRequest(InputInterface $input): bool
     {
         return true;
     }
@@ -89,7 +90,7 @@ class CommandRequestHandler implements RequestHandlerInterface
      *
      * @return int The priority of the request handler.
      */
-    public function getPriority()
+    public function getPriority(): int
     {
         return 50;
     }
index 4dee4e5..dc6e949 100644 (file)
@@ -351,7 +351,7 @@ class Bootstrap
      */
     protected function sendResponse()
     {
-        if ($this->response instanceof \Psr\Http\Message\ResponseInterface) {
+        if ($this->response instanceof \Psr\Http\Message\ResponseInterface && !($this->response instanceof \TYPO3\CMS\Core\Http\NullResponse)) {
             if (!headers_sent()) {
                 // If the response code was not changed by legacy code (still is 200)
                 // then allow the PSR-7 response object to explicitly set it.
diff --git a/typo3/sysext/core/Classes/Http/NullResponse.php b/typo3/sysext/core/Classes/Http/NullResponse.php
new file mode 100644 (file)
index 0000000..4550302
--- /dev/null
@@ -0,0 +1,25 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Core\Http;
+
+/*
+ * 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!
+ */
+
+/**
+ * A null response object
+ *
+ * @internal Note that this is not public API yet.
+ */
+class NullResponse extends Response
+{
+}
index c2a945b..185aec5 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Core\Http;
 
 /*
@@ -14,6 +15,9 @@ namespace TYPO3\CMS\Core\Http;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+
 /**
  * The interface for a request handler
  * see RequestHandler in EXT:backend/Classes/Http/ and EXT:frontend/Classes/Http
@@ -25,20 +29,20 @@ interface RequestHandlerInterface
     /**
      * Handles a raw request
      *
-     * @param \Psr\Http\Message\ServerRequestInterface $request
-     * @return \Psr\Http\Message\ResponseInterface|null
+     * @param ServerRequestInterface $request
+     * @return ResponseInterface
      * @api
      */
-    public function handleRequest(\Psr\Http\Message\ServerRequestInterface $request);
+    public function handleRequest(ServerRequestInterface $request);
 
     /**
      * Checks if the request handler can handle the given request.
      *
-     * @param \Psr\Http\Message\ServerRequestInterface $request
+     * @param ServerRequestInterface $request
      * @return bool TRUE if it can handle the request, otherwise FALSE
      * @api
      */
-    public function canHandleRequest(\Psr\Http\Message\ServerRequestInterface $request);
+    public function canHandleRequest(ServerRequestInterface $request);
 
     /**
      * Returns the priority - how eager the handler is to actually handle the
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Important-83724-APIAndBehaviorChangeInRequestHandlerClasses.rst b/typo3/sysext/core/Documentation/Changelog/master/Important-83724-APIAndBehaviorChangeInRequestHandlerClasses.rst
new file mode 100644 (file)
index 0000000..858bfd3
--- /dev/null
@@ -0,0 +1,29 @@
+.. include:: ../../Includes.txt
+
+======================================================================
+Important: #83724 - API and behavior change in request handler classes
+======================================================================
+
+See :issue:`83724`
+
+Description
+===========
+
+In preparation for a better PSR-7 and a new PSR-15 integration the internal request handler classes where changed:
+
+* All methods gained strict argument and return type declarations.
+* Instead of calling :php:`HttpUtility::redirect()` a :php:`RedirectResponse` is returned.
+* Instead of returning :php:`null` a :php:`NullResponse` is returned.
+
+Impact
+======
+
+Extending one of the core request handlers without adding type declarations (to overwritten methods),
+will trigger a PHP fatal error.
+
+Affected Installations
+======================
+
+All 3rd party extensions extending one of the core request handlers.
+
+.. index:: PHP-API, NotScanned
index f9bd3b9..9c5498a 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Frontend\Http;
 
 /*
@@ -14,10 +15,12 @@ namespace TYPO3\CMS\Frontend\Http;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Core\Core\Bootstrap;
 use TYPO3\CMS\Core\Exception;
 use TYPO3\CMS\Core\Http\Dispatcher;
+use TYPO3\CMS\Core\Http\NullResponse;
 use TYPO3\CMS\Core\Http\RequestHandlerInterface;
 use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
@@ -49,9 +52,9 @@ class EidRequestHandler implements RequestHandlerInterface
      * Handles a frontend request based on the _GP "eID" variable.
      *
      * @param ServerRequestInterface $request
-     * @return \Psr\Http\Message\ResponseInterface|null
+     * @return ResponseInterface
      */
-    public function handleRequest(ServerRequestInterface $request)
+    public function handleRequest(ServerRequestInterface $request): ResponseInterface
     {
         // Starting time tracking
         $configuredCookieName = trim($GLOBALS['TYPO3_CONF_VARS']['BE']['cookieName']) ?: 'be_typo_user';
@@ -77,7 +80,7 @@ class EidRequestHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request The request to process
      * @return bool If the request is not an eID request, TRUE otherwise FALSE
      */
-    public function canHandleRequest(ServerRequestInterface $request)
+    public function canHandleRequest(ServerRequestInterface $request): bool
     {
         return !empty($request->getQueryParams()['eID']) || !empty($request->getParsedBody()['eID']);
     }
@@ -88,7 +91,7 @@ class EidRequestHandler implements RequestHandlerInterface
      *
      * @return int The priority of the request handler.
      */
-    public function getPriority()
+    public function getPriority(): int
     {
         return 80;
     }
@@ -97,10 +100,10 @@ class EidRequestHandler implements RequestHandlerInterface
      * Dispatches the request to the corresponding eID class or eID script
      *
      * @param ServerRequestInterface $request
-     * @return \Psr\Http\Message\ResponseInterface|null
+     * @return ResponseInterface
      * @throws Exception
      */
-    protected function dispatch($request)
+    protected function dispatch(ServerRequestInterface $request): ResponseInterface
     {
         /** @var Response $response */
         $response = GeneralUtility::makeInstance(Response::class);
@@ -126,6 +129,6 @@ class EidRequestHandler implements RequestHandlerInterface
             throw new Exception('Registered eID has invalid script path.', 1416391467);
         }
         include $scriptPath;
-        return null;
+        return new NullResponse();
     }
 }
index 320b08e..62d04fe 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Frontend\Http;
 
 /*
@@ -14,9 +15,12 @@ namespace TYPO3\CMS\Frontend\Http;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\FrontendBackendUserAuthentication;
 use TYPO3\CMS\Core\Core\Bootstrap;
 use TYPO3\CMS\Core\FrontendEditing\FrontendEditingController;
+use TYPO3\CMS\Core\Http\NullResponse;
 use TYPO3\CMS\Core\Http\RequestHandlerInterface;
 use TYPO3\CMS\Core\Log\LogManager;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
@@ -58,7 +62,7 @@ class RequestHandler implements RequestHandlerInterface
 
     /**
      * The request handed over
-     * @var \Psr\Http\Message\ServerRequestInterface
+     * @var ServerRequestInterface
      */
     protected $request;
 
@@ -75,10 +79,10 @@ class RequestHandler implements RequestHandlerInterface
     /**
      * Handles a frontend request
      *
-     * @param \Psr\Http\Message\ServerRequestInterface $request
-     * @return \Psr\Http\Message\ResponseInterface|null
+     * @param ServerRequestInterface $request
+     * @return ResponseInterface
      */
-    public function handleRequest(\Psr\Http\Message\ServerRequestInterface $request)
+    public function handleRequest(ServerRequestInterface $request): ResponseInterface
     {
         $response = null;
         $this->request = $request;
@@ -274,16 +278,17 @@ class RequestHandler implements RequestHandlerInterface
         }
         GeneralUtility::makeInstance(LogManager::class)
             ->getLogger(get_class())->debug('END of FRONTEND session', ['_FLUSH' => true]);
-        return $response;
+
+        return $response ?: new NullResponse();
     }
 
     /**
      * This request handler can handle any frontend request.
      *
-     * @param \Psr\Http\Message\ServerRequestInterface $request
+     * @param ServerRequestInterface $request
      * @return bool If the request is not an eID request, TRUE otherwise FALSE
      */
-    public function canHandleRequest(\Psr\Http\Message\ServerRequestInterface $request)
+    public function canHandleRequest(ServerRequestInterface $request): bool
     {
         return $request->getQueryParams()['eID'] || $request->getParsedBody()['eID'] ? false : true;
     }
@@ -294,7 +299,7 @@ class RequestHandler implements RequestHandlerInterface
      *
      * @return int The priority of the request handler.
      */
-    public function getPriority()
+    public function getPriority(): int
     {
         return 50;
     }
index 8b6f31a..f503676 100644 (file)
@@ -114,7 +114,7 @@ class InstallerRequestHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request
      * @return bool Returns always TRUE
      */
-    public function canHandleRequest(ServerRequestInterface $request)
+    public function canHandleRequest(ServerRequestInterface $request): bool
     {
         $localConfigurationFileLocation = (new ConfigurationManager())->getLocalConfigurationFileLocation();
         return !@is_file($localConfigurationFileLocation) || EnableFileService::isFirstInstallAllowed();
index 5cffce3..f0db11a 100644 (file)
@@ -199,7 +199,7 @@ class RequestHandler implements RequestHandlerInterface
      * @param ServerRequestInterface $request
      * @return bool Returns always TRUE
      */
-    public function canHandleRequest(ServerRequestInterface $request)
+    public function canHandleRequest(ServerRequestInterface $request): bool
     {
         $basicIntegrity = $this->bootstrap->checkIfEssentialConfigurationExists()
             && !empty($GLOBALS['TYPO3_CONF_VARS']['BE']['installToolPassword'])
@@ -215,7 +215,7 @@ class RequestHandler implements RequestHandlerInterface
      *
      * @return int The priority of the request handler.
      */
-    public function getPriority()
+    public function getPriority(): int
     {
         return 50;
     }