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 c8d18ba

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[Routing] Fix trailing slash redirection for non-safe verbs
1 parent 1ee3950 commit c8d18ba
Copy full SHA for c8d18ba

File tree

8 files changed

+204
-61
lines changed
Filter options

8 files changed

+204
-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
+24-11Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,20 @@
1111

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

14-
use PHPUnit\Framework\TestCase;
14+
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcher;
1515
use Symfony\Component\Routing\Route;
1616
use Symfony\Component\Routing\RouteCollection;
1717
use Symfony\Component\Routing\RequestContext;
1818

19-
class RedirectableUrlMatcherTest extends TestCase
19+
class RedirectableUrlMatcherTest extends UrlMatcherTest
2020
{
2121
public function testRedirectWhenNoSlash()
2222
{
2323
$coll = new RouteCollection();
2424
$coll->add('foo', new Route('/foo/'));
2525

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

@@ -38,7 +38,7 @@ public function testRedirectWhenNoSlashForNonSafeMethod()
3838

3939
$context = new RequestContext();
4040
$context->setMethod('POST');
41-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, $context));
41+
$matcher = $this->getUrlMatcher($coll, $context);
4242
$matcher->match('/foo');
4343
}
4444

@@ -47,7 +47,7 @@ public function testSchemeRedirectRedirectsToFirstScheme()
4747
$coll = new RouteCollection();
4848
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('FTP', 'HTTPS')));
4949

50-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
50+
$matcher = $this->getUrlMatcher($coll);
5151
$matcher
5252
->expects($this->once())
5353
->method('redirect')
@@ -62,11 +62,10 @@ public function testNoSchemaRedirectIfOnOfMultipleSchemesMatches()
6262
$coll = new RouteCollection();
6363
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('https', 'http')));
6464

65-
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
65+
$matcher = $this->getUrlMatcher($coll);
6666
$matcher
6767
->expects($this->never())
68-
->method('redirect')
69-
;
68+
->method('redirect');
7069
$matcher->match('/foo');
7170
}
7271

@@ -75,8 +74,22 @@ public function testRedirectPreservesUrlEncoding()
7574
$coll = new RouteCollection();
7675
$coll->add('foo', new Route('/foo:bar/'));
7776

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.