From 794ac4a3fddae0a4df63571cee15588c5c06c8d6 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Wed, 27 Mar 2024 10:50:28 +0100 Subject: [PATCH] TASK: Fix constraints for `#[Flow\Route]` annotation --- Neos.Flow/Classes/Annotations/Route.php | 34 ++++++++++++++----- .../Routing/AttributeRoutesProviderTest.php | 9 +++-- .../Mvc/Routing/Dto/RouteAnnotationTest.php | 29 ++++++++-------- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/Neos.Flow/Classes/Annotations/Route.php b/Neos.Flow/Classes/Annotations/Route.php index 24d182c17b..5875699867 100644 --- a/Neos.Flow/Classes/Annotations/Route.php +++ b/Neos.Flow/Classes/Annotations/Route.php @@ -30,22 +30,38 @@ final class Route { /** - * @param string $uriPattern The uri-pattern for the route without leading '/'. Must not contain `{@action}` or `{@controller}`. - * @param string|null $name (default null) The name ouf the route as it shows up in the route:list command - * @param array $httpMethods (default []) List of http verbs like 'GET', 'POST', 'PUT', 'DELETE', if not specified 'any' is used - * @param array $defaults (default []) Values to set for this route. Dan define arguments but also specify the `@format` if required. + * Magic route values cannot be set as default nor be contained as segments like `{\@action}` or `{\@controller}` in the uriPattern. + * The magic route value `\@format` is allowed if necessary. + */ + private const PRESERVED_DEFAULTS = ['@package', '@subpackage', '@controller', '@action']; + + /** + * @param string $uriPattern The uri-pattern for the route without leading '/'. + * @param string $name (default null) The name ouf the route as it shows up in the route:list command + * @param array $httpMethods (default []) List of http verbs like 'GET', 'POST', 'PUT', 'DELETE', if not specified any will be matched + * @param array $defaults (default []) Values to set for this route. */ public function __construct( public readonly string $uriPattern, - public readonly ?string $name = null, + public readonly string $name = '', public readonly array $httpMethods = [], public readonly array $defaults = [], ) { - if (str_contains($uriPattern, '{@controller}') || str_contains($uriPattern, '{@action}')) { - throw new \DomainException(sprintf('It is not allowed to override {@controller} or {@action} in route annotations "%s"', $uriPattern), 1711129634); + if ($uriPattern === '' || str_starts_with($uriPattern, '/')) { + throw new \DomainException(sprintf('Uri pattern must not be empty or begin with a slash: "%s"', $uriPattern), 1711529592); + } + foreach ($httpMethods as $httpMethod) { + if ($httpMethod === '' || ctype_lower($httpMethod)) { + throw new \DomainException(sprintf('Http method must not be empty or be lower case: "%s"', $httpMethod), 1711530485); + } } - if (in_array(array_keys($defaults), ['@package', '@subpackage', '@controller', '@action'])) { - throw new \DomainException(sprintf('It is not allowed to override @package, @controller, @subpackage and @action in route annotation defaults "%s"', $uriPattern), 1711129638); + foreach (self::PRESERVED_DEFAULTS as $preservedDefaultName) { + if (str_contains($uriPattern, sprintf('{%s}', $preservedDefaultName))) { + throw new \DomainException(sprintf('It is not allowed to use "%s" in the uri pattern "%s"', $preservedDefaultName, $uriPattern), 1711129634); + } + if (array_key_exists($preservedDefaultName, $defaults)) { + throw new \DomainException(sprintf('It is not allowed to override "%s" as default', $preservedDefaultName), 1711129638); + } } } } diff --git a/Neos.Flow/Tests/Unit/Mvc/Routing/AttributeRoutesProviderTest.php b/Neos.Flow/Tests/Unit/Mvc/Routing/AttributeRoutesProviderTest.php index 22b7de2560..0a63ca722b 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Routing/AttributeRoutesProviderTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Routing/AttributeRoutesProviderTest.php @@ -76,7 +76,12 @@ public function routesFromAnnotationAreCreatedWhenClassNamesMatch(): void ->with('Vendor\Example\Controller\ExampleController', 'specialAction', Flow\Route::class) ->willReturn([ new Flow\Route(uriPattern: 'my/path'), - new Flow\Route(uriPattern: 'my/other/path', name: 'specialRoute', defaults: ['test' => 'foo'], httpMethods: ['get', 'post']) + new Flow\Route( + uriPattern: 'my/other/path', + name: 'specialRoute', + httpMethods: ['GET', 'POST'], + defaults: ['test' => 'foo'] + ) ]); $this->mockObjectManager->expects($this->once()) @@ -111,7 +116,7 @@ public function routesFromAnnotationAreCreatedWhenClassNamesMatch(): void '@format' => 'html', 'test' => 'foo', ]); - $expectedRoute2->setHttpMethods(['get', 'post']); + $expectedRoute2->setHttpMethods(['GET', 'POST']); $this->assertEquals( Routes::create($expectedRoute1, $expectedRoute2), diff --git a/Neos.Flow/Tests/Unit/Mvc/Routing/Dto/RouteAnnotationTest.php b/Neos.Flow/Tests/Unit/Mvc/Routing/Dto/RouteAnnotationTest.php index ae1b605c39..5069ab6e20 100644 --- a/Neos.Flow/Tests/Unit/Mvc/Routing/Dto/RouteAnnotationTest.php +++ b/Neos.Flow/Tests/Unit/Mvc/Routing/Dto/RouteAnnotationTest.php @@ -35,19 +35,6 @@ public function simpleRoutes() self::assertSame(['test' => 'foo'], $route->defaults); } - /** - * @test - */ - public function httpMethodIsUpperCase() - { - /** @see \Neos\Flow\Mvc\Routing\Route::matches() where we do case-sensitive comparison */ - $route = new Flow\Route( - uriPattern: 'my/other/path', - httpMethods: ['post'] - ); - self::assertSame(['POST'], $route->httpMethods); - } - /** * @test */ @@ -83,7 +70,7 @@ public function uriPatternMustNotStartWithLeadingSlash() */ public function uriPatternMustNotBeEmpty() { - $this->expectExceptionCode(1711529595); + $this->expectExceptionCode(1711529592); new Flow\Route(uriPattern: ''); } @@ -97,4 +84,18 @@ public function httpMethodMustNotBeEmptyString() new Flow\Route(uriPattern: 'foo', httpMethods: ['']); } + + /** + * @test + */ + public function httpMethodMustBeUpperCase() + { + $this->expectExceptionCode(1711530485); + + /** @see \Neos\Flow\Mvc\Routing\Route::matches() where we do case-sensitive comparison against uppercase */ + new Flow\Route( + uriPattern: 'my/other/path', + httpMethods: ['post'] + ); + } }