[Routing] Adjust urlescaping rules, fixes #752

Only + and % are now encoded in generated routes, since they are the only characters that, if not encoded, could cause problems/conflicts when decoded. Namely + turns into a space, and % followed by numbers could do funky things.

The matcher decodes everything which works since nothing will have %NN without being escaped, and + are escaped as well.

src/Symfony/Component/Routing/Generator/UrlGenerator.php

@@ -125,8 +125,7 @@ class UrlGenerator implements UrlGeneratorInterface
                     }
 
                     if (!$isEmpty || !$optional) {
-                        // %2F is not valid in a URL, so we don't encode it (which is fine as the requirements explicitly allowed it)
-                        $url = $token[1].str_replace('%2F', '/', rawurlencode($tparams[$token[3]])).$url;
+                        $url = $token[1].strtr($tparams[$token[3]], array('%'=>'%25', '+'=>'%2B')).$url;
                     }
 
                     $optional = false;

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

@@ -61,6 +61,7 @@ class PhpMatcherDumper extends MatcherDumper
     public function match(\$pathinfo)
     {
         \$allow = array();
+        \$pathinfo = urldecode(\$pathinfo);
 
 $code
         throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();

src/Symfony/Component/Routing/Matcher/UrlMatcher.php

@@ -93,6 +93,8 @@ class UrlMatcher implements UrlMatcherInterface
 
     protected function matchCollection($pathinfo, RouteCollection $routes)
     {
+        $pathinfo = urldecode($pathinfo);
+
         foreach ($routes as $name => $route) {
             if ($route instanceof RouteCollection) {
                 if (false === strpos($route->getPrefix(), '{') && $route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) {

tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher1.php

@@ -23,6 +23,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
     public function match($pathinfo)
     {
         $allow = array();
+        $pathinfo = urldecode($pathinfo);
 
         // foo
         if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#x', $pathinfo, $matches)) {
@@ -106,38 +107,38 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
                     $matches['_route'] = 'foo';
                     return $matches;
                 }
-        
+
                 // bar
                 if (preg_match('#^/a/b/(?P<bar>[^/]+?)$#x', $pathinfo, $matches)) {
                     $matches['_route'] = 'bar';
                     return $matches;
                 }
-        
+
                 // foo1
                 if (preg_match('#^/a/b/(?P<foo1>[^/]+?)$#x', $pathinfo, $matches)) {
                     $matches['_route'] = 'foo1';
                     return $matches;
                 }
-        
+
                 // bar1
                 if (preg_match('#^/a/b/(?P<bar1>[^/]+?)$#x', $pathinfo, $matches)) {
                     $matches['_route'] = 'bar1';
                     return $matches;
                 }
-        
+
             }
-    
+
             // ababa
             if ($pathinfo === '/ababa') {
                 return array('_route' => 'ababa');
             }
-    
+
             // foo
             if (preg_match('#^/aba/(?P<foo>[^/]+?)$#x', $pathinfo, $matches)) {
                 $matches['_route'] = 'foo';
                 return $matches;
             }
-    
+
         }
 
         // foo

tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher2.php

@@ -23,6 +23,7 @@ class ProjectUrlMatcher extends Symfony\Tests\Component\Routing\Fixtures\Redirec
     public function match($pathinfo)
     {
         $allow = array();
+        $pathinfo = urldecode($pathinfo);
 
         // foo
         if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#x', $pathinfo, $matches)) {
@@ -118,38 +119,38 @@ class ProjectUrlMatcher extends Symfony\Tests\Component\Routing\Fixtures\Redirec
                     $matches['_route'] = 'foo';
                     return $matches;
                 }
-        
+
                 // bar
                 if (preg_match('#^/a/b/(?P<bar>[^/]+?)$#x', $pathinfo, $matches)) {
                     $matches['_route'] = 'bar';
                     return $matches;
                 }
-        
+
                 // foo1
                 if (preg_match('#^/a/b/(?P<foo1>[^/]+?)$#x', $pathinfo, $matches)) {
                     $matches['_route'] = 'foo1';
                     return $matches;
                 }
-        
+
                 // bar1
                 if (preg_match('#^/a/b/(?P<bar1>[^/]+?)$#x', $pathinfo, $matches)) {
                     $matches['_route'] = 'bar1';
                     return $matches;
                 }
-        
+
             }
-    
+
             // ababa
             if ($pathinfo === '/ababa') {
                 return array('_route' => 'ababa');
             }
-    
+
             // foo
             if (preg_match('#^/aba/(?P<foo>[^/]+?)$#x', $pathinfo, $matches)) {
                 $matches['_route'] = 'foo';
                 return $matches;
             }
-    
+
         }
 
         // foo

tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php

@@ -150,6 +150,17 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('_locale' => 'fr', '_route' => 'foo', 'foo' => 'foo'), $matcher->match('/fr/b/foo'));
     }
 
+    public function testMatchNonAlpha()
+    {
+        $collection = new RouteCollection();
+        $chars = '!"$%éà &\'()*+,./:;<=>@ABCDEFGHIJKLMNOPQRSTUVWXYZ\\[]^_`abcdefghijklmnopqrstuvwxyz{|}~-';
+        $collection->add('foo', new Route('/{foo}/bar', array(), array('foo' => '['.preg_quote($chars).']+')));
+
+        $matcher = new UrlMatcher($collection, new RequestContext(), array());
+        $this->assertEquals(array('_route' => 'foo', 'foo' => $chars), $matcher->match('/'.urlencode($chars).'/bar'));
+        $this->assertEquals(array('_route' => 'foo', 'foo' => $chars), $matcher->match('/'.strtr($chars, array('%' => '%25', '+' => '%2B')).'/bar'));
+    }
+
     public function testMatchRegression()
     {
         $coll = new RouteCollection();