[Routing] fix Compiler when previous char of placeholder is no real s…

…eparator (closes #5423)

src/Symfony/Component/Routing/RouteCompiler.php

@@ -21,6 +21,13 @@ class RouteCompiler implements RouteCompilerInterface
 {
     const REGEX_DELIMITER = '#';
 
+    /**
+     * This string defines the characters that are automatically considered separators in front of
+     * optional placeholders (with default and no static text following). Such a single separator
+     * can be left out together with the optional placeholder from matching and generating URLs.
+     */
+    const SEPARATORS = '/,;.:-_~+*=@|';
+
     /**
      * {@inheritDoc}
      *
@@ -35,7 +42,7 @@ public function compile(Route $route)
         $variables = array();
         $matches = array();
         $pos = 0;
-        $lastSeparator = '';
+        $lastSeparator = '/';
 
         // Match all variables enclosed in "{}" and iterate over them. But we only want to match the innermost variable
         // in case of nested "{}", e.g. {foo{bar}}. This in ensured because \w does not match "{" or "}" itself.
@@ -46,6 +53,7 @@ public function compile(Route $route)
             $precedingText = substr($pattern, $pos, $match[0][1] - $pos);
             $pos = $match[0][1] + strlen($match[0][0]);
             $precedingChar = strlen($precedingText) > 0 ? substr($precedingText, -1) : '';
+            $isSeparator = '' !== $precedingChar && false !== strpos(static::SEPARATORS, $precedingChar);
 
             if (is_numeric($varName)) {
                 throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $varName, $pattern));
@@ -54,12 +62,15 @@ public function compile(Route $route)
                 throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName));
             }
 
-            if (strlen($precedingText) > 1) {
+            if ($isSeparator && strlen($precedingText) > 1) {
                 $tokens[] = array('text', substr($precedingText, 0, -1));
+            } elseif (!$isSeparator && strlen($precedingText) > 0) {
+                $tokens[] = array('text', $precedingText);
             }
+
             // use the character preceding the variable as a separator
             // save it for later use as default separator for variables that follow directly without having a preceding char e.g. "/{x}{y}"
-            if ('' !== $precedingChar) {
+            if ($isSeparator) {
                 $lastSeparator = $precedingChar;
             }
 
@@ -70,7 +81,7 @@ public function compile(Route $route)
                 $regexp = sprintf('[^%s]+', preg_quote($lastSeparator !== $nextSeparator ? $lastSeparator.$nextSeparator : $lastSeparator, self::REGEX_DELIMITER));
             }
 
-            $tokens[] = array('variable', $precedingChar, $regexp, $varName);
+            $tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName);
             $variables[] = $varName;
         }
 
@@ -108,7 +119,7 @@ public function compile(Route $route)
      *
      * @param string $pattern The route pattern
      *
-     * @return string The next static character (or empty string when none available)
+     * @return string The next static character that functions as separator (or empty string when none available)
      */
     private function findNextSeparator($pattern)
     {
@@ -119,7 +130,7 @@ private function findNextSeparator($pattern)
         // first remove all placeholders from the pattern so we can find the next real static character
         $pattern = preg_replace('#\{\w+\}#', '', $pattern);
 
-        return isset($pattern[0]) ? $pattern[0] : '';
+        return isset($pattern[0]) && false !== strpos(static::SEPARATORS, $pattern[0]) ? $pattern[0] : '';
     }
 
     /**

src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php

@@ -268,6 +268,23 @@ public function testAdjacentVariables()
         $generator->generate('test', array('x' => 'do.t', 'y' => '123', 'z' => 'bar', '_format' => 'xml'));
     }
 
+    public function testOptionalVariableWithNoRealSeparator()
+    {
+        $routes = $this->getRoutes('test', new Route('/get{what}', array('what' => 'All')));
+        $generator = $this->getGenerator($routes);
+
+        $this->assertSame('/app.php/get', $generator->generate('test'));
+        $this->assertSame('/app.php/getSites', $generator->generate('test', array('what' => 'Sites')));
+    }
+
+    public function testRequiredVariableWithNoRealSeparator()
+    {
+        $routes = $this->getRoutes('test', new Route('/get{what}Suffix'));
+        $generator = $this->getGenerator($routes);
+
+        $this->assertSame('/app.php/getSitesSuffix', $generator->generate('test', array('what' => 'Sites')));
+    }
+
     protected function getGenerator(RouteCollection $routes, array $parameters = array(), $logger = null)
     {
         $context = new RequestContext('/app.php');

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

@@ -252,6 +252,30 @@ public function testAdjacentVariables()
         $matcher->match('/wxy.html');
     }
 
+    public function testOptionalVariableWithNoRealSeparator()
+    {
+        $coll = new RouteCollection();
+        $coll->add('test', new Route('/get{what}', array('what' => 'All')));
+        $matcher = new UrlMatcher($coll, new RequestContext());
+
+        $this->assertEquals(array('what' => 'All', '_route' => 'test'), $matcher->match('/get'));
+        $this->assertEquals(array('what' => 'Sites', '_route' => 'test'), $matcher->match('/getSites'));
+
+        // Usually the character in front of an optional parameter can be left out, e.g. with pattern '/get/{what}' just '/get' would match.
+        // But here the 't' in 'get' is not a separating character, so it makes no sense to match without it.
+        $this->setExpectedException('Symfony\Component\Routing\Exception\ResourceNotFoundException');
+        $matcher->match('/ge');
+    }
+
+    public function testRequiredVariableWithNoRealSeparator()
+    {
+        $coll = new RouteCollection();
+        $coll->add('test', new Route('/get{what}Suffix'));
+        $matcher = new UrlMatcher($coll, new RequestContext());
+
+        $this->assertEquals(array('what' => 'Sites', '_route' => 'test'), $matcher->match('/getSitesSuffix'));
+    }
+
     /**
      * @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
      */

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

@@ -123,10 +123,10 @@ public function provideCompileData()
             array(
                 'Route with nested placeholders',
                 array('/{static{var}static}'),
-                '/{stati', '#^/\{static(?<var>[^cs]+)static\}$#s', array('var'), array(
+                '/{static', '#^/\{static(?<var>[^/]+)static\}$#s', array('var'), array(
                 array('text', 'static}'),
-                array('variable', 'c', '[^cs]+', 'var'),
-                array('text', '/{stati'),
+                array('variable', '', '[^/]+', 'var'),
+                array('text', '/{static'),
             )),
 
             array(