From 36b7055470b44dacfa10fe3c5ee4239d6ab383c0 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Thu, 10 Nov 2016 15:01:21 +0100 Subject: [PATCH 1/4] [Routing] Fail properly when a route parameter name cannot be used as a PCRE subpattern name --- .../Component/Routing/RouteCompiler.php | 22 +++++++++++++++---- .../Routing/Tests/RouteCompilerTest.php | 16 +++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php index f6637da666b8..9e489bfdafdb 100644 --- a/src/Symfony/Component/Routing/RouteCompiler.php +++ b/src/Symfony/Component/Routing/RouteCompiler.php @@ -28,12 +28,20 @@ class RouteCompiler implements RouteCompilerInterface */ const SEPARATORS = '/,;.:-_~+*=@|'; + /** + * The maximum supported length of a PCRE subpattern name + * http://pcre.org/current/doc/html/pcre2pattern.html#SEC16 + * + * @var int + */ + const VARIABLE_MAXIMUM_LENGTH = 32; + /** * {@inheritdoc} * * @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. "(?P<123>.+)". + * @throws \DomainException If a variable name starts with a digit or if it is too long to be successfully used as + * a PCRE subpattern. */ public static function compile(Route $route) { @@ -95,13 +103,19 @@ private static function compilePattern(Route $route, $pattern, $isHost) $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)); + // A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start a digit so the + // variable would not be usable as a Controller action argument. + if (preg_match('/^\d/', $varName)) { + throw new \DomainException(sprintf('Variable name "%s" cannot start with a digit in route pattern "%s". Please use a different name.', $varName, $pattern)); } if (in_array($varName, $variables)) { throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName)); } + if (strlen($varName) > self::VARIABLE_MAXIMUM_LENGTH) { + throw new \DomainException(sprintf('Variable name "%s" cannot be longer than %s characters in route pattern "%s". Please use a shorter name.', $varName, self::VARIABLE_MAXIMUM_LENGTH, $pattern)); + } + if ($isSeparator && strlen($precedingText) > 1) { $tokens[] = array('text', substr($precedingText, 0, -1)); } elseif (!$isSeparator && strlen($precedingText) > 0) { diff --git a/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php b/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php index b4b4f45a8379..f08b9688a921 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Routing\Tests; use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCompiler; class RouteCompilerTest extends \PHPUnit_Framework_TestCase { @@ -176,16 +177,16 @@ public function testRouteWithSameVariableTwice() } /** - * @dataProvider getNumericVariableNames + * @dataProvider getVariableNamesStartingWithADigit * @expectedException \DomainException */ - public function testRouteWithNumericVariableName($name) + public function testRouteWithVariableNameStartingWithADigit($name) { $route = new Route('/{'.$name.'}'); $route->compile(); } - public function getNumericVariableNames() + public function getVariableNamesStartingWithADigit() { return array( array('09'), @@ -264,4 +265,13 @@ public function provideCompileWithHostData() ), ); } + + /** + * @expectedException \DomainException + */ + public function testRouteWithTooLongVariableName() + { + $route = new Route(sprintf('/{%s}', str_repeat('a', RouteCompiler::VARIABLE_MAXIMUM_LENGTH + 1))); + $route->compile(); + } } From de9e977e2b7961c093c602b4c500dd41b8313678 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Thu, 10 Nov 2016 15:13:10 +0100 Subject: [PATCH 2/4] comment fix --- src/Symfony/Component/Routing/RouteCompiler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php index 9e489bfdafdb..c5ec0afa4f9f 100644 --- a/src/Symfony/Component/Routing/RouteCompiler.php +++ b/src/Symfony/Component/Routing/RouteCompiler.php @@ -103,7 +103,7 @@ private static function compilePattern(Route $route, $pattern, $isHost) $precedingChar = strlen($precedingText) > 0 ? substr($precedingText, -1) : ''; $isSeparator = '' !== $precedingChar && false !== strpos(static::SEPARATORS, $precedingChar); - // A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start a digit so the + // A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start with a digit so the // variable would not be usable as a Controller action argument. if (preg_match('/^\d/', $varName)) { throw new \DomainException(sprintf('Variable name "%s" cannot start with a digit in route pattern "%s". Please use a different name.', $varName, $pattern)); From ba96e2e35bd8a8c4be869ceb35228ad788a8991c Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Thu, 10 Nov 2016 15:36:42 +0100 Subject: [PATCH 3/4] cs fix --- src/Symfony/Component/Routing/RouteCompiler.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php index c5ec0afa4f9f..a7cb4fad18cd 100644 --- a/src/Symfony/Component/Routing/RouteCompiler.php +++ b/src/Symfony/Component/Routing/RouteCompiler.php @@ -30,7 +30,7 @@ class RouteCompiler implements RouteCompilerInterface /** * The maximum supported length of a PCRE subpattern name - * http://pcre.org/current/doc/html/pcre2pattern.html#SEC16 + * http://pcre.org/current/doc/html/pcre2pattern.html#SEC16. * * @var int */ @@ -41,7 +41,7 @@ class RouteCompiler implements RouteCompilerInterface * * @throws \LogicException If a variable is referenced more than once * @throws \DomainException If a variable name starts with a digit or if it is too long to be successfully used as - * a PCRE subpattern. + * a PCRE subpattern. */ public static function compile(Route $route) { From 55a8ed6ca4f295472da816affcbab4ed8a425073 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Fri, 25 Nov 2016 12:10:25 +0100 Subject: [PATCH 4/4] add internal phpdoc tag on the variable maximum length constant --- src/Symfony/Component/Routing/RouteCompiler.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php index a7cb4fad18cd..1ce18ceb6607 100644 --- a/src/Symfony/Component/Routing/RouteCompiler.php +++ b/src/Symfony/Component/Routing/RouteCompiler.php @@ -33,6 +33,8 @@ class RouteCompiler implements RouteCompilerInterface * http://pcre.org/current/doc/html/pcre2pattern.html#SEC16. * * @var int + * + * @internal */ const VARIABLE_MAXIMUM_LENGTH = 32;