10000 [Router] Fix DumpedUrlMatcher behaviour with trailing slash by xavierleune · Pull Request #33362 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Router] Fix DumpedUrlMatcher behaviour with trailing slash #33362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Router] Fix DumpedUrlMatcher behaviour with trailing slash #33362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[Router] Fix DumpedUrlMatcher behaviour with trailing slash
  • Loading branch information
Xavier Leune committed Aug 27, 2019
commit 8a2b788e78971794b874539dbd3a6fa583e040b4
60 changes: 52 additions & 8 deletions src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,30 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
*/
private function generateMatchMethod($supportsRedirections)
{
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections), "\n");

return <<<EOF
public function match(\$rawPathinfo)
{
try {
return \$this->matchWithoutTrailingSlashManagement(\$rawPathinfo);
} catch (MethodNotAllowedException \$e) {
throw \$e;
} catch (ResourceNotFoundException \$e) {
return \$this->matchWithTrailingSlashManagement(\$rawPathinfo);
}
}

{$this->generateMatchWithTrailingSlashManagement($supportsRedirections)}

{$this->generateMatchWithoutTrailingSlashManagement($supportsRedirections)}
EOF;
}

private function generateMatchWithTrailingSlashManagement($supportsRedirections)
{
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections, true), "\n");

return <<<EOF
public function matchWithTrailingSlashManagement(\$rawPathinfo)
{
\$allow = [];
\$pathinfo = rawurldecode(\$rawPathinfo);
Expand All @@ -109,6 +129,30 @@ public function match(\$rawPathinfo)
\$canonicalMethod = 'GET';
}

$code

throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
}
EOF;
}

private function generateMatchWithoutTrailingSlashManagement($supportsRedirections)
{
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections, false), "\n");

return <<<EOF
public function matchWithoutTrailingSlashManagement(\$rawPathinfo)
{
\$allow = [];
\$pathinfo = rawurldecode(\$rawPathinfo);
\$context = \$this->context;
\$request = \$this->request ?: \$this->createRequest(\$pathinfo);
\$requestMethod = \$canonicalMethod = \$context->getMethod();

if ('HEAD' === \$requestMethod) {
\$canoni 8000 calMethod = 'GET';
}

$code

throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
Expand All @@ -124,7 +168,7 @@ public function match(\$rawPathinfo)
*
* @return string PHP code
*/
private function compileRoutes(RouteCollection $routes, $supportsRedirections)
private function compileRoutes(RouteCollection $routes, $supportsRedirections, $trailingSlashManagement = true)
{
$fetchedHost = false;
$groups = $this->groupRoutesByHostRegex($routes);
Expand All @@ -141,7 +185,7 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections)
}

$tree = $this->buildStaticPrefixCollection($collection);
$groupCode = $this->compileStaticPrefixRoutes($tree, $supportsRedirections);
$groupCode = $this->compileStaticPrefixRoutes($tree, $supportsRedirections, $trailingSlashManagement);

if (null !== $regex) {
// apply extra indention at each line (except empty ones)
Expand Down Expand Up @@ -184,7 +228,7 @@ private function buildStaticPrefixCollection(DumperCollection $collection)
*
* @return string PHP code
*/
private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $supportsRedirections, $ifOrElseIf = 'if')
private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $supportsRedirections, $trailingSlashManagement, $ifOrElseIf = 'if')
{
$code = '';
$prefix = $collection->getPrefix();
Expand All @@ -200,7 +244,7 @@ private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $
$code .= $this->compileStaticPrefixRoutes($route, $supportsRedirections, $ifOrElseIf);
$ifOrElseIf = 'elseif';
} else {
$code .= $this->compileRoute($route[1]->getRoute(), $route[1]->getName(), $supportsRedirections, $prefix)."\n";
$code .= $this->compileRoute($route[1]->getRoute(), $route[1]->getName(), $supportsRedirections, $trailingSlashManagement, $prefix)."\n";
$ifOrElseIf = 'if';
}
}
Expand All @@ -226,7 +270,7 @@ private function compileStaticPrefixRoutes(StaticPrefixCollection $collection, $
*
* @throws \LogicException
*/
private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null)
private function compileRoute(Route $route, $name, $supportsRedirections, $trailingSlashManagement, $parentPrefix = null)
{
$code = '';
$compiledRoute = $route->compile();
Expand All @@ -236,7 +280,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
$hostMatches = false;
$methods = $route->getMethods();

$supportsTrailingSlash = $supportsRedirections && (!$methods || \in_array('GET', $methods));
$supportsTrailingSlash = $supportsRedirections && (!$methods || \in_array('GET', $methods)) && $trailingSlashManagement;
$regex = $compiledRoute->getRegex();

if (!\count($compiledRoute->getPathVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#'.('u' === substr($regex, -1) ? 'u' : ''), $regex, $m)) {
Expand Down
30 changes: 28 additions & 2 deletions src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Routing\Matcher;

use Symfony\Component\Routing\Exception\MethodNotAllowedException;
use Symfony\Component\Routing\Exception\NoConfigurationException;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\Route;

Expand All @@ -25,14 +27,22 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
public function match($pathinfo)
{
try {
$parameters = parent::match($pathinfo);
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo, false);
} catch (ResourceNotFoundException $e) {
if ('/' === substr($pathinfo, -1) || !\in_array($this->context->getMethod(), ['HEAD', 'GET'])) {
throw $e;
}

try {
$parameters = parent::match($pathinfo.'/');
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo.'/', true);

return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
} catch (ResourceNotFoundException $e2) {
throw $e;
}
} catch (MethodNotAllowedException $e) {
try {
$parameters = $this->matchCollectionByTrailingSlashSupport($pathinfo.'/', true);

return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
} catch (ResourceNotFoundException $e2) {
Expand Down Expand Up @@ -62,4 +72,20 @@ protected function handleRouteRequirements($pathinfo, $name, Route $route)

return [self::REQUIREMENT_MATCH, null];
}

protected function matchCollectionByTrailingSlashSupport($pathinfo, $trailingSlashSupport)
{
$this->allow = [];
if ($ret = $this->matchCollection(r 8000 awurldecode($pathinfo), $this->routes, $trailingSlashSupport)) {
return $ret;
}

if ('/' === $pathinfo && !$this->allow) {
throw new NoConfigurationException();
}

throw 0 < \count($this->allow)
? new MethodNotAllowedException(array_unique($this->allow))
: new ResourceNotFoundException(sprintf('No routes found for "%s".', $pathinfo));
}
}
3 changes: 1 addition & 2 deletions src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@ public function getTracesForRequest(Request $request)
return $traces;
}

protected function matchCollection($pathinfo, RouteCollection $routes)
protected function matchCollection($pathinfo, RouteCollection $routes, $supportsTrailingSlash)
{
// HEAD and GET are equivalent as per RFC
if ('HEAD' === $method = $this->context->getMethod()) {
$method = 'GET';
}
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;

foreach ($routes as $name => $route) {
$compiledRoute = $route->compile();
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Routing/Matcher/UrlMatcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ public function getContext()
public function match($pathinfo)
{
$this->allow = [];
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;

if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes)) {
if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes, $supportsTrailingSlash)) {
return $ret;
}

Expand Down Expand Up @@ -116,13 +117,12 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
* @throws ResourceNotFoundException If the resource could not be found
* @throws MethodNotAllowedException If the resource was found but the request method is not allowed
*/
protected function matchCollection($pathinfo, RouteCollection $routes)
protected function matchCollection($pathinfo, RouteCollection $routes, $supportsTrailingSlash)
{
// HEAD and GET are equivalent as per RFC
if ('HEAD' === $method = $this->context->getMethod()) {
$method = 'GET';
}
$supportsTrailingSlash = '/' !== $pathinfo && '' !== $pathinfo && $this instanceof RedirectableUrlMatcherInterface;

foreach ($routes as $name => $route) {
$compiledRoute = $route->compile();
Expand Down
11 changes: 0 additions & 11 deletions src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,6 @@ public function testSchemeRequirement()
$this->assertSame(['_route' => 'foo'], $matcher->match('/foo'));
}

public function testFallbackPage()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guarded against the regression found in #29297

{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo/'));
$coll->add('bar', new Route('/{name}'));

$matcher = $this->getUrlMatcher($coll);
$matcher->expects($this->once())->method('redirect')->with('/foo/')->willReturn(['_route' => 'foo']);
$this->assertSame(['_route' => 'foo'], $matcher->match('/foo'));
}

public function testSlashAndVerbPrecedenceWithRedirection()
{
$coll = new RouteCollection();
Expand Down
22 changes: 22 additions & 0 deletions src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,28 @@ public function testGreedyTrailingRequirement()
$this->assertEquals(['_route' => 'a', 'a' => 'foo/'], $matcher->match('/foo/'));
}

public function testRouteWithAndWithoutTrailingSlash()
{
$coll = new RouteCollection();
$coll->add('hard_route_with_trailing_slash', new Route('/a/'));
$coll->add('dynamic_route_without_trailing_slash', new Route('/{b}', [], ['b' => '.+']));

$matcher = $this->getUrlMatcher($coll);

$this->assertEquals(['_route' => 'hard_route_with_trailing_slash'], $matcher->match('/a/'), 'truc3');
$this->assertEquals(['_route' => 'dynamic_route_without_trailing_slash', 'b' => 'a'], $matcher->match('/a'), 'truc4');

// Order of definitions matters
$coll = new RouteCollection();
$coll->add('dynamic_route_without_trailing_slash', new Route('/{b}', [], ['b' => '.+']));
$coll->add('hard_route_with_trailing_slash', new Route('/a/'));

$matcher = $this->getUrlMatcher($coll);

$this->assertEquals(['_route' => 'dynamic_route_without_trailing_slash', 'b' => 'a/'], $matcher->match('/a/'), 'truc1');
$this->assertEquals(['_route' => 'dynamic_route_without_trailing_slash', 'b' => 'a'], $matcher->match('/a'), 'truc2');
}

protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
{
return new UrlMatcher($routes, $context ?: new RequestContext());
Expand Down
0