[TASK] Remove dependencies of \TYPO3\CMS\Extbase\Mvc\Web\Request 35/61235/4
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Sat, 6 Jul 2019 10:59:52 +0000 (12:59 +0200)
committerBenjamin Franzke <bfr@qbus.de>
Mon, 8 Jul 2019 20:04:11 +0000 (22:04 +0200)
According to PSR-7 request objects should be immutable.
That can only be achieved if the state of the request
object is defined during construction. Therefore, the
web request object of Extbase mustn't have any service
dependencies that alter the state of the object during
runtime.

Although there are still setters that can alter the
state of request objects, the more crucial issue with
the object state were the injected services.

Releases: master
Resolves: #88697
Change-Id: Iedf6fc3d33a1e5b89bce20019cbfc484b1466cf8
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61235
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Jörg Bösche <typo3@joergboesche.de>
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
typo3/sysext/extbase/Classes/Mvc/Web/Request.php
typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php
typo3/sysext/extbase/Tests/Unit/Mvc/Web/RequestBuilderTest.php
typo3/sysext/fluid/Classes/Core/Widget/WidgetRequestBuilder.php
typo3/sysext/fluid/Classes/View/StandaloneView.php
typo3/sysext/fluid/Tests/Unit/Core/Widget/WidgetRequestBuilderTest.php

index 9881373..6c3efec 100644 (file)
@@ -19,7 +19,9 @@ use TYPO3\CMS\Core\Page\PageRenderer;
 use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
 use TYPO3\CMS\Extbase\Mvc\Exception\StopActionException;
 use TYPO3\CMS\Extbase\Mvc\View\ViewInterface;
+use TYPO3\CMS\Extbase\Mvc\Web\ReferringRequest;
 use TYPO3\CMS\Extbase\Mvc\Web\Request as WebRequest;
+use TYPO3\CMS\Extbase\Security\Cryptography\HashService;
 use TYPO3\CMS\Extbase\Validation\Validator\ConjunctionValidator;
 use TYPO3\CMS\Extbase\Validation\Validator\ValidatorInterface;
 use TYPO3Fluid\Fluid\View\TemplateView;
@@ -40,6 +42,11 @@ class ActionController extends AbstractController
     protected $cacheService;
 
     /**
+     * @var HashService;
+     */
+    protected $hashService;
+
+    /**
      * The current view, as resolved by resolveView()
      *
      * @var ViewInterface
@@ -104,6 +111,14 @@ class ActionController extends AbstractController
     }
 
     /**
+     * @param HashService $hashService
+     */
+    public function injectHashService(HashService $hashService)
+    {
+        $this->hashService = $hashService;
+    }
+
+    /**
      * @param \TYPO3\CMS\Extbase\Mvc\Controller\MvcPropertyMappingConfigurationService $mvcPropertyMappingConfigurationService
      */
     public function injectMvcPropertyMappingConfigurationService(\TYPO3\CMS\Extbase\Mvc\Controller\MvcPropertyMappingConfigurationService $mvcPropertyMappingConfigurationService)
@@ -519,7 +534,18 @@ class ActionController extends AbstractController
      */
     protected function forwardToReferringRequest()
     {
-        $referringRequest = $this->request->getReferringRequest();
+        $referringRequest = null;
+        $referringRequestArguments = $this->request->getInternalArguments()['__referrer']['@request'] ?? null;
+        if (is_string($referringRequestArguments)) {
+            $referrerArray = json_decode(
+                $this->hashService->validateAndStripHmac($referringRequestArguments),
+                true
+            );
+            $arguments = [];
+            $referringRequest = new ReferringRequest();
+            $referringRequest->setArguments(array_replace_recursive($arguments, $referrerArray));
+        }
+
         if ($referringRequest !== null) {
             $originalRequest = clone $this->request;
             $this->request->setOriginalRequest($originalRequest);
index a8fafa3..0016ab7 100644 (file)
@@ -20,11 +20,6 @@ namespace TYPO3\CMS\Extbase\Mvc\Web;
 class Request extends \TYPO3\CMS\Extbase\Mvc\Request
 {
     /**
-     * @var \TYPO3\CMS\Extbase\Security\Cryptography\HashService
-     */
-    protected $hashService;
-
-    /**
      * @var string The requested representation format
      */
     protected $format = 'html';
@@ -50,43 +45,6 @@ class Request extends \TYPO3\CMS\Extbase\Mvc\Request
     protected $isCached = false;
 
     /**
-     * @var \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface
-     */
-    protected $configurationManager;
-
-    /**
-     * @var \TYPO3\CMS\Extbase\Service\EnvironmentService
-     */
-    protected $environmentService;
-
-    /**
-     * @param \TYPO3\CMS\Extbase\Security\Cryptography\HashService $hashService
-     * @internal only to be used within Extbase, not part of TYPO3 Core API.
-     */
-    public function injectHashService(\TYPO3\CMS\Extbase\Security\Cryptography\HashService $hashService)
-    {
-        $this->hashService = $hashService;
-    }
-
-    /**
-     * @param \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface $configurationManager
-     * @internal only to be used within Extbase, not part of TYPO3 Core API.
-     */
-    public function injectConfigurationManager(\TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface $configurationManager)
-    {
-        $this->configurationManager = $configurationManager;
-    }
-
-    /**
-     * @param \TYPO3\CMS\Extbase\Service\EnvironmentService $environmentService
-     * @internal only to be used within Extbase, not part of TYPO3 Core API.
-     */
-    public function injectEnvironmentService(\TYPO3\CMS\Extbase\Service\EnvironmentService $environmentService)
-    {
-        $this->environmentService = $environmentService;
-    }
-
-    /**
      * Sets the request method
      *
      * @param string $method Name of the request method
@@ -150,9 +108,6 @@ class Request extends \TYPO3\CMS\Extbase\Mvc\Request
      */
     public function getBaseUri()
     {
-        if ($this->environmentService->isEnvironmentInBackendMode()) {
-            return $this->baseUri . TYPO3_mainDir;
-        }
         return $this->baseUri;
     }
 
@@ -177,29 +132,4 @@ class Request extends \TYPO3\CMS\Extbase\Mvc\Request
     {
         return $this->isCached;
     }
-
-    /**
-     * Get a freshly built request object pointing to the Referrer.
-     *
-     * @return ReferringRequest the referring request, or null if no referrer found
-     * @internal only to be used within Extbase, not part of TYPO3 Core API.
-     */
-    public function getReferringRequest()
-    {
-        if (isset($this->internalArguments['__referrer']['@request'])) {
-            $referrerArray = json_decode($this->hashService->validateAndStripHmac($this->internalArguments['__referrer']['@request']), true);
-            $arguments = [];
-            // todo: Creating a referring request object here with a new statement is strange.
-            // todo: As request objects have inject methods and are still meant to be created via object manager,
-            // todo: this creates a partly non functional object. This is ok here as only the arguments matter, but
-            // todo: it should be solved better.
-            // todo: As an alternative, all request objects could be created via RequestBuilder and if there is a
-            // todo: redirect in a controller which is the cause of need for a referring object, said object should be
-            // todo: created before dispatching the new request and not in a getter.
-            $referringRequest = new ReferringRequest();
-            $referringRequest->setArguments(array_replace_recursive($arguments, $referrerArray));
-            return $referringRequest;
-        }
-        return null;
-    }
 }
index f7d1180..5611fe4 100644 (file)
@@ -200,6 +200,12 @@ class RequestBuilder implements \TYPO3\CMS\Core\SingletonInterface
 
         $controllerClassName = $this->resolveControllerClassName($parameters);
         $actionName = $this->resolveActionName($controllerClassName, $parameters);
+
+        $baseUri = \TYPO3\CMS\Core\Utility\GeneralUtility::getIndpEnv('TYPO3_SITE_URL');
+        if ($this->environmentService->isEnvironmentInBackendMode()) {
+            $baseUri .= TYPO3_mainDir;
+        }
+
         /** @var \TYPO3\CMS\Extbase\Mvc\Web\Request $request */
         $request = $this->objectManager->get(\TYPO3\CMS\Extbase\Mvc\Web\Request::class);
         $request->setPluginName($this->pluginName);
@@ -209,7 +215,7 @@ class RequestBuilder implements \TYPO3\CMS\Core\SingletonInterface
         $request->setControllerActionName($actionName);
         // @todo Use Environment
         $request->setRequestUri(\TYPO3\CMS\Core\Utility\GeneralUtility::getIndpEnv('TYPO3_REQUEST_URL'));
-        $request->setBaseUri(\TYPO3\CMS\Core\Utility\GeneralUtility::getIndpEnv('TYPO3_SITE_URL'));
+        $request->setBaseUri($baseUri);
         $request->setMethod($this->environmentService->getServerRequestMethod());
         if (isset($parameters['format']) && is_string($parameters['format']) && $parameters['format'] !== '') {
             $request->setFormat(filter_var($parameters['format'], FILTER_SANITIZE_STRING));
index aaa855b..fb95997 100644 (file)
@@ -103,8 +103,10 @@ class RequestBuilderTest extends UnitTestCase
         $this->mockObjectManager = $this->createMock(ObjectManagerInterface::class);
         $this->mockExtensionService = $this->createMock(ExtensionService::class);
         $this->mockEnvironmentService = $this->getMockBuilder(EnvironmentService::class)
-            ->setMethods(['getServerRequestMethod'])
+            ->setMethods(['getServerRequestMethod', 'isEnvironmentInFrontendMode', 'isEnvironmentInBackendMode'])
             ->getMock();
+        $this->mockEnvironmentService->expects($this->any())->method('isEnvironmentInFrontendMode')->willReturn(true);
+        $this->mockEnvironmentService->expects($this->any())->method('isEnvironmentInBackendMode')->willReturn(false);
     }
 
     /**
index 786be3b..134aadd 100644 (file)
@@ -44,9 +44,14 @@ class WidgetRequestBuilder extends RequestBuilder
      */
     public function build(): RequestInterface
     {
+        $baseUri = \TYPO3\CMS\Core\Utility\GeneralUtility::getIndpEnv('TYPO3_SITE_URL');
+        if ($this->environmentService->isEnvironmentInBackendMode()) {
+            $baseUri .= TYPO3_mainDir;
+        }
+
         $request = $this->objectManager->get(WidgetRequest::class);
         $request->setRequestUri(GeneralUtility::getIndpEnv('TYPO3_REQUEST_URL'));
-        $request->setBaseUri(GeneralUtility::getIndpEnv('TYPO3_SITE_URL'));
+        $request->setBaseUri($baseUri);
         $request->setMethod($_SERVER['REQUEST_METHOD'] ?? null);
         if (strtolower($_SERVER['REQUEST_METHOD']) === 'post') {
             $request->setArguments(GeneralUtility::_POST());
index 8eeac0c..2c61b90 100644 (file)
@@ -20,6 +20,7 @@ use TYPO3\CMS\Extbase\Mvc\Controller\ControllerContext;
 use TYPO3\CMS\Extbase\Mvc\Web\Request as WebRequest;
 use TYPO3\CMS\Extbase\Mvc\Web\Routing\UriBuilder;
 use TYPO3\CMS\Extbase\Object\ObjectManager;
+use TYPO3\CMS\Extbase\Service\EnvironmentService;
 use TYPO3\CMS\Fluid\Core\Rendering\RenderingContext;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 use TYPO3Fluid\Fluid\View\Exception\InvalidTemplateResourceException;
@@ -54,10 +55,15 @@ class StandaloneView extends AbstractTemplateView
         }
         $configurationManager->setContentObject($contentObject);
 
+        $baseUri = \TYPO3\CMS\Core\Utility\GeneralUtility::getIndpEnv('TYPO3_SITE_URL');
+        if ($this->objectManager->get(EnvironmentService::class)->isEnvironmentInBackendMode()) {
+            $baseUri .= TYPO3_mainDir;
+        }
+
         /** @var WebRequest $request */
         $request = $this->objectManager->get(WebRequest::class);
         $request->setRequestUri(GeneralUtility::getIndpEnv('TYPO3_REQUEST_URL'));
-        $request->setBaseUri(GeneralUtility::getIndpEnv('TYPO3_SITE_URL'));
+        $request->setBaseUri($baseUri);
         /** @var UriBuilder $uriBuilder */
         $uriBuilder = $this->objectManager->get(UriBuilder::class);
         $uriBuilder->setRequest($request);
index ed31e16..758d687 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Fluid\Tests\Unit\Core\Widget;
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Object\ObjectManagerInterface;
+use TYPO3\CMS\Extbase\Service\EnvironmentService;
 use TYPO3\CMS\Fluid\Core\Widget\AjaxWidgetContextHolder;
 use TYPO3\CMS\Fluid\Core\Widget\WidgetContext;
 use TYPO3\CMS\Fluid\Core\Widget\WidgetRequest;
@@ -66,6 +67,10 @@ class WidgetRequestBuilderTest extends UnitTestCase
         $this->mockWidgetContext = $this->createMock(WidgetContext::class);
         $this->mockAjaxWidgetContextHolder = $this->createMock(AjaxWidgetContextHolder::class);
         $this->widgetRequestBuilder->injectAjaxWidgetContextHolder($this->mockAjaxWidgetContextHolder);
+        $environmentServiceMock = $this->createMock(EnvironmentService::class);
+        $environmentServiceMock->expects($this->any())->method('isEnvironmentInFrontendMode')->willReturn(true);
+        $environmentServiceMock->expects($this->any())->method('isEnvironmentInBackendMode')->willReturn(false);
+        $this->widgetRequestBuilder->injectEnvironmentService($environmentServiceMock);
     }
 
     /**