symfony · mpdude · Dec 10, 2017 · Dec 11, 2017
diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
@@ -96,10 +96,10 @@ private function generateMatchMethod($supportsRedirections)
        $code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections), "\n");

        return <<<EOF
-    public function match(\$pathinfo)
+    public function match(\$rawPathinfo)
    {
        \$allow = array();
-        \$pathinfo = rawurldecode(\$pathinfo);
+        \$pathinfo = rawurldecode(\$rawPathinfo);
        \$context = \$this->context;
        \$request = \$this->request;

@@ -284,7 +284,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
        if ($hasTrailingSlash) {
            $code .= <<<EOF
            if (substr(\$pathinfo, -1) !== '/') {
-                return \$this->redirect(\$pathinfo.'/', '$name');
+                return \$this->redirect(\$rawPathinfo.'/', '$name');
            }


@@ -299,7 +299,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
            $code .= <<<EOF
            \$requiredSchemes = $schemes;
            if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
-                return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
+                return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes));
            }



diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
@@ -15,10 +15,10 @@ public function __construct(RequestContext $context)
        $this->context = $context;
    }

-    public function match($pathinfo)
+    public function match($rawPathinfo)
    {
        $allow = array();
-        $pathinfo = rawurldecode($pathinfo);
+        $pathinfo = rawurldecode($rawPathinfo);
        $context = $this->context;
        $request = $this->request;


diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
@@ -15,10 +15,10 @@ public function __construct(RequestContext $context)
        $this->context = $context;
    }

-    public function match($pathinfo)
+    public function match($rawPathinfo)
    {
        $allow = array();
-        $pathinfo = rawurldecode($pathinfo);
+        $pathinfo = rawurldecode($rawPathinfo);
        $context = $this->context;
        $request = $this->request;

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

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

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

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

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

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

diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php
@@ -15,10 +15,10 @@ public function __construct(RequestContext $context)
        $this->context = $context;
    }

-    public function match($pathinfo)
+    public function match($rawPathinfo)
    {
        $allow = array();
-        $pathinfo = rawurldecode($pathinfo);
+        $pathinfo = rawurldecode($rawPathinfo);
        $context = $this->context;
        $request = $this->request;


diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php
@@ -13,11 +13,39 @@

 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper;
+use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
+use Symfony\Component\Routing\Matcher\UrlMatcher;
+use Symfony\Component\Routing\RequestContext;
 use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;

 class PhpMatcherDumperTest extends TestCase
 {
+    /**
+     * @var string
+     */
+    private $matcherClass;
+
+    /**
+     * @var string
+     */
+    private $dumpPath;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->matcherClass = uniqid('ProjectUrlMatcher');
+        $this->dumpPath = sys_get_temp_dir().DIRECTORY_SEPARATOR.'php_matcher.'.$this->matcherClass.'.php';
+    }
+
+    protected function tearDown()
+    {
+        parent::tearDown();
+
+        @unlink($this->dumpPath);
+    }
+
    /**
     * @expectedException \LogicException
     */
@@ -36,6 +64,23 @@ public function testDumpWhenSchemeIsUsedWithoutAProperDumper()
        $dumper->dump();
    }

+    public function testRedirectPreservesUrlEncoding()
+    {
+        $collection = new RouteCollection();
+        $collection->add('foo', new Route('/foo:bar/'));
+
+        $class = $this->generateDumpedMatcher($collection, true);
+
+        $matcher = $this->getMockBuilder($class)
+                        ->setMethods(array('redirect'))
+                        ->setConstructorArgs(array(new RequestContext()))
+                        ->getMock();
+
+        $matcher->expects($this->once())->method('redirect')->with('/foo%3Abar/', 'foo');
+
+        $matcher->match('/foo%3Abar');
+    }
+
    /**
     * @dataProvider getRouteCollections
     */
@@ -285,4 +330,31 @@ public function getRouteCollections()
           array($rootprefixCollection, 'url_matcher3.php', array()),
        );
    }
+
+    /**
+     * @param $dumper
+     */
+    private function generateDumpedMatcher(RouteCollection $collection, $redirectableStub = false)
+    {
+        $options = array('class' => $this->matcherClass);
+
+        if ($redirectableStub) {
+            $options['base_class'] = '\Symfony\Component\Routing\Tests\Matcher\Dumper\RedirectableUrlMatcherStub';
+        }
+
+        $dumper = new PhpMatcherDumper($collection);
+        $code = $dumper->dump($options);
+
+        file_put_contents($this->dumpPath, $code);
+        include $this->dumpPath;
+
+        return $this->matcherClass;
+    }
+}
+
+abstract class RedirectableUrlMatcherStub extends UrlMatcher implements RedirectableUrlMatcherInterface
+{
+    public function redirect($path, $route, $scheme = null)
+    {
+    }
 }
diff --git a/src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php
@@ -69,4 +69,14 @@ public function testNoSchemaRedirectIfOnOfMultipleSchemesMatches()
        ;
        $matcher->match('/foo');
    }
+
+    public function testRedirectPreservesUrlEncoding()
+    {
+        $coll = new RouteCollection();
+        $coll->add('foo', new Route('/foo:bar/'));
+
+        $matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
+        $matcher->expects($this->once())->method('redirect')->with('/foo%3Abar/');
+        $matcher->match('/foo%3Abar');
+    }
 }