From acefeb9b5a328e9c606ef86205d1fab269586c0e Mon Sep 17 00:00:00 2001 From: HypeMC Date: Thu, 14 Apr 2022 09:22:15 +0200 Subject: [PATCH] [Routing] Add route_parameters variable to condition expression --- UPGRADE-6.2.md | 7 ++++ src/Symfony/Component/Routing/CHANGELOG.md | 6 ++++ .../Dumper/CompiledUrlMatcherDumper.php | 4 +-- .../Dumper/CompiledUrlMatcherTrait.php | 33 +++++++++---------- .../Routing/Matcher/TraceableUrlMatcher.php | 6 ++-- .../Component/Routing/Matcher/UrlMatcher.php | 25 +++++++++++--- .../Fixtures/dumper/compiled_url_matcher3.php | 9 +++-- .../Dumper/CompiledUrlMatcherDumperTest.php | 4 +++ .../Tests/Matcher/TraceableUrlMatcherTest.php | 9 +++++ .../Routing/Tests/Matcher/UrlMatcherTest.php | 31 +++++++++++++++++ 10 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 UPGRADE-6.2.md diff --git a/UPGRADE-6.2.md b/UPGRADE-6.2.md new file mode 100644 index 0000000000000..4dc684cab8f7d --- /dev/null +++ b/UPGRADE-6.2.md @@ -0,0 +1,7 @@ +UPGRADE FROM 6.1 to 6.2 +======================= + +Routing +------- + + * Add argument `$routeParameters` to `UrlMatcher::handleRouteRequirements()` diff --git a/src/Symfony/Component/Routing/CHANGELOG.md b/src/Symfony/Component/Routing/CHANGELOG.md index 36181ac71600a..8d6f80a152990 100644 --- a/src/Symfony/Component/Routing/CHANGELOG.md +++ b/src/Symfony/Component/Routing/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG ========= +6.2 +--- + + * Add `params` variable to condition expression + * Deprecate not passing route parameters as the forth argument to `UrlMatcher::handleRouteRequirements()` + 6.1 --- diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherDumper.php index 8aba7d74e5889..c3953d08e44cc 100644 --- a/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherDumper.php +++ b/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherDumper.php @@ -115,7 +115,7 @@ public function getCompiledRoutes(bool $forDump = false): array } $checkConditionCode = <<indent(implode("\n", $conditions), 3)} } @@ -426,7 +426,7 @@ private function compileRoute(Route $route, string $name, string|array|null $var } if ($condition = $route->getCondition()) { - $condition = $this->getExpressionLanguage()->compile($condition, ['context', 'request']); + $condition = $this->getExpressionLanguage()->compile($condition, ['context', 'request', 'params']); $condition = $conditions[$condition] ??= (str_contains($condition, '$request') ? 1 : -1) * \count($conditions); } else { $condition = null; diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherTrait.php b/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherTrait.php index 7e869e0491960..207d054fb3ed5 100644 --- a/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherTrait.php +++ b/src/Symfony/Component/Routing/Matcher/Dumper/CompiledUrlMatcherTrait.php @@ -92,10 +92,6 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche $supportsRedirections = 'GET' === $canonicalMethod && $this instanceof RedirectableUrlMatcherInterface; foreach ($this->staticRoutes[$trimmedPathinfo] ?? [] as [$ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash, , $condition]) { - if ($condition && !($this->checkCondition)($condition, $context, 0 < $condition ? $request ??= $this->request ?: $this->createRequest($pathinfo) : null)) { - continue; - } - if ($requiredHost) { if ('{' !== $requiredHost[0] ? $requiredHost !== $host : !preg_match($requiredHost, $host, $hostMatches)) { continue; @@ -106,6 +102,10 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche } } + if ($condition && !($this->checkCondition)($condition, $context, 0 < $condition ? $request ??= $this->request ?: $this->createRequest($pathinfo) : null, $ret)) { + continue; + } + if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) { if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) { return $allow = $allowSchemes = []; @@ -132,13 +132,8 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche foreach ($this->regexpList as $offset => $regex) { while (preg_match($regex, $matchedPathinfo, $matches)) { foreach ($this->dynamicRoutes[$m = (int) $matches['MARK']] as [$ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash, $hasTrailingVar, $condition]) { - if (null !== $condition) { - if (0 === $condition) { // marks the last route in the regexp - continue 3; - } - if (!($this->checkCondition)($condition, $context, 0 < $condition ? $request ??= $this->request ?: $this->createRequest($pathinfo) : null)) { - continue; - } + if (0 === $condition) { // marks the last route in the regexp + continue 3; } $hasTrailingVar = $trimmedPathinfo !== $pathinfo && $hasTrailingVar; @@ -151,17 +146,21 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche } } - if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) { - if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) { - return $allow = $allowSchemes = []; + foreach ($vars as $i => $v) { + if (isset($matches[1 + $i])) { + $ret[$v] = $matches[1 + $i]; } + } + + if ($condition && !($this->checkCondition)($condition, $context, 0 < $condition ? $request ??= $this->request ?: $this->createRequest($pathinfo) : null, $ret)) { continue; } - foreach ($vars as $i => $v) { - if (isset($matches[1 + $i])) { - $ret[$v] = $matches[1 + $i]; + if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) { + if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) { + return $allow = $allowSchemes = []; } + continue; } if ($requiredSchemes && !isset($requiredSchemes[$context->getScheme()])) { diff --git a/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php b/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php index 323941e788e24..0f0cb3fd9e6a2 100644 --- a/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php @@ -115,7 +115,9 @@ protected function matchCollection(string $pathinfo, RouteCollection $routes): a continue; } - $status = $this->handleRouteRequirements($pathinfo, $name, $route); + $attributes = $this->getAttributes($route, $name, array_replace($matches, $hostMatches)); + + $status = $this->handleRouteRequirements($pathinfo, $name, $route, $attributes); if (self::REQUIREMENT_MISMATCH === $status[0]) { $this->addTrace(sprintf('Condition "%s" does not evaluate to "true"', $route->getCondition()), self::ROUTE_ALMOST_MATCHES, $name, $route); @@ -146,7 +148,7 @@ protected function matchCollection(string $pathinfo, RouteCollection $routes): a $this->addTrace('Route matches!', self::ROUTE_MATCHES, $name, $route); - return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, $status[1] ?? [])); + return array_replace($attributes, $status[1] ?? []); } return []; diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index dc9e09f00cfb2..b077c0bc3eb7a 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php @@ -167,7 +167,9 @@ protected function matchCollection(string $pathinfo, RouteCollection $routes): a continue; } - $status = $this->handleRouteRequirements($pathinfo, $name, $route); + $attributes = $this->getAttributes($route, $name, array_replace($matches, $hostMatches)); + + $status = $this->handleRouteRequirements($pathinfo, $name, $route, $attributes); if (self::REQUIREMENT_MISMATCH === $status[0]) { continue; @@ -190,7 +192,7 @@ protected function matchCollection(string $pathinfo, RouteCollection $routes): a continue; } - return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, $status[1] ?? [])); + return array_replace($attributes, $status[1] ?? []); } return []; @@ -220,10 +222,25 @@ protected function getAttributes(Route $route, string $name, array $attributes): * * @return array The first element represents the status, the second contains additional information */ - protected function handleRouteRequirements(string $pathinfo, string $name, Route $route): array + protected function handleRouteRequirements(string $pathinfo, string $name, Route $route/*, array $routeParameters*/): array { + if (\func_num_args() < 4) { + trigger_deprecation('symfony/routing', '6.2', 'The "%s()" method will have a new "array $routeParameters" argument in version 7.0, not defining it is deprecated.', __METHOD__); + $routeParameters = []; + } else { + $routeParameters = func_get_arg(3); + + if (!\is_array($routeParameters)) { + throw new \TypeError(sprintf('"%s": Argument $routeParameters is expected to be an array, got "%s".', __METHOD__, get_debug_type($routeParameters))); + } + } + // expression condition - if ($route->getCondition() && !$this->getExpressionLanguage()->evaluate($route->getCondition(), ['context' => $this->context, 'request' => $this->request ?: $this->createRequest($pathinfo)])) { + if ($route->getCondition() && !$this->getExpressionLanguage()->evaluate($route->getCondition(), [ + 'context' => $this->context, + 'request' => $this->request ?: $this->createRequest($pathinfo), + 'params' => $routeParameters, + ])) { return [self::REQUIREMENT_MISMATCH, null]; } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/compiled_url_matcher3.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/compiled_url_matcher3.php index 4fe52b3c83ca6..d74be502c5331 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/compiled_url_matcher3.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/compiled_url_matcher3.php @@ -14,17 +14,20 @@ [ // $regexpList 0 => '{^(?' .'|/rootprefix/([^/]++)(*:27)' + .'|/with\\-condition/(\\d+)(*:56)' .')/?$}sD', ], [ // $dynamicRoutes - 27 => [ - [['_route' => 'dynamic'], ['var'], null, null, false, true, null], + 27 => [[['_route' => 'dynamic'], ['var'], null, null, false, true, null]], + 56 => [ + [['_route' => 'with-condition-dynamic'], ['id'], null, null, false, true, -2], [null, null, null, null, false, false, 0], ], ], - static function ($condition, $context, $request) { // $checkCondition + static function ($condition, $context, $request, $params) { // $checkCondition switch ($condition) { case -1: return ($context->getMethod() == "GET"); + case -2: return ($params["id"] < 100); } }, ]; diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/CompiledUrlMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/CompiledUrlMatcherDumperTest.php index 4886d717685c6..0df1c0cf40589 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/CompiledUrlMatcherDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/CompiledUrlMatcherDumperTest.php @@ -296,6 +296,10 @@ public function getRouteCollections() $route = new Route('/with-condition'); $route->setCondition('context.getMethod() == "GET"'); $rootprefixCollection->add('with-condition', $route); + $route = new Route('/with-condition/{id}'); + $route->setRequirement('id', '\d+'); + $route->setCondition("params['id'] < 100"); + $rootprefixCollection->add('with-condition-dynamic', $route); /* test case 4 */ $headMatchCasesCollection = new RouteCollection(); diff --git a/src/Symfony/Component/Routing/Tests/Matcher/TraceableUrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/TraceableUrlMatcherTest.php index b33e93caa1a8d..03b11a697aa0d 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/TraceableUrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/TraceableUrlMatcherTest.php @@ -104,6 +104,7 @@ public function testRoutesWithConditions() { $routes = new RouteCollection(); $routes->add('foo', new Route('/foo', [], [], [], 'baz', [], [], "request.headers.get('User-Agent') matches '/firefox/i'")); + $routes->add('bar', new Route('/bar/{id}', [], [], [], 'baz', [], [], "params['id'] < 100")); $context = new RequestContext(); $context->setHost('baz'); @@ -117,6 +118,14 @@ public function testRoutesWithConditions() $matchingRequest = Request::create('/foo', 'GET', [], [], [], ['HTTP_USER_AGENT' => 'Firefox']); $traces = $matcher->getTracesForRequest($matchingRequest); $this->assertEquals('Route matches!', $traces[0]['log']); + + $notMatchingRequest = Request::create('/bar/1000', 'GET'); + $traces = $matcher->getTracesForRequest($notMatchingRequest); + $this->assertEquals("Condition \"params['id'] < 100\" does not evaluate to \"true\"", $traces[1]['log']); + + $matchingRequest = Request::create('/bar/10', 'GET'); + $traces = $matcher->getTracesForRequest($matchingRequest); + $this->assertEquals('Route matches!', $traces[1]['log']); } protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null) diff --git a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php index 74ceb1e0be213..41126642e4767 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php @@ -474,6 +474,37 @@ public function testRequestCondition() $this->assertEquals(['bar' => 'bar', '_route' => 'foo'], $matcher->match('/foo/bar')); } + public function testRouteParametersCondition() + { + $coll = new RouteCollection(); + $route = new Route('/foo'); + $route->setCondition("params['_route'] matches '/^s[a-z]+$/'"); + $coll->add('static', $route); + $route = new Route('/bar'); + $route->setHost('en.example.com'); + $route->setCondition("params['_route'] matches '/^s[a-z\-]+$/'"); + $coll->add('static-with-host', $route); + $route = new Route('/foo/{id}'); + $route->setCondition("params['id'] < 100"); + $coll->add('dynamic1', $route); + $route = new Route('/foo/{id}'); + $route->setCondition("params['id'] > 100 and params['id'] < 1000"); + $coll->add('dynamic2', $route); + $route = new Route('/bar/{id}/'); + $route->setCondition("params['id'] < 100"); + $coll->add('dynamic-with-slash', $route); + $matcher = $this->getUrlMatcher($coll, new RequestContext('/sub/front.php', 'GET', 'en.example.com')); + + $this->assertEquals(['_route' => 'static'], $matcher->match('/foo')); + $this->assertEquals(['_route' => 'static-with-host'], $matcher->match('/bar')); + $this->assertEquals(['_route' => 'dynamic1', 'id' => '10'], $matcher->match('/foo/10')); + $this->assertEquals(['_route' => 'dynamic2', 'id' => '200'], $matcher->match('/foo/200')); + $this->assertEquals(['_route' => 'dynamic-with-slash', 'id' => '10'], $matcher->match('/bar/10/')); + + $this->expectException(ResourceNotFoundException::class); + $matcher->match('/foo/3000'); + } + public function testDecodeOnce() { $coll = new RouteCollection();