Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 73fbd08

Browse filesBrowse files
fancywebnicolas-grekas
authored andcommitted
[Routing] Fail properly when a route parameter name cannot be used as a PCRE subpattern name
1 parent ed11f22 commit 73fbd08
Copy full SHA for 73fbd08

File tree

2 files changed

+33
-7
lines changed
Filter options

2 files changed

+33
-7
lines changed

‎src/Symfony/Component/Routing/RouteCompiler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/RouteCompiler.php
+20-4Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,22 @@ class RouteCompiler implements RouteCompilerInterface
2828
*/
2929
const SEPARATORS = '/,;.:-_~+*=@|';
3030

31+
/**
32+
* The maximum supported length of a PCRE subpattern name
33+
* http://pcre.org/current/doc/html/pcre2pattern.html#SEC16.
34+
*
35+
* @var int
36+
*
37+
* @internal
38+
*/
39+
const VARIABLE_MAXIMUM_LENGTH = 32;
40+
3141
/**
3242
* {@inheritdoc}
3343
*
3444
* @throws \LogicException If a variable is referenced more than once
35-
* @throws \DomainException If a variable name is numeric because PHP raises an error for such
36-
* subpatterns in PCRE and thus would break matching, e.g. "(?P<123>.+)".
45+
* @throws \DomainException If a variable name starts with a digit or if it is too long to be successfully used as
46+
* a PCRE subpattern.
3747
*/
3848
public static function compile(Route $route)
3949
{
@@ -95,13 +105,19 @@ private static function compilePattern(Route $route, $pattern, $isHost)
95105
$precedingChar = strlen($precedingText) > 0 ? substr($precedingText, -1) : '';
96106
$isSeparator = '' !== $precedingChar && false !== strpos(static::SEPARATORS, $precedingChar);
97107

98-
if (is_numeric($varName)) {
99-
throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $varName, $pattern));
108+
// A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start with a digit so the
109+
// variable would not be usable as a Controller action argument.
110+
if (preg_match('/^\d/', $varName)) {
111+
throw new \DomainException(sprintf('Variable name "%s" cannot start with a digit in route pattern "%s". Please use a different name.', $varName, $pattern));
100112
}
101113
if (in_array($varName, $variables)) {
102114
throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName));
103115
}
104116

117+
if (strlen($varName) > self::VARIABLE_MAXIMUM_LENGTH) {
118+
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));
119+
}
120+
105121
if ($isSeparator && strlen($precedingText) > 1) {
106122
$tokens[] = array('text', substr($precedingText, 0, -1));
107123
} elseif (!$isSeparator && strlen($precedingText) > 0) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
+13-3Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Routing\Tests;
1313

1414
use Symfony\Component\Routing\Route;
15+
use Symfony\Component\Routing\RouteCompiler;
1516

1617
class RouteCompilerTest extends \PHPUnit_Framework_TestCase
1718
{
@@ -176,16 +177,16 @@ public function testRouteWithSameVariableTwice()
176177
}
177178

178179
/**
179-
* @dataProvider getNumericVariableNames
180+
* @dataProvider getVariableNamesStartingWithADigit
180181
* @expectedException \DomainException
181182
*/
182-
public function testRouteWithNumericVariableName($name)
183+
public function testRouteWithVariableNameStartingWithADigit($name)
183184
{
184185
$route = new Route('/{'.$name.'}');
185186
$route->compile();
186187
}
187188

188-
public function getNumericVariableNames()
189+
public function getVariableNamesStartingWithADigit()
189190
{
190191
return array(
191192
array('09'),
@@ -264,4 +265,13 @@ public function provideCompileWithHostData()
264265
),
265266
);
266267
}
268+
269+
/**
270+
* @expectedException \DomainException
271+
*/
272+
public function testRouteWithTooLongVariableName()
273+
{
274+
$route = new Route(sprintf('/{%s}', str_repeat('a', RouteCompiler::VARIABLE_MAXIMUM_LENGTH + 1)));
275+
$route->compile();
276+
}
267277
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.