Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ceb4e73

Browse filesBrowse files
fabpotfabpot
committed
bug #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
Copy full SHA for ceb4e73

File tree

8 files changed

+203
-61
lines changed
Filter options

8 files changed

+203
-61
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
+7-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
+19-4Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php
+1-1Lines 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

Copy file name to clipboard
+43Lines 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

Copy file name to clipboard
+39Lines 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\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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php
+23-11Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.