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 8c2a77b

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #20474 [Routing] Fail properly when a route parameter name cannot be used as a PCRE subpattern name (fancyweb)
This PR was squashed before being merged into the 2.7 branch (closes #20474). Discussion ---------- [Routing] Fail properly when a route parameter name cannot be used as a PCRE subpattern name | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#7139 This is a follow up PR to #20327. I also decided to improve the current `is_numeric()` check that is done by a truer one. The current limitation is that a PCRE subpattern name must not start with a digit which is different. Commits ------- 73fbd08 [Routing] Fail properly when a route parameter name cannot be used as a PCRE subpattern name
2 parents b550d7e + 73fbd08 commit 8c2a77b
Copy full SHA for 8c2a77b

File tree

Expand file treeCollapse file tree

2 files changed

+31
-7
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+31
-7
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/RouteCompiler.php
+18-4Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,20 @@ 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+
* @internal
36+
*/
37+
const VARIABLE_MAXIMUM_LENGTH = 32;
38+
3139
/**
3240
* {@inheritdoc}
3341
*
3442
* @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>.+)".
43+
* @throws \DomainException If a variable name starts with a digit or if it is too long to be successfully used as
44+
* a PCRE subpattern.
3745
*/
3846
public static function compile(Route $route)
3947
{
@@ -95,13 +103,19 @@ private static function compilePattern(Route $route, $pattern, $isHost)
95103
$precedingChar = strlen($precedingText) > 0 ? substr($precedingText, -1) : '';
96104
$isSeparator = '' !== $precedingChar && false !== strpos(static::SEPARATORS, $precedingChar);
97105

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));
106+
// A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start with a digit so the
107+
// variable would not be usable as a Controller action argument.
108+
if (preg_match('/^\d/', $varName)) {
109+
throw new \DomainException(sprintf('Variable name "%s" cannot start with a digit in route pattern "%s". Please use a different name.', $varName, $pattern));
100110
}
101111
if (in_array($varName, $variables)) {
102112
throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName));
103113
}
104114

115+
if (strlen($varName) > self::VARIABLE_MAXIMUM_LENGTH) {
116+
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));
117+
}
118+
105119
if ($isSeparator && strlen($precedingText) > 1) {
106120
$tokens[] = array('text', substr($precedingText, 0, -1));
107121
} 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.