merged branch Tobion/numeric-placeholder (PR symfony#5439)

fabpot · fabpot · commit 8b7ba9b1e5c6 · 2012-09-10T11:27:26.000+02:00
Commits ------- be28e56 [Routing] disallow numeric named variables in pattern Discussion ---------- [Routing] compile check for numeric named variables in pattern Because PHP raises an error for such subpatterns in PCRE and thus would break matching, e.g. this is not allowed as regex `(?<123>.+)`. So add a compile time check for a non-working pattern like '/{123}'. --------------------------------------------------------------------------- by sstok at 2012-09-06T08:31:42Z Strangely enough Regex buddy gives no warning or error with the pattern. Is the name all numeric invalid or just the beginning? 1e4 and 0xFF would be perfectly valid but returns true with is_nummeric() --------------------------------------------------------------------------- by Tobion at 2012-09-06T08:59:07Z Any numeric is not valid. I guess this limitation is unique to PHP's binding to PCRE. I think it's because the returned matches array of of preg_match contains both the subpattern as integer index and as named variable. So having a numeric named variable would conflict as `array['1'] === array[1]`.
diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php
@@ -23,7 +23,9 @@ class RouteCompiler implements RouteCompilerInterface
     /**
      * {@inheritDoc}
      *
-     * @throws \LogicException If a variable is referenced more than once
+     * @throws \LogicException  If a variable is referenced more than once
+     * @throws \DomainException If a variable name is numeric because PHP raises an error for such
+     *                          subpatterns in PCRE and thus would break matching, e.g. "(?<123>.+)".
      */
     public function compile(Route $route)
     {
@@ -56,6 +58,9 @@ public function compile(Route $route)
 
             $tokens[] = array('variable', $match[0][0][0], $regexp, $var);
 
+            if (is_numeric($var)) {
+                throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $var, $route->getPattern()));
+            }
             if (in_array($var, $variables)) {
                 throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $route->getPattern(), $var));
             }
diff --git a/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php b/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
@@ -133,4 +133,23 @@ public function testRouteWithSameVariableTwice()
 
         $compiled = $route->compile();
     }
+
+    /**
+     * @dataProvider getNumericVariableNames
+     * @expectedException \DomainException
+     */
+    public function testRouteWithNumericVariableName($name)
+    {
+        $route = new Route('/{'. $name . '}');
+        $route->compile();
+    }
+
+    public function getNumericVariableNames()
+    {
+        return array(
+           array('09'),
+           array('123'),
+           array('1e2')
+        );
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,9 @@ class RouteCompiler implements RouteCompilerInterface`
`23`	`23`	`/**`
`24`	`24`	`* {@inheritDoc}`
`25`	`25`	`*`
`26`		`- * @throws \LogicException If a variable is referenced more than once`
	`26`	`+ * @throws \LogicException If a variable is referenced more than once`
	`27`	`+ * @throws \DomainException If a variable name is numeric because PHP raises an error for such`
	`28`	`+ * subpatterns in PCRE and thus would break matching, e.g. "(?<123>.+)".`
`27`	`29`	`*/`
`28`	`30`	`public function compile(Route $route)`
`29`	`31`	`{`
`@@ -56,6 +58,9 @@ public function compile(Route $route)`
`56`	`58`
`57`	`59`	`$tokens[] = array('variable', $match[0][0][0], $regexp, $var);`
`58`	`60`
	`61`	`+ if (is_numeric($var)) {`
	`62`	`+ throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $var, $route->getPattern()));`
	`63`	`+ }`
`59`	`64`	`if (in_array($var, $variables)) {`
`60`	`65`	`throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $route->getPattern(), $var));`
`61`	`66`	`}`