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 01229fb

Browse filesBrowse files
fabpotfabpot
committed
bug #25427 Preserve percent-encoding in URLs when performing redirects in the UrlMatcher (mpdude)
This PR was squashed before being merged into the 2.7 branch (closes #25427). Discussion ---------- Preserve percent-encoding in URLs when performing redirects in the UrlMatcher | 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 | While investigating #25373, I found that when the *dumped* `UrlMatcher` performs redirections due to missing trailing slashes on URLs, it does so using an url*de*coded URL. This is wrong, as it may lead to wrong interpretations of URLs upon the next request. For example, think of an URL that contains `%23` in the middle of the path info. Upon redirection, this will be turned into `#` with an obvious effect. Commits ------- 8146510 Preserve percent-encoding in URLs when performing redirects in the UrlMatcher
2 parents 9ff3776 + 8146510 commit 01229fb
Copy full SHA for 01229fb

File tree

6 files changed

+97
-15
lines changed
Filter options

6 files changed

+97
-15
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
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@ private function generateMatchMethod($supportsRedirections)
9696
$code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections), "\n");
9797

9898
return <<<EOF
99-
public function match(\$pathinfo)
99+
public function match(\$rawPathinfo)
100100
{
101101
\$allow = array();
102-
\$pathinfo = rawurldecode(\$pathinfo);
102+
\$pathinfo = rawurldecode(\$rawPathinfo);
103103
\$context = \$this->context;
104104
\$request = \$this->request;
105105
@@ -284,7 +284,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
284284
if ($hasTrailingSlash) {
285285
$code .= <<<EOF
286286
if (substr(\$pathinfo, -1) !== '/') {
287-
return \$this->redirect(\$pathinfo.'/', '$name');
287+
return \$this->redirect(\$rawPathinfo.'/', '$name');
288288
}
289289
290290
@@ -299,7 +299,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
299299
$code .= <<<EOF
300300
\$requiredSchemes = $schemes;
301301
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
302-
return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
302+
return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes));
303303
}
304304
305305

‎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
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ public function __construct(RequestContext $context)
1515
$this->context = $context;
1616
}
1717

18-
public function match($pathinfo)
18+
public function match($rawPathinfo)
1919
{
2020
$allow = array();
21-
$pathinfo = rawurldecode($pathinfo);
21+
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
2323
$request = $this->request;
2424

‎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
+7-7Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ public function __construct(RequestContext $context)
1515
$this->context = $context;
1616
}
1717

18-
public function match($pathinfo)
18+
public function match($rawPathinfo)
1919
{
2020
$allow = array();
21-
$pathinfo = rawurldecode($pathinfo);
21+
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
2323
$request = $this->request;
2424

@@ -67,7 +67,7 @@ public function match($pathinfo)
6767
// baz3
6868
if ('/test/baz3' === rtrim($pathinfo, '/')) {
6969
if (substr($pathinfo, -1) !== '/') {
70-
return $this->redirect($pathinfo.'/', 'baz3');
70+
return $this->redirect($rawPathinfo.'/', 'baz3');
7171
}
7272

7373
return array('_route' => 'baz3');
@@ -78,7 +78,7 @@ public function match($pathinfo)
7878
// baz4
7979
if (preg_match('#^/test/(?P<foo>[^/]++)/?$#s', $pathinfo, $matches)) {
8080
if (substr($pathinfo, -1) !== '/') {
81-
return $this->redirect($pathinfo.'/', 'baz4');
81+
return $this->redirect($rawPathinfo.'/', 'baz4');
8282
}
8383

8484
return $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array ());
@@ -171,7 +171,7 @@ public function match($pathinfo)
171171
// hey
172172
if ('/multi/hey' === rtrim($pathinfo, '/')) {
173173
if (substr($pathinfo, -1) !== '/') {
174-
return $this->redirect($pathinfo.'/', 'hey');
174+
return $this->redirect($rawPathinfo.'/', 'hey');
175175
}
176176

177177
return array('_route' => 'hey');
@@ -318,7 +318,7 @@ public function match($pathinfo)
318318
if ('/secure' === $pathinfo) {
319319
$requiredSchemes = array ( 'https' => 0,);
320320
if (!isset($requiredSchemes[$this->context->getScheme()])) {
321-
return $this->redirect($pathinfo, 'secure', key($requiredSchemes));
321+
return $this->redirect($rawPathinfo, 'secure', key($requiredSchemes));
322322
}
323323

324324
return array('_route' => 'secure');
@@ -328,7 +328,7 @@ public function match($pathinfo)
328328
if ('/nonsecure' === $pathinfo) {
329329
$requiredSchemes = array ( 'http' => 0,);
330330
if (!isset($requiredSchemes[$this->context->getScheme()])) {
331-
return $this->redirect($pathinfo, 'nonsecure', key($requiredSchemes));
331+
return $this->redirect($rawPathinfo, 'nonsecure', key($requiredSchemes));
332332
}
333333

334334
return array('_route' => 'nonsecure');

‎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
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ public function __construct(RequestContext $context)
1515
$this->context = $context;
1616
}
1717

18-
public function match($pathinfo)
18+
public function match($rawPathinfo)
1919
{
2020
$allow = array();
21-
$pathinfo = rawurldecode($pathinfo);
21+
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
2323
$request = $this->request;
2424

‎src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php
+72Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,39 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper;
16+
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
17+
use Symfony\Component\Routing\Matcher\UrlMatcher;
18+
use Symfony\Component\Routing\RequestContext;
1619
use Symfony\Component\Routing\Route;
1720
use Symfony\Component\Routing\RouteCollection;
1821

1922
class PhpMatcherDumperTest extends TestCase
2023
{
24+
/**
25+
* @var string
26+
*/
27+
private $matcherClass;
28+
29+
/**
30+
* @var string
31+
*/
32+
private $dumpPath;
33+
34+
protected function setUp()
35+
{
36+
parent::setUp();
37+
38+
$this->matcherClass = uniqid('ProjectUrlMatcher');
39+
$this->dumpPath = sys_get_temp_dir().DIRECTORY_SEPARATOR.'php_matcher.'.$this->matcherClass.'.php';
40+
}
41+
42+
protected function tearDown()
43+
{
44+
parent::tearDown();
45+
46+
@unlink($this->dumpPath);
47+
}
48+
2149
/**
2250
* @expectedException \LogicException
2351
*/
@@ -36,6 +64,23 @@ public function testDumpWhenSchemeIsUsedWithoutAProperDumper()
3664
$dumper->dump();
3765
}
3866

67+
public function testRedirectPreservesUrlEncoding()
68+
{
69+
$collection = new RouteCollection();
70+
$collection->add('foo', new Route('/foo:bar/'));
71+
72+
$class = $this->generateDumpedMatcher($collection, true);
73+
74+
$matcher = $this->getMockBuilder($class)
75+
->setMethods(array('redirect'))
76+
->setConstructorArgs(array(new RequestContext()))
77+
->getMock();
78+
79+
$matcher->expects($this->once())->method('redirect')->with('/foo%3Abar/', 'foo');
80+
81+
$matcher->match('/foo%3Abar');
82+
}
83+
3984
/**
4085
* @dataProvider getRouteCollections
4186
*/
@@ -285,4 +330,31 @@ public function getRouteCollections()
285330
array($rootprefixCollection, 'url_matcher3.php', array()),
286331
);
287332
}
333+
334+
/**
335+
* @param $dumper
336+
*/
337+
private function generateDumpedMatcher(RouteCollection $collection, $redirectableStub = false)
338+
{
339+
$options = array('class' => $this->matcherClass);
340+
341+
if ($redirectableStub) {
342+
$options['base_class'] = '\Symfony\Component\Routing\Tests\Matcher\Dumper\RedirectableUrlMatcherStub';
343+
}
344+
345+
$dumper = new PhpMatcherDumper($collection);
346+
$code = $dumper->dump($options);
347+
348+
file_put_contents($this->dumpPath, $code);
349+
include $this->dumpPath;
350+
351+
return $this->matcherClass;
352+
}
353+
}
354+
355+
abstract class RedirectableUrlMatcherStub extends UrlMatcher implements RedirectableUrlMatcherInterface
356+
{
357+
public function redirect($path, $route, $scheme = null)
358+
{
359+
}
288360
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,14 @@ public function testNoSchemaRedirectIfOnOfMultipleSchemesMatches()
6969
;
7070
$matcher->match('/foo');
7171
}
72+
73+
public function testRedirectPreservesUrlEncoding()
74+
{
75+
$coll = new RouteCollection();
76+
$coll->add('foo', new Route('/foo:bar/'));
77+
78+
$matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
79+
$matcher->expects($this->once())->method('redirect')->with('/foo%3Abar/');
80+
$matcher->match('/foo%3Abar');
81+
}
7282
}

0 commit comments

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