From eb6aee6e1a077e1a0fed402498a6de0187d74e00 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 29 Nov 2018 11:30:41 +0100 Subject: [PATCH] [Routing] fix greediness of trailing slash --- .../Matcher/Dumper/PhpMatcherDumper.php | 67 +++++++++++++------ .../Component/Routing/Matcher/UrlMatcher.php | 20 ++++-- .../Tests/Fixtures/dumper/url_matcher1.php | 43 ++++++++---- .../Tests/Fixtures/dumper/url_matcher10.php | 11 ++- .../Tests/Fixtures/dumper/url_matcher11.php | 16 +++-- .../Tests/Fixtures/dumper/url_matcher12.php | 11 ++- .../Tests/Fixtures/dumper/url_matcher13.php | 6 +- .../Tests/Fixtures/dumper/url_matcher2.php | 59 +++++++++++----- .../Tests/Fixtures/dumper/url_matcher3.php | 17 +++-- .../Tests/Fixtures/dumper/url_matcher4.php | 7 +- .../Tests/Fixtures/dumper/url_matcher5.php | 26 ++++--- .../Tests/Fixtures/dumper/url_matcher6.php | 16 +++-- .../Tests/Fixtures/dumper/url_matcher7.php | 26 ++++--- .../Tests/Fixtures/dumper/url_matcher8.php | 11 ++- .../Routing/Tests/Matcher/UrlMatcherTest.php | 19 ++++++ 15 files changed, 245 insertions(+), 110 deletions(-) diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php index c33b7d06c5..cb6aa6fda6 100644 --- a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php +++ b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php @@ -550,16 +550,23 @@ EOF; private function compileSwitchDefault(bool $hasVars, bool $matchHost): string { $code = sprintf(" - if ('/' !== \$pathinfo) { - if (!\$hasTrailingSlash && '/' === \$pathinfo[-1]%s) { - %s; - } - if (\$hasTrailingSlash && '/' !== \$pathinfo[-1]) { - %2\$s; + if ('/' !== \$pathinfo) {%s + if (\$hasTrailingSlash !== ('/' === \$pathinfo[-1])) {%s + break; } }\n", - $hasVars ? ' && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n[\'MARK\']' : '', - $this->supportsRedirections ? 'return null' : 'break' + $hasVars ? " + if ('/' === \$pathinfo[-1]) { + if (preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) { + \$matches = \$n; + } else { + \$hasTrailingSlash = true; + } + }\n" : '', + $this->supportsRedirections ? " + if (!\$requiredMethods || isset(\$requiredMethods['GET'])) { + return null; + }" : '' ); if ($hasVars) { @@ -616,25 +623,41 @@ EOF; */ private function compileRoute(Route $route, string $name, bool $checkHost, bool $hasTrailingSlash): string { - $code = " // $name"; - - if ('/' !== $route->getPath()) { - $code .= sprintf(" - if ('/' !== \$pathinfo && '/' %s \$pathinfo[-1]) { - %s; - }\n", - $hasTrailingSlash ? '!==' : '===', - $this->supportsRedirections ? 'return null' : 'break' - ); - } else { - $code .= "\n"; - } - $compiledRoute = $route->compile(); $conditions = array(); $matches = (bool) $compiledRoute->getPathVariables(); $hostMatches = (bool) $compiledRoute->getHostVariables(); $methods = array_flip($route->getMethods()); + $code = " // $name"; + + if ('/' === $route->getPath()) { + $code .= "\n"; + } elseif (!$matches) { + $code .= sprintf(" + if ('/' !== \$pathinfo && '/' %s \$pathinfo[-1]) { + %s; + }\n\n", + $hasTrailingSlash ? '!==' : '===', + $this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break' + ); + } elseif ($hasTrailingSlash) { + $code .= sprintf(" + if ('/' !== \$pathinfo[-1]) { + %s; + } + if ('/' !== \$pathinfo && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) { + \$matches = \$n; + }\n\n", + $this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break' + ); + } else { + $code .= sprintf(" + if ('/' !== \$pathinfo && '/' === \$pathinfo[-1] && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) { + %s; + }\n\n", + $this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break' + ); + } if ($route->getCondition()) { $expression = $this->getExpressionLanguage()->compile($route->getCondition(), array('context', 'request')); diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index 13b231bf96..f0f76c2a27 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php @@ -135,11 +135,12 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface foreach ($routes as $name => $route) { $compiledRoute = $route->compile(); $staticPrefix = $compiledRoute->getStaticPrefix(); + $requiredMethods = $route->getMethods(); // check the static prefix of the URL first. Only use the more expensive preg_match when it matches if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) { // no-op - } elseif (!$supportsTrailingSlash) { + } elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods))) { continue; } elseif ('/' === $staticPrefix[-1] && substr($staticPrefix, 0, -1) === $pathinfo) { return; @@ -161,11 +162,18 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface } if ($supportsTrailingSlash) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1))) { - return; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $m)) { + $matches = $m; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return; + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || \in_array('GET', $requiredMethods)) { + return; + } + continue; } } @@ -181,7 +189,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface } $hasRequiredScheme = !$route->getSchemes() || $route->hasScheme($this->context->getScheme()); - if ($requiredMethods = $route->getMethods()) { + if ($requiredMethods) { // HEAD and GET are equivalent as per RFC if ('HEAD' === $method = $this->context->getMethod()) { $method = 'GET'; diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php index 472b96a35c..340a38ffe8 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php @@ -55,10 +55,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - break; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } @@ -145,15 +142,23 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches = array('foo' => $matches[1] ?? null); // baz4 - if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) { + if ('/' !== $pathinfo[-1]) { break; } + if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } + return $this->mergeDefaults(array('_route' => 'baz4') + $matches, array()); // baz5 - if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) { + if ('/' !== $pathinfo[-1]) { break; } + if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } + $ret = $this->mergeDefaults(array('_route' => 'baz5') + $matches, array()); if (!isset(($a = array('POST' => 0))[$requestMethod])) { $allow += $a; @@ -164,9 +169,13 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher not_baz5: // baz.baz6 - if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) { + if ('/' !== $pathinfo[-1]) { break; } + if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } + $ret = $this->mergeDefaults(array('_route' => 'baz.baz6') + $matches, array()); if (!isset(($a = array('PUT' => 0))[$requestMethod])) { $allow += $a; @@ -181,9 +190,10 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches = array('foo' => $matches[1] ?? null); // foo1 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { break; } + $ret = $this->mergeDefaults(array('_route' => 'foo1') + $matches, array()); if (!isset(($a = array('PUT' => 0))[$requestMethod])) { $allow += $a; @@ -198,9 +208,10 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches = array('foo1' => $matches[1] ?? null); // foo2 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { break; } + return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array()); break; @@ -208,9 +219,10 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null); // foo3 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { break; } + return $this->mergeDefaults(array('_route' => 'foo3') + $matches, array()); break; @@ -238,10 +250,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - break; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php index 956a8bad68..ab907b2393 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php @@ -2794,10 +2794,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - break; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php index 3cc68ed993..562587cbff 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php @@ -119,11 +119,19 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - return null; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher12.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher12.php index 8b446a2fc2..d92394f5a6 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher12.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher12.php @@ -64,10 +64,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - break; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher13.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher13.php index 4965615b43..55ceb3f675 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher13.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher13.php @@ -45,15 +45,17 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches = array('foo' => $matches[1] ?? null, 'foo' => $matches[2] ?? null); // r1 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { break; } + return $this->mergeDefaults(array('_route' => 'r1') + $matches, array()); // r2 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { break; } + return $this->mergeDefaults(array('_route' => 'r2') + $matches, array()); break; diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php index c4bf167f30..3db8bbc8b6 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php @@ -92,11 +92,11 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - return null; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } @@ -182,15 +182,23 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec $matches = array('foo' => $matches[1] ?? null); // baz4 - if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) { + if ('/' !== $pathinfo[-1]) { return null; } + if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } + return $this->mergeDefaults(array('_route' => 'baz4') + $matches, array()); // baz5 - if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) { - return null; + if ('/' !== $pathinfo[-1]) { + break; } + if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } + $ret = $this->mergeDefaults(array('_route' => 'baz5') + $matches, array()); if (!isset(($a = array('POST' => 0))[$requestMethod])) { $allow += $a; @@ -201,9 +209,13 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec not_baz5: // baz.baz6 - if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) { - return null; + if ('/' !== $pathinfo[-1]) { + break; } + if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } + $ret = $this->mergeDefaults(array('_route' => 'baz.baz6') + $matches, array()); if (!isset(($a = array('PUT' => 0))[$requestMethod])) { $allow += $a; @@ -218,9 +230,10 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec $matches = array('foo' => $matches[1] ?? null); // foo1 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { - return null; + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + break; } + $ret = $this->mergeDefaults(array('_route' => 'foo1') + $matches, array()); if (!isset(($a = array('PUT' => 0))[$requestMethod])) { $allow += $a; @@ -235,9 +248,10 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec $matches = array('foo1' => $matches[1] ?? null); // foo2 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { return null; } + return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array()); break; @@ -245,9 +259,10 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec $matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null); // foo3 - if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { + if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { return null; } + return $this->mergeDefaults(array('_route' => 'foo3') + $matches, array()); break; @@ -275,11 +290,19 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - return null; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php index 16ea1ad4e5..c3f1a8da33 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php @@ -32,6 +32,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { break; } + if (($context->getMethod() == "GET")) { return array('_route' => 'with-condition'); } @@ -47,10 +48,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - break; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } @@ -88,10 +86,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - break; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher4.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher4.php index 5e370d8345..83a344c214 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher4.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher4.php @@ -32,6 +32,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { break; } + $ret = array('_route' => 'put_and_post'); if (!isset(($a = array('PUT' => 0, 'POST' => 1))[$requestMethod])) { $allow += $a; @@ -44,6 +45,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher if ('/' !== $pathinfo && '/' === $pathinfo[-1]) { break; } + $ret = array('_route' => 'put_and_get_and_head'); if (!isset(($a = array('PUT' => 0, 'GET' => 1, 'HEAD' => 2))[$canonicalMethod])) { $allow += $a; @@ -67,10 +69,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - break; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher5.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher5.php index 6c01afb098..ad94919fcb 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher5.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher5.php @@ -84,11 +84,11 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - return null; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } @@ -127,11 +127,19 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - return null; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher6.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher6.php index 746082cbbe..749ebd1183 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher6.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher6.php @@ -45,10 +45,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - break; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } @@ -104,10 +101,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - break; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher7.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher7.php index 68547bf391..bb0bda2b2a 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher7.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher7.php @@ -80,11 +80,11 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1]) { - return null; - } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } @@ -139,11 +139,19 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - return null; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { - return null; + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { + if (!$requiredMethods || isset($requiredMethods['GET'])) { + return null; + } + break; } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher8.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher8.php index e8e71e427b..f80eb4c9d8 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher8.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher8.php @@ -52,10 +52,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m]; if ('/' !== $pathinfo) { - if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { - break; + if ('/' === $pathinfo[-1]) { + if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) { + $matches = $n; + } else { + $hasTrailingSlash = true; + } } - if ($hasTrailingSlash && '/' !== $pathinfo[-1]) { + + if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) { break; } } diff --git a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php index 1816ba197d..136fd17c7e 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php @@ -689,6 +689,25 @@ class UrlMatcherTest extends TestCase $this->assertEquals('a', $matcher->match('/foo/')['_route']); } + public function testSlashVariant2() + { + $coll = new RouteCollection(); + $coll->add('a', new Route('/foo/{bar}/', array(), array('bar' => '.*'))); + + $matcher = $this->getUrlMatcher($coll); + $this->assertEquals(array('_route' => 'a', 'bar' => 'bar'), $matcher->match('/foo/bar/')); + } + + public function testSlashWithVerb() + { + $coll = new RouteCollection(); + $coll->add('a', new Route('/{foo}', array(), array(), array(), '', array(), array('put', 'delete'))); + $coll->add('b', new Route('/bar/')); + + $matcher = $this->getUrlMatcher($coll); + $this->assertSame(array('_route' => 'b'), $matcher->match('/bar/')); + } + protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null) { return new UrlMatcher($routes, $context ?: new RequestContext());