Skip to content

Commit 0b7f4f5

Browse files
lolli42lolli42
committed
[TASK] Harden extbase RequestBuilder
Use DI, clean up, declare clear build() returns an extbase RequestInterface. Resolves: #105679 Releases: main Change-Id: I207384a4cdd7a9d7a4433179a22cc4007b31bb0c Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87165 Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Andreas Nedbal <andy@pixelde.su> Tested-by: Simon Praetorius <simon@praetorius.me> Reviewed-by: Simon Praetorius <simon@praetorius.me> Tested-by: Andreas Nedbal <andy@pixelde.su>
1 parent d218467 commit 0b7f4f5

File tree

3 files changed

+30
-57
lines changed

3 files changed

+30
-57
lines changed

‎Build/phpstan/phpstan-baseline.neon‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3204,12 +3204,6 @@ parameters:
32043204
count: 1
32053205
path: ../../typo3/sysext/extbase/Tests/Functional/Mvc/Validation/ActionControllerValidationTest.php
32063206

3207-
-
3208-
message: '#^Call to static method PHPUnit\\Framework\\Assert\:\:assertInstanceOf\(\) with ''TYPO3\\\\CMS\\\\Extbase\\\\Mvc\\\\RequestInterface'' and TYPO3\\CMS\\Extbase\\Mvc\\Request will always evaluate to true\.$#'
3209-
identifier: staticMethod.alreadyNarrowedType
3210-
count: 14
3211-
path: ../../typo3/sysext/extbase/Tests/Functional/Mvc/Web/RequestBuilderTest.php
3212-
32133207
-
32143208
message: '#^Call to static method PHPUnit\\Framework\\Assert\:\:assertInstanceOf\(\) with ''TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Generic\\\\Mapper\\\\DataMap'' and TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Mapper\\DataMap will always evaluate to true\.$#'
32153209
identifier: staticMethod.alreadyNarrowedType

‎typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php‎

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -18,59 +18,38 @@
1818
namespace TYPO3\CMS\Extbase\Mvc\Web;
1919

2020
use Psr\Http\Message\ServerRequestInterface;
21+
use Symfony\Component\DependencyInjection\Attribute\Autoconfigure;
2122
use TYPO3\CMS\Backend\Module\ExtbaseModule;
2223
use TYPO3\CMS\Core\Error\Http\PageNotFoundException;
2324
use TYPO3\CMS\Core\Http\UploadedFile;
2425
use TYPO3\CMS\Core\Routing\PageArguments;
25-
use TYPO3\CMS\Core\SingletonInterface;
2626
use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
2727
use TYPO3\CMS\Extbase\Mvc\Exception as MvcException;
2828
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidActionNameException;
2929
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidArgumentNameException;
3030
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidControllerNameException;
3131
use TYPO3\CMS\Extbase\Mvc\ExtbaseRequestParameters;
3232
use TYPO3\CMS\Extbase\Mvc\Request;
33+
use TYPO3\CMS\Extbase\Mvc\RequestInterface;
3334
use TYPO3\CMS\Extbase\Service\ExtensionService;
3435

3536
/**
36-
* Builds a web request.
37+
* Builds an extbase web request.
3738
*
3839
* @internal only to be used within Extbase, not part of TYPO3 Core API.
3940
*/
40-
class RequestBuilder implements SingletonInterface
41+
#[Autoconfigure(public: true)]
42+
readonly class RequestBuilder
4143
{
42-
protected ConfigurationManagerInterface $configurationManager;
43-
protected ExtensionService $extensionService;
44-
45-
public function __construct(ConfigurationManagerInterface $configurationManager, ExtensionService $extensionService)
46-
{
47-
$this->configurationManager = $configurationManager;
48-
$this->extensionService = $extensionService;
49-
}
44+
public function __construct(
45+
protected ConfigurationManagerInterface $configurationManager,
46+
protected ExtensionService $extensionService,
47+
) {}
5048

5149
/**
52-
* @throws MvcException
53-
* @see \TYPO3\CMS\Extbase\Core\Bootstrap::initializeConfiguration
50+
* Decorate a PSR-7 request as extbase web Request with the extbase attribute.
5451
*/
55-
protected function loadDefaultValues(array $configuration = []): RequestBuilderDefaultValues
56-
{
57-
// todo: See comment in \TYPO3\CMS\Extbase\Core\Bootstrap::initializeConfiguration for further explanation
58-
// todo: on why we shouldn't use the configuration manager here.
59-
$configuration = array_replace_recursive($this->configurationManager->getConfiguration(ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK), $configuration);
60-
61-
try {
62-
return RequestBuilderDefaultValues::fromConfiguration($configuration);
63-
} catch (\InvalidArgumentException $e) {
64-
throw MvcException::fromPrevious($e);
65-
}
66-
}
67-
68-
/**
69-
* Builds a web request object from the raw HTTP information and the configuration
70-
*
71-
* @return Request The web request as an object
72-
*/
73-
public function build(ServerRequestInterface $mainRequest)
52+
public function build(ServerRequestInterface $mainRequest): RequestInterface
7453
{
7554
$configuration = [];
7655
// Parameters, which are not part of the request URL (e.g. due to "useArgumentsWithoutNamespace"), which however
@@ -156,14 +135,29 @@ public function build(ServerRequestInterface $mainRequest)
156135
return new Request($mainRequest->withAttribute('extbase', $extbaseAttribute));
157136
}
158137

138+
/**
139+
* @throws MvcException
140+
*/
141+
protected function loadDefaultValues(array $configuration = []): RequestBuilderDefaultValues
142+
{
143+
// todo: See comment in \TYPO3\CMS\Extbase\Core\Bootstrap::initializeConfiguration for further explanation
144+
// todo: on why we shouldn't use the configuration manager here.
145+
$configuration = array_replace_recursive($this->configurationManager->getConfiguration(ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK), $configuration);
146+
try {
147+
return RequestBuilderDefaultValues::fromConfiguration($configuration);
148+
} catch (\InvalidArgumentException $e) {
149+
throw MvcException::fromPrevious($e);
150+
}
151+
}
152+
159153
/**
160154
* Returns the current ControllerName extracted from given $parameters.
161155
* If no controller is specified, the defaultControllerName will be returned.
162156
* If that's not available, an exception is thrown.
163157
*
164-
* @throws \TYPO3\CMS\Extbase\Mvc\Exception\InvalidControllerNameException
158+
* @throws InvalidControllerNameException
165159
* @throws MvcException if the controller could not be resolved
166-
* @throws \TYPO3\CMS\Core\Error\Http\PageNotFoundException
160+
* @throws PageNotFoundException
167161
* @return class-string
168162
*/
169163
protected function resolveControllerClassName(RequestBuilderDefaultValues $defaultValues, array $parameters): string
@@ -194,9 +188,9 @@ protected function resolveControllerClassName(RequestBuilderDefaultValues $defau
194188
* If that's not available or the specified action is not defined in the current plugin, an exception is thrown.
195189
*
196190
* @param class-string $controllerClassName
197-
* @throws \TYPO3\CMS\Extbase\Mvc\Exception\InvalidActionNameException
191+
* @throws InvalidActionNameException
198192
* @throws MvcException
199-
* @throws \TYPO3\CMS\Core\Error\Http\PageNotFoundException
193+
* @throws PageNotFoundException
200194
* @return non-empty-string
201195
*/
202196
protected function resolveActionName(RequestBuilderDefaultValues $defaultValues, string $controllerClassName, array $parameters): string

‎typo3/sysext/extbase/Tests/Functional/Mvc/Web/RequestBuilderTest.php‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
use TYPO3\CMS\Extbase\Mvc\Exception;
3535
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidActionNameException;
3636
use TYPO3\CMS\Extbase\Mvc\Exception\InvalidControllerNameException;
37-
use TYPO3\CMS\Extbase\Mvc\RequestInterface;
3837
use TYPO3\CMS\Extbase\Mvc\Web\RequestBuilder;
3938
use TYPO3\TestingFramework\Core\Functional\Framework\FrameworkState;
4039
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
@@ -96,7 +95,6 @@ public function buildBuildsARequestInterfaceObject(): void
9695
$mainRequest = $mainRequest->withAttribute('module', $module);
9796
$request = $this->get(RequestBuilder::class)->build($mainRequest);
9897

99-
self::assertInstanceOf(RequestInterface::class, $request);
10098
self::assertSame('html', $request->getFormat());
10199
}
102100

@@ -127,7 +125,6 @@ public function loadDefaultValuesOverridesFormatIfConfigured(): void
127125
$mainRequest = $mainRequest->withAttribute('module', $module);
128126
$request = $this->get(RequestBuilder::class)->build($mainRequest);
129127

130-
self::assertInstanceOf(RequestInterface::class, $request);
131128
self::assertSame('json', $request->getFormat());
132129
}
133130

@@ -158,7 +155,6 @@ public function buildOverridesFormatIfSetInGetParameters(): void
158155
$mainRequest = $mainRequest->withAttribute('module', $module);
159156
$request = $this->get(RequestBuilder::class)->build($mainRequest);
160157

161-
self::assertInstanceOf(RequestInterface::class, $request);
162158
self::assertSame('json', $request->getFormat());
163159
}
164160

@@ -234,7 +230,6 @@ public function uploadedFileCanBeRetrievedFromRequest(): void
234230
$mainRequest = $mainRequest->withAttribute('normalizedParams', $normalizedParams)->withAttribute('module', $module);
235231
$request = $this->get(RequestBuilder::class)->build($mainRequest);
236232

237-
self::assertInstanceOf(RequestInterface::class, $request);
238233
$uploadedFiles = $request->getUploadedFiles();
239234
$uploadedFile = $uploadedFiles['dummy'];
240235
self::assertInstanceOf(UploadedFile::class, $uploadedFile);
@@ -304,7 +299,6 @@ public function multipleUploadedFileCanBeRetrievedFromRequest(): void
304299
$mainRequest = $mainRequest->withAttribute('normalizedParams', $normalizedParams)->withAttribute('module', $module);
305300
$request = $this->get(RequestBuilder::class)->build($mainRequest);
306301

307-
self::assertInstanceOf(RequestInterface::class, $request);
308302
$uploadedFiles = $request->getUploadedFiles();
309303
$uploadedFile1 = $uploadedFiles['dummy']['pdf'];
310304
self::assertInstanceOf(UploadedFile::class, $uploadedFile1);
@@ -441,7 +435,6 @@ public function resolveControllerClassNameReturnsDefaultControllerIfCallDefaultA
441435
$mainRequest = $mainRequest->withQueryParams(['controller' => 'NonExistentController']);
442436
$request = $this->get(RequestBuilder::class)->build($mainRequest);
443437

444-
self::assertInstanceOf(RequestInterface::class, $request);
445438
self::assertSame('Blog', $request->getControllerName());
446439
}
447440

@@ -473,7 +466,6 @@ public function resolveControllerClassNameReturnsControllerDefinedViaParametersI
473466
$mainRequest = $mainRequest->withQueryParams(['controller' => 'User']);
474467
$request = $this->get(RequestBuilder::class)->build($mainRequest);
475468

476-
self::assertInstanceOf(RequestInterface::class, $request);
477469
self::assertSame('User', $request->getControllerName());
478470
}
479471

@@ -572,7 +564,6 @@ public function resolveActionNameReturnsDefaultActionIfCallDefaultActionIfAction
572564
$mainRequest = $mainRequest->withQueryParams(['tx_blog_example_blog' => ['action' => 'NonExistentAction']]);
573565
$request = $this->get(RequestBuilder::class)->build($mainRequest);
574566

575-
self::assertInstanceOf(RequestInterface::class, $request);
576567
self::assertSame('list', $request->getControllerActionName());
577568
}
578569

@@ -603,7 +594,6 @@ public function resolveActionNameReturnsActionDefinedViaParametersIfActionIsConf
603594
$mainRequest = $mainRequest->withQueryParams(['action' => 'show']);
604595
$request = $this->get(RequestBuilder::class)->build($mainRequest);
605596

606-
self::assertInstanceOf(RequestInterface::class, $request);
607597
self::assertSame('show', $request->getControllerActionName());
608598
}
609599

@@ -669,7 +659,6 @@ public function resolveActionNameReturnsActionDefinedViaParametersOfServerReques
669659

670660
$request = $this->get(RequestBuilder::class)->build($mainRequest);
671661

672-
self::assertInstanceOf(RequestInterface::class, $request);
673662
self::assertSame('show', $request->getControllerActionName());
674663
}
675664

@@ -707,7 +696,6 @@ public function resolveActionNameReturnsActionDefinedViaPageArgumentOfServerRequ
707696

708697
$request = $this->get(RequestBuilder::class)->build($mainRequest);
709698

710-
self::assertInstanceOf(RequestInterface::class, $request);
711699
self::assertSame('show', $request->getControllerActionName());
712700
}
713701

@@ -741,7 +729,6 @@ public function resolveActionNameReturnsActionDefinedViaParsedBodyOfServerReques
741729

742730
$request = $this->get(RequestBuilder::class)->build($mainRequest);
743731

744-
self::assertInstanceOf(RequestInterface::class, $request);
745732
self::assertSame('show', $request->getControllerActionName());
746733
}
747734

@@ -777,7 +764,6 @@ public function silentlyIgnoreInvalidParameterAndUseDefaultAction(): void
777764

778765
$request = $this->get(RequestBuilder::class)->build($mainRequest);
779766

780-
self::assertInstanceOf(RequestInterface::class, $request);
781767
self::assertSame('list', $request->getControllerActionName());
782768
}
783769

@@ -818,7 +804,6 @@ public function controllerActionParametersAreAddedToRequest(): void
818804

819805
$request = $this->get(RequestBuilder::class)->build($mainRequest);
820806

821-
self::assertInstanceOf(RequestInterface::class, $request);
822807
self::assertSame('show', $request->getControllerActionName());
823808
self::assertSame('Blog', $request->getArgument('controller'));
824809
self::assertSame('show', $request->getArgument('action'));

0 commit comments

Comments
 (0)