From 8ada892a445ba2801268cab47947e5f90e073c2a Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Tue, 15 Jun 2021 15:26:32 +0200
Subject: [PATCH] [TASK] Deprecate extbase StopActionException
There are three possible cases an extbase controller
action can come up with:
a) Return a casual psr-7 response (html, json, ...)
b) Return an extbase specific ForwardResponse to dispatch
extbase-internally to another action.
c) Have a redirect the client should receive to
call some other Url.
a) and b) are simple in v11 - the action returns a
PSR-7 ResponseInterface. c) however throws StopActionException
instead, which is caught by extbase Dispatcher and
then returned. This is ugly.
The patch changes this and deprecates the StopActionException.
We can not drop the throw call since that would be breaking,
but we prepare towards a clean v12 solution, which leads to
the sitation that *all* extbase actions return a response.
Resolves: #94351
Releases: master
Change-Id: Ie5a53109959a008ab1666f52d5a81e6e7ba3efdb
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69498
Tested-by: core-ci <typo3@b13.com>
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
---
...on-94351-ExtextbaseStopActionException.rst | 60 +++++++++++++++++++
.../Mvc/Controller/ActionController.php | 26 +++-----
.../sysext/extbase/Classes/Mvc/Dispatcher.php | 8 +++
.../Mvc/Exception/StopActionException.php | 7 +++
.../UploadExtensionFileController.php | 5 +-
.../Domain/Finishers/RedirectFinisher.php | 43 ++++---------
.../Classes/ViewHelpers/RenderViewHelper.php | 7 +++
.../Domain/Finishers/RedirectFinisherTest.php | 4 +-
8 files changed, 110 insertions(+), 50 deletions(-)
create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Deprecation-94351-ExtextbaseStopActionException.rst
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-94351-ExtextbaseStopActionException.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-94351-ExtextbaseStopActionException.rst
new file mode 100644
index 000000000000..8f918249676e
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-94351-ExtextbaseStopActionException.rst
@@ -0,0 +1,60 @@
+.. include:: ../../Includes.txt
+
+=====================================================
+Deprecation: #94351 - ext:extbase StopActionException
+=====================================================
+
+See :issue:`94351`
+
+Description
+===========
+
+To further prepare towards clean PSR-7 Request / Response handling in
+extbase, the extbase internal :php:`TYPO3\CMS\Extbase\Mvc\Exception\StopActionException`
+has been deprecated.
+
+
+Impact
+======
+
+No deprecation is logged, but the :php:`StopActionException` will be
+removed in v12 as breaking change. Extension developers with extbase
+based controllers can prepare in v11 towards this.
+
+
+Affected Installations
+======================
+
+Extensions with extbase controllers that throw :php:`StopActionException` or
+use methods :php:`redirect` or :php:`redirectToUri` from extbase :php:`ActionController`
+are affected.
+
+
+Migration
+=========
+
+As a goal, extbase actions will *always* return a :php:`ResponseInterface`
+in v12. v11 prepares towards this, but still throws the :php:`StopActionException`
+in :php:`redirectToUri`. Developers should prepare towards this, however.
+
+Example before:
+
+.. code-block:: php
+
+ public function fooAction()
+ {
+ $this->redirect('otherAction');
+ }
+
+Example compatible with v10, v11 and v12 - IDE's and static code analyzers
+may complain in v10 and v11, though:
+
+.. code-block:: php
+
+ public function fooAction(): ResponseInterface
+ {
+ // A return is added!
+ return $this->redirect('otherAction');
+ }
+
+.. index:: PHP-API, NotScanned, ext:extbase
diff --git a/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php b/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
index 9a597cefd05a..46cfa6e8b75c 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
@@ -20,6 +20,7 @@ use Psr\Http\Message\ResponseFactoryInterface;
use Psr\Http\Message\ResponseInterface;
use TYPO3\CMS\Core\Http\HtmlResponse;
use TYPO3\CMS\Core\Http\PropagateResponseException;
+use TYPO3\CMS\Core\Http\RedirectResponse;
use TYPO3\CMS\Core\Http\Response;
use TYPO3\CMS\Core\Http\Stream;
use TYPO3\CMS\Core\Messaging\AbstractMessage;
@@ -946,9 +947,6 @@ abstract class ActionController implements ControllerInterface
*
* Redirect will be sent to the client which then performs another request to the new URI.
*
- * NOTE: This method only supports web requests and will thrown an exception
- * if used with other request types.
- *
* @param string $actionName Name of the action to forward to
* @param string|null $controllerName Unqualified object name of the controller to forward to. If not specified, the current controller is used.
* @param string|null $extensionName Name of the extension containing the controller to forward to. If not specified, the current extension is assumed.
@@ -956,9 +954,10 @@ abstract class ActionController implements ControllerInterface
* @param int|null $pageUid Target page uid. If NULL, the current page uid is used
* @param null $_ (optional) Unused
* @param int $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other
- * @throws StopActionException
+ * @throws StopActionException deprecated since TYPO3 11.0, method will RETURN a Core\Http\RedirectResponse instead of throwing in v12
+ * @todo: ': ResponseInterface' (without ?) in v12 as method return type together with redirectToUri() cleanup
*/
- protected function redirect($actionName, $controllerName = null, $extensionName = null, array $arguments = null, $pageUid = null, $_ = null, $statusCode = 303)
+ protected function redirect($actionName, $controllerName = null, $extensionName = null, array $arguments = null, $pageUid = null, $_ = null, $statusCode = 303): void
{
if ($controllerName === null) {
$controllerName = $this->request->getControllerName();
@@ -977,24 +976,17 @@ abstract class ActionController implements ControllerInterface
/**
* Redirects the web request to another uri.
*
- * NOTE: This method only supports web requests and will thrown an exception if used with other request types.
- *
* @param mixed $uri A string representation of a URI
* @param null $_ (optional) Unused
* @param int $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other"
- * @throws StopActionException
+ * @throws StopActionException deprecated since TYPO3 11.0, will be removed in 12.0
+ * @todo: ': ResponseInterface' (without ?) in v12 as method return type together with redirectToUri() cleanup
*/
- protected function redirectToUri($uri, $_ = null, $statusCode = 303)
+ protected function redirectToUri($uri, $_ = null, $statusCode = 303): void
{
$uri = $this->addBaseUriIfNecessary($uri);
- $response = new HtmlResponse(
- '',
- $statusCode,
- [
- 'Location' => (string)$uri
- ]
- );
-
+ $response = new RedirectResponse($uri, $statusCode);
+ // @deprecated since v11, will be removed in v12. RETURN the response instead. See Dispatcher class, too.
throw new StopActionException('redirectToUri', 1476045828, null, $response);
}
diff --git a/typo3/sysext/extbase/Classes/Mvc/Dispatcher.php b/typo3/sysext/extbase/Classes/Mvc/Dispatcher.php
index 094e2c2ab4e4..abc8c6467964 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Dispatcher.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Dispatcher.php
@@ -18,6 +18,7 @@ namespace TYPO3\CMS\Extbase\Mvc;
use Psr\Container\ContainerInterface;
use Psr\EventDispatcher\EventDispatcherInterface;
use Psr\Http\Message\ResponseInterface;
+use TYPO3\CMS\Core\Http\RedirectResponse;
use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Extbase\Annotation\IgnoreValidation;
use TYPO3\CMS\Extbase\Event\Mvc\AfterRequestDispatchedEvent;
@@ -90,8 +91,15 @@ class Dispatcher implements SingletonInterface
try {
$response = $controller->processRequest($request);
if ($response instanceof ForwardResponse) {
+ // The controller action returned an extbase internal Forward response:
+ // Another action should be dispatched.
$request = static::buildRequestFromCurrentRequestAndForwardResponse($request, $response);
}
+ if ($response instanceof RedirectResponse) {
+ // The controller action returned a core HTTP redirect response.
+ // Dispatching ends here and response is sent to client.
+ return $response;
+ }
} catch (StopActionException $ignoredException) {
$response = $ignoredException->getResponse();
}
diff --git a/typo3/sysext/extbase/Classes/Mvc/Exception/StopActionException.php b/typo3/sysext/extbase/Classes/Mvc/Exception/StopActionException.php
index abc7b75beccd..5d0ac115c3e2 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Exception/StopActionException.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Exception/StopActionException.php
@@ -28,6 +28,10 @@ use TYPO3\CMS\Extbase\Mvc\Exception;
* continues dispatching the request or returns control to the request handler.
*
* See the Action Controller's forward() and redirectToUri() methods for more information.
+ *
+ * @deprecated since v11, will be removed in v12. This action shouldn't be thrown anymore:
+ * Actions that extbase-internally forward to another action should RETURN Extbase\Http\ForwardResponse
+ * instead. Actions that initiate a client redirect should RETURN a Core\Http\RedirectResponse instead.
*/
class StopActionException extends Exception
{
@@ -38,6 +42,9 @@ class StopActionException extends Exception
public function __construct($message = '', $code = 0, \Throwable $previous = null, ResponseInterface $response = null)
{
+ // @deprecated since v11, will be removed in v12. Can not trigger_error() here since
+ // extbase ActionController still has to use this exception for b/w compatibility.
+ // See the usages of this exception when dropping in v12.
$this->response = $response ?? new Response();
parent::__construct($message, $code, $previous);
}
diff --git a/typo3/sysext/extensionmanager/Classes/Controller/UploadExtensionFileController.php b/typo3/sysext/extensionmanager/Classes/Controller/UploadExtensionFileController.php
index cc2edf5ee201..4371ab43b825 100644
--- a/typo3/sysext/extensionmanager/Classes/Controller/UploadExtensionFileController.php
+++ b/typo3/sysext/extensionmanager/Classes/Controller/UploadExtensionFileController.php
@@ -98,7 +98,7 @@ class UploadExtensionFileController extends AbstractController
* Extract an uploaded file and install the matching extension
*
* @param bool $overwrite Overwrite existing extension if TRUE
- * @throws \TYPO3\CMS\Extbase\Mvc\Exception\StopActionException
+ * @throws StopActionException @deprecated since v11, will be removed in v12
*/
public function extractAction($overwrite = false)
{
@@ -145,10 +145,12 @@ class UploadExtensionFileController extends AbstractController
FlashMessage::OK
);
} else {
+ // @deprecated since v11, change to return $this->redirect()
$this->redirect('unresolvedDependencies', 'List', null, ['extensionKey' => $extensionKey]);
}
}
} catch (StopActionException $exception) {
+ // @deprecated since v11, will be removed in v12: redirect() will no longer throw in v12, drop this catch block
throw $exception;
} catch (InvalidFileException $exception) {
$this->addFlashMessage($exception->getMessage(), '', FlashMessage::ERROR);
@@ -156,6 +158,7 @@ class UploadExtensionFileController extends AbstractController
$this->removeExtensionAndRestoreFromBackup($fileName);
$this->addFlashMessage($exception->getMessage(), '', FlashMessage::ERROR);
}
+ // @deprecated since v11, change to return $this->redirect()
$this->redirect('index', 'List', null, [
self::TRIGGER_RefreshModuleMenu => true,
self::TRIGGER_RefreshTopbar => true
diff --git a/typo3/sysext/form/Classes/Domain/Finishers/RedirectFinisher.php b/typo3/sysext/form/Classes/Domain/Finishers/RedirectFinisher.php
index 22eaf663c985..f1c3b9186bac 100644
--- a/typo3/sysext/form/Classes/Domain/Finishers/RedirectFinisher.php
+++ b/typo3/sysext/form/Classes/Domain/Finishers/RedirectFinisher.php
@@ -17,10 +17,9 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Domain\Finishers;
-use Psr\Http\Message\ResponseInterface;
-use TYPO3\CMS\Core\Http\Stream;
+use TYPO3\CMS\Core\Http\PropagateResponseException;
+use TYPO3\CMS\Core\Http\RedirectResponse;
use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Extbase\Mvc\Exception\StopActionException;
use TYPO3\CMS\Extbase\Mvc\Web\Routing\UriBuilder;
/**
@@ -37,7 +36,6 @@ class RedirectFinisher extends AbstractFinisher
protected $defaultOptions = [
'pageUid' => 1,
'additionalParameters' => '',
- 'delay' => 0,
'statusCode' => 303,
];
@@ -46,11 +44,6 @@ class RedirectFinisher extends AbstractFinisher
*/
protected $request;
- /**
- * @var ResponseInterface
- */
- protected $response;
-
/**
* @var \TYPO3\CMS\Extbase\Mvc\Web\Routing\UriBuilder
*/
@@ -64,7 +57,6 @@ class RedirectFinisher extends AbstractFinisher
{
$formRuntime = $this->finisherContext->getFormRuntime();
$this->request = $formRuntime->getRequest();
- $this->response = $formRuntime->getResponse();
$this->uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
$this->uriBuilder->setRequest($this->request);
@@ -73,11 +65,10 @@ class RedirectFinisher extends AbstractFinisher
$additionalParameters = $this->parseOption('additionalParameters');
$additionalParameters = is_string($additionalParameters) ? $additionalParameters : '';
$additionalParameters = '&' . ltrim($additionalParameters, '&');
- $delay = (int)$this->parseOption('delay');
$statusCode = (int)$this->parseOption('statusCode');
$this->finisherContext->cancel();
- $this->redirect($pageUid, $additionalParameters, $delay, $statusCode);
+ $this->redirect($pageUid, $additionalParameters, $statusCode);
}
/**
@@ -90,18 +81,17 @@ class RedirectFinisher extends AbstractFinisher
*
* @param int $pageUid Target page uid. If NULL, the current page uid is used
* @param string $additionalParameters
- * @param int $delay (optional) The delay in seconds. Default is no delay.
* @param int $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other
* @see forward()
*/
- protected function redirect(int $pageUid = 1, string $additionalParameters = '', int $delay = 0, int $statusCode = 303)
+ protected function redirect(int $pageUid = 1, string $additionalParameters = '', int $statusCode = 303)
{
$typolinkConfiguration = [
'parameter' => $pageUid,
'additionalParams' => $additionalParameters,
];
$redirectUri = $this->getTypoScriptFrontendController()->cObj->typoLink_URL($typolinkConfiguration);
- $this->redirectToUri($redirectUri, $delay, $statusCode);
+ $this->redirectToUri($redirectUri, $statusCode);
}
/**
@@ -110,25 +100,18 @@ class RedirectFinisher extends AbstractFinisher
* NOTE: This method only supports web requests and will thrown an exception if used with other request types.
*
* @param string $uri A string representation of a URI
- * @param int $delay (optional) The delay in seconds. Default is no delay.
* @param int $statusCode (optional) The HTTP status code for the redirect. Default is "303 See Other
- * @throws StopActionException
+ * @throws PropagateResponseException
*/
- protected function redirectToUri(string $uri, int $delay = 0, int $statusCode = 303)
+ protected function redirectToUri(string $uri, int $statusCode = 303)
{
$uri = $this->addBaseUriIfNecessary($uri);
- $escapedUri = htmlentities($uri, ENT_QUOTES, 'utf-8');
-
- $body = new Stream('php://temp', 'r+');
- $body->write('<html><head><meta http-equiv="refresh" content="' . (int)$delay . ';url=' . $escapedUri . '"/></head></html>');
- $body->rewind();
-
- $this->response = $this->response
- ->withBody($body)
- ->withStatus($statusCode)
- ->withHeader('Location', (string)$uri)
- ;
- throw new StopActionException('redirectToUri', 1477070964, null, $this->response);
+ $response = new RedirectResponse($uri, $statusCode);
+ // End processing and dispatching by throwing a PropagateResponseException with our response.
+ // @todo: Should be changed to *return* a response instead, but this requires the ContentObjectRender
+ // @todo: to deal with responses instead of strings, if the form is used in a fluid template rendered by the
+ // @todo: FluidTemplateContentObject and the extbase bootstrap isn't used.
+ throw new PropagateResponseException($response, 1477070964);
}
/**
diff --git a/typo3/sysext/form/Classes/ViewHelpers/RenderViewHelper.php b/typo3/sysext/form/Classes/ViewHelpers/RenderViewHelper.php
index 5e91edf626f7..92dfded347ad 100644
--- a/typo3/sysext/form/Classes/ViewHelpers/RenderViewHelper.php
+++ b/typo3/sysext/form/Classes/ViewHelpers/RenderViewHelper.php
@@ -111,6 +111,13 @@ class RenderViewHelper extends AbstractViewHelper
try {
return $form->render();
} catch (StopActionException $exception) {
+ // @deprecated since v11, will be removed in v12: StopActionException is deprecated, drop this catch block.
+ // RedirectFinisher for throws a PropagateResponseException instead which bubbles up into Middleware.
+ trigger_error(
+ 'Throwing StopActionException is deprecated. If really needed, throw a (internal) PropagateResponseException'
+ . ' instead, for now. Note this is subject to change.',
+ E_USER_DEPRECATED
+ );
return $exception->getResponse()->getBody()->getContents();
}
}
diff --git a/typo3/sysext/form/Tests/Unit/Domain/Finishers/RedirectFinisherTest.php b/typo3/sysext/form/Tests/Unit/Domain/Finishers/RedirectFinisherTest.php
index a9f71d926a0a..f7358a4efde1 100644
--- a/typo3/sysext/form/Tests/Unit/Domain/Finishers/RedirectFinisherTest.php
+++ b/typo3/sysext/form/Tests/Unit/Domain/Finishers/RedirectFinisherTest.php
@@ -18,9 +18,9 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Tests\Unit\Domain\Finishers;
use Prophecy\Argument;
+use TYPO3\CMS\Core\Http\PropagateResponseException;
use TYPO3\CMS\Core\Http\Response;
use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Extbase\Mvc\Exception\StopActionException;
use TYPO3\CMS\Extbase\Mvc\Request;
use TYPO3\CMS\Extbase\Mvc\Web\Routing\UriBuilder;
use TYPO3\CMS\Extbase\Object\Exception;
@@ -84,7 +84,7 @@ class RedirectFinisherTest extends UnitTestCase
try {
$redirectFinisherMock->execute($finisherContextProphecy->reveal());
self::fail('RedirectFinisher did not throw expected exception.');
- } /** @noinspection PhpRedundantCatchClauseInspection */ catch (StopActionException $e) {
+ } /** @noinspection PhpRedundantCatchClauseInspection */ catch (PropagateResponseException $e) {
$response = $e->getResponse();
self::assertSame($uriPrefix . $expectedPage, $response->getHeader('Location')[0]);
}
--
GitLab