8000 bug #25962 [Routing] Fix trailing slash redirection for non-safe verb… · pierredup/symfony@ceb4e73 · GitHub
[go: up one dir, main page]
Skip to content

Commit ceb4e73

Browse files
fabpotfabpot
committed
bug symfony#25962 [Routing] Fix trailing slash redirection for non-safe verbs (nicolas-grekas)
This PR was merged into the 2.7 branch. Discussion ---------- [Routing] Fix trailing slash redirection for non-safe verbs | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This test dumped matchers using the existing test cases for (Redirectable)UrlMatcher so that we are sure they behave the same. Fixes the differences found while doing so. Commits ------- ad593ae [Routing] Fix trailing slash redirection for non-safe verbs
2 parents 471c410 + ad593ae commit ceb4e73

File tree

8 files changed

+203
-61
lines changed

8 files changed

+203
-61
lines changed

‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public function match(\$rawPathinfo)
101101
\$allow = array();
102102
\$pathinfo = rawurldecode(\$rawPathinfo);
103103
\$context = \$this->context;
104-
\$request = \$this->request;
104+
\$request = \$this->request ?: \$this->createRequest(\$pathinfo);
105105
106106
$code
107107
@@ -283,7 +283,11 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
283283

284284
if ($hasTrailingSlash) {
285285
$code .= <<<EOF
286-
if (substr(\$pathinfo, -1) !== '/') {
286+
if ('/' === substr(\$pathinfo, -1)) {
287+
// no-op
288+
} elseif (!in_array(\$this->context->getMethod(), array('HEAD', 'GET'))) {
289+
goto $gotoname;
290+
} else {
287291
return \$this->redirect(\$rawPathinfo.'/', '$name');
288292
}
289293
@@ -329,7 +333,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
329333
}
330334
$code .= " }\n";
331335

332-
if ($methods) {
336+
if ($methods || $hasTrailingSlash) {
333337
$code .= " $gotoname:\n";
334338
}
335339

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function match($rawPathinfo)
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
23-
$request = $this->request;
23+
$request = $this->request ?: $this->createRequest($pathinfo);
2424

2525
// foo
2626
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#s', $pathinfo, $matches)) {

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function match($rawPathinfo)
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
23-
$request = $this->request;
23+
$request = $this->request ?: $this->createRequest($pathinfo);
2424

2525
// foo
2626
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#s', $pathinfo, $matches)) {
@@ -66,23 +66,33 @@ public function match($rawPathinfo)
6666

6767
// baz3
6868
if ('/test/baz3' === rtrim($pathinfo, '/')) {
69-
if (substr($pathinfo, -1) !== '/') {
69+
if ('/' === substr($pathinfo, -1)) {
70+
// no-op
71+
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
72+
goto not_baz3;
73+
} else {
7074
return $this->redirect($rawPathinfo.'/', 'baz3');
7175
}
7276

7377
return array('_route' => 'baz3');
7478
}
79+
not_baz3:
7580

7681
}
7782

7883
// baz4
7984
if (preg_match('#^/test/(?P<foo>[^/]++)/?$#s', $pathinfo, $matches)) {
80-
if (substr($pathinfo, -1) !== '/') {
85+
if ('/' === substr($pathinfo, -1)) {
86+
// no-op
87+
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
88+
goto not_baz4;
89+
} else {
8190
return $this->redirect($rawPathinfo.'/', 'baz4');
8291
}
8392

8493
return $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array ());
8594
}
95+
not_baz4:
8696

8797
// baz5
8898
if (preg_match('#^/test/(?P<foo>[^/]++)/$#s', $pathinfo, $matches)) {
@@ -170,12 +180,17 @@ public function match($rawPathinfo)
170180

171181
// hey
172182
if ('/multi/hey' === rtrim($pathinfo, '/')) {
173-
if (substr($pathinfo, -1) !== '/') {
183+
if ('/' === substr($pathinfo, -1)) {
184+
// no-op
185+
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
186+
goto not_hey;
187+
} else {
174188
return $this->redirect($rawPathinfo.'/', 'hey');
175189
}
176190

177191
return array('_route' => 'hey');
178192
}
193+
not_hey:
179194

180195
}
181196

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function match($rawPathinfo)
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
23-
$request = $this->request;
23+
$request = $this->request ?: $this->createRequest($pathinfo);
2424

2525
if (0 === strpos($pathinfo, '/rootprefix')) {
2626
// static

‎src/Symfony/Component/Routing/Tests/Matcher/DumpedRedirectableUrlMatcherTest.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Routing\Tests\Matcher;
13+
14+
use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper;
15+
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
16+
use Symfony\Component\Routing\Matcher\UrlMatcher;
17+
use Symfony\Component\Routing\RouteCollection;
18+
use Symfony\Component\Routing\RequestContext;
19+
20+
class DumpedRedirectableUrlMatcherTest extends RedirectableUrlMatcherTest
21+
{
22+
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
23+
{
24+
static $i = 0;
25+
26+
$class = 'DumpedRedirectableUrlMatcher'.++$i;
27+
$dumper = new PhpMatcherDumper($routes);
28+
$dumpedRoutes = eval('?>'.$dumper->dump(array('class' => $class, 'base_class' => 'Symfony\Component\Routing\Tests\Matcher\TestDumpedRedirectableUrlMatcher')));
29+
30+
return $this->getMockBuilder($class)
31+
->setConstructorArgs(array($context ?: new RequestContext()))
32+
->setMethods(array('redirect'))
33+
->getMock();
34+
}
35+
}
36+
37+
class TestDumpedRedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
38+
{
39+
public function redirect($path, $route, $scheme = null)
40+
{
41+
return array();
42+
}
43+
}

‎src/Symfony/Component/Routing/Tests/Matcher/DumpedUrlMatcherTest.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Routing\Tests\Matcher;
13+
14+
use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper;
15+
use Symfony\Component\Routing\ F438 RouteCollection;
16+
use Symfony\Component\Routing\RequestContext;
17+
18+
class DumpedUrlMatcherTest extends UrlMatcherTest
19+
{
20+
/**
21+
* @expectedException \LogicException
22+
* @expectedExceptionMessage The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.
23+
*/
24+
public function testSchemeRequirement()
25+
{
26+
parent::testSchemeRequirement();
27+
}
28+
29+
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
30+
{
31+
static $i = 0;
32+
33+
$class = 'DumpedUrlMatcher'.++$i;
34+
$dumper = new PhpMatcherDumper($routes);
35+
$dumpedRoutes = eval('?>'.$dumper->dump(array('class' => $class)));
36+
37+
return new $class($context ?: new RequestContext());
38+
}
39+
}

‎src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,19 @@
1111

1212
namespace Symfony\Component\Routing\Tests\Matcher;
1313

14-
use PHPUnit\Framework\TestCase;
1514
use Symfony\Component\Routing\Route;
1615
use Symfony\Component\Routing\RouteCollection;
1716
use Symfony\Component\Routing\RequestContext;
1817

19-
class RedirectableUrlMatcherTest extends TestCase
18+
class RedirectableUrlMatcherTest extends UrlMatcherTest
2019
{
2120
public function testRedirectWhenNoSlash()
2221
{
2322
$coll = new RouteCollection();
2423
$coll->add('foo', new Route('/foo/'));
2524

26-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
27-
$matcher->expects($this->once())->method('redirect');
25+
$matcher = $this->getUrlMatcher($coll);
26+
$matcher->expects($this->once())->method('redirect')->will($this->returnValue(array()));
2827
$matcher->match('/foo');
2928
}
3029

@@ -38,7 +37,7 @@ public function testRedirectWhenNoSlashForNonSafeMethod()
3837

3938
$context = new RequestContext();
4039
$context->setMethod('POST');
41-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, $context));
40+
$matcher = $this->getUrlMatcher($coll, $context);
4241
$matcher->match('/foo');
4342
}
4443

@@ -47,7 +46,7 @@ public function testSchemeRedirectRedirectsToFirstScheme()
4746
$coll = new RouteCollection();
4847
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('FTP', 'HTTPS')));
4948

50-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
49+
$matcher = $this->getUrlMatcher($coll);
5150
$matcher
5251
->expects($this->once())
5352
->method('redirect')
@@ -62,11 +61,10 @@ public function testNoSchemaRedirectIfOnOfMultipleSchemesMatches()
6261
$coll = new RouteCollection();
6362
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('https', 'http')));
6463

65-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
64+
$matcher = $this->getUrlMatcher($coll);
6665
$matcher
6766
->expects($this->never())
68-
->method('redirect')
69-
;
67+
->method('redirect');
7068
$matcher->match('/foo');
7169
}
7270

@@ -75,8 +73,22 @@ public function testRedirectPreservesUrlEncoding()
7573
$coll = new RouteCollection();
7674
$coll->add('foo', new Route('/foo:bar/'));
7775

78-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
79-
$matcher->expects($this->once())->method('redirect')->with('/foo%3Abar/');
76+
$matcher = $this->getUrlMatcher($coll);
77+
$matcher->expects($this->once())->method('redirect')->with('/foo%3Abar/')->willReturn(array());
8078
$matcher->match('/foo%3Abar');
8179
}
80+
81+
public function testSchemeRequirement()
82+
{
83+
$coll = new RouteCollection();
84+
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('https')));
85+
$matcher = $this->getUrlMatcher($coll, new RequestContext());
86+
$matcher->expects($this->once())->method('redirect')->with('/foo', 'foo', 'https')->willReturn(array('_route' => 'foo'));
87+
$this->assertSame(array('_route' => 'foo'), $matcher->match('/foo'));
88+
}
89+
90+
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
91+
{
92+
return $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($routes, $context ?: new RequestContext()));
93+
}
8294
}

0 commit comments

Comments
 (0)
0