[Routing] fixed regression in Routing matching algorithm

src/Symfony/Component/Routing/RouteCompiler.php

@@ -37,13 +37,17 @@ class RouteCompiler implements RouteCompilerInterface
             if ($text = substr($pattern, $pos, $match[0][1] - $pos)) {
                 $tokens[] = array('text', $text);
             }
+            $seps = array($pattern[$pos]);
             $pos = $match[0][1] + strlen($match[0][0]);
             $var = $match[1][0];
 
             if ($req = $route->getRequirement($var)) {
                 $regexp = $req;
             } else {
-                $regexp = $pos !== $len ? sprintf('[^%s]*?', $pattern[$pos]) : '.*?';
+                if ($pos !== $len) {
+                    $seps[] = $pattern[$pos];
+                }
+                $regexp = sprintf('[^%s]*?', preg_quote(implode('', array_unique($seps)), '#'));
             }
 
             $tokens[] = array('variable', $match[0][0][0], $regexp, $var);

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

@@ -7,10 +7,10 @@ RewriteCond %{REQUEST_URI} ^/foo/(baz|symfony)$
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_def:test]
 
 # bar
-RewriteCond %{REQUEST_URI} ^/bar/(.*?)$
+RewriteCond %{REQUEST_URI} ^/bar/([^/]*?)$
 RewriteCond %{REQUEST_METHOD} !^(get|head)$ [NC]
 RewriteRule .* - [S=1,E=_ROUTING__allow_get:1,E=_ROUTING__allow_head:1]
-RewriteCond %{REQUEST_URI} ^/bar/(.*?)$
+RewriteCond %{REQUEST_URI} ^/bar/([^/]*?)$
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:bar,E=_ROUTING_foo:%1]
 
 # baz

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

@@ -30,7 +30,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
         }
 
         // bar
-        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>.*?)$#x', $pathinfo, $matches)) {
+        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>[^/]*?)$#x', $pathinfo, $matches)) {
             if (!in_array($this->context->getMethod(), array('get', 'head'))) {
                 $allow = array_merge($allow, array('get', 'head'));
                 goto not_bar;

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

@@ -30,7 +30,7 @@ class ProjectUrlMatcher extends Symfony\Tests\Component\Routing\Fixtures\Redirec
         }
 
         // bar
-        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>.*?)$#x', $pathinfo, $matches)) {
+        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>[^/]*?)$#x', $pathinfo, $matches)) {
             if (!in_array($this->context->getMethod(), array('get', 'head'))) {
                 $allow = array_merge($allow, array('get', 'head'));
                 goto not_bar;

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

@@ -97,4 +97,14 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
         $matcher = new UrlMatcher($collection, new RequestContext('', 'head'), array());
         $this->assertInternalType('array', $matcher->match('/foo'));
     }
+
+    public function testMatchRegression()
+    {
+        $coll = new RouteCollection();
+        $coll->add('foo', new Route('/foo/{foo}'));
+        $coll->add('bar', new Route('/foo/bar/{foo}'));
+
+        $matcher = new UrlMatcher($coll, new RequestContext());
+        $this->assertEquals(array('foo' => 'bar', '_route' => 'bar'), $matcher->match('/foo/bar/bar'));
+    }
 }

tests/Symfony/Tests/Component/Routing/RouteCompilerTest.php

@@ -43,24 +43,24 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
             array(
                 'Route with a variable',
                 array('/foo/{bar}'),
-                '/foo', '#^/foo/(?P<bar>.*?)$#x', array('bar'), array(
-                    array('variable', '/', '.*?', 'bar'),
+                '/foo', '#^/foo/(?P<bar>[^/]*?)$#x', array('bar'), array(
+                    array('variable', '/', '[^/]*?', 'bar'),
                     array('text', '/foo'),
                 )),
 
             array(
                 'Route with a variable that has a default value',
                 array('/foo/{bar}', array('bar' => 'bar')),
-                '/foo', '#^/foo(?:/(?P<bar>.*?))?$#x', array('bar'), array(
-                    array('variable', '/', '.*?', 'bar'),
+                '/foo', '#^/foo(?:/(?P<bar>[^/]*?))?$#x', array('bar'), array(
+                    array('variable', '/', '[^/]*?', 'bar'),
                     array('text', '/foo'),
                 )),
 
             array(
                 'Route with several variables',
                 array('/foo/{bar}/{foobar}'),
-                '/foo', '#^/foo/(?P<bar>[^/]*?)/(?P<foobar>.*?)$#x', array('bar', 'foobar'), array(
-                    array('variable', '/', '.*?', 'foobar'),
+                '/foo', '#^/foo/(?P<bar>[^/]*?)/(?P<foobar>[^/]*?)$#x', array('bar', 'foobar'), array(
+                    array('variable', '/', '[^/]*?', 'foobar'),
                     array('variable', '/', '[^/]*?', 'bar'),
                     array('text', '/foo'),
                 )),
@@ -68,8 +68,8 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
             array(
                 'Route with several variables that have default values',
                 array('/foo/{bar}/{foobar}', array('bar' => 'bar', 'foobar' => '')),
-                '/foo', '#^/foo(?:/(?P<bar>[^/]*?)(?:/(?P<foobar>.*?))?)?$#x', array('bar', 'foobar'), array(
-                    array('variable', '/', '.*?', 'foobar'),
+                '/foo', '#^/foo(?:/(?P<bar>[^/]*?)(?:/(?P<foobar>[^/]*?))?)?$#x', array('bar', 'foobar'), array(
+                    array('variable', '/', '[^/]*?', 'foobar'),
                     array('variable', '/', '[^/]*?', 'bar'),
                     array('text', '/foo'),
                 )),
@@ -77,8 +77,8 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
             array(
                 'Route with several variables but some of them have no default values',
                 array('/foo/{bar}/{foobar}', array('bar' => 'bar')),
-                '/foo', '#^/foo/(?P<bar>[^/]*?)/(?P<foobar>.*?)$#x', array('bar', 'foobar'), array(
-                    array('variable', '/', '.*?', 'foobar'),
+                '/foo', '#^/foo/(?P<bar>[^/]*?)/(?P<foobar>[^/]*?)$#x', array('bar', 'foobar'), array(
+                    array('variable', '/', '[^/]*?', 'foobar'),
                     array('variable', '/', '[^/]*?', 'bar'),
                     array('text', '/foo'),
                 )),