8000 bug #26538 [Routing] remove capturing groups from requirements, they … · symfony/symfony@b79f29e · GitHub
[go: up one dir, main page]
Skip to content

Commit b79f29e

Browse files
nicolas-grekasnicolas-grekas
committed
bug #26538 [Routing] remove capturing groups from requirements, they break the merged regex (nicolas-grekas)
This PR was merged into the 4.1-dev branch. Discussion ---------- [Routing] remove capturing groups from requirements, they break the merged regex | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Group positions are now used to extract variables. Capturing groups in requirements break them for now. Commits ------- 8444022 [Routing] remove capturing groups from requirements, they break the merged regex
2 parents 0f9246f + 8444022 commit b79f29e

File tree

3 files changed

+55
-5
lines changed

3 files changed

+55
-5
lines changed

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)
180180
if (!$useUtf8 && $needsUtf8) {
181181
throw new \LogicException(sprintf('Cannot mix UTF-8 requirement with non-UTF-8 charset for variable "%s" in pattern "%s".', $varName, $pattern));
182182
}
183+
$regexp = self::transformCapturingGroupsToNonCapturings($regexp);
183< 8000 /code>184
}
184185

185186
$tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName);
@@ -247,7 +248,7 @@ private static function determineStaticPrefix(Route $route, array $tokens): stri
247248
}
248249

249250
/**
250-
* Returns the next static character in the Route pattern that will serve as a separator (or the empty string when none available)
251+
* Returns the next static character in the Route pattern that will serve as a separator (or the empty string when none available).
251252
*/
252253
private static function findNextSeparator(string $pattern, bool $useUtf8): string
253254
{
@@ -304,4 +305,25 @@ private static function computeRegexp(array $tokens, int $index, int $firstOptio
304305
}
305306
}
306307
}
308+
309+
private static function transformCapturingGroupsToNonCapturings(string $regexp): string
310+
{
311+
for ($i = 0; $i < \strlen($regexp); ++$i) {
312+
if ('\\' === $regexp[$i]) {
313+
++$i;
314+
continue;
315+
}
316+
if ('(' !== $regexp[$i] || !isset($regexp[$i + 2])) {
317+
continue;
318+
}
319+
if ('*' === $regexp[++$i] || '?' === $regexp[$i]) {
320+
++$i;
321+
continue;
322+
}
323+
$regexp = substr_replace($regexp, '?:', $i, 0);
324+
$i += 2;
325+
}
326+
327+
return $regexp;
328+
}
307329
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,15 @@ public function testUnicodeRoute()
587587
$this->assertEquals(array('_route' => 'b', 'a' => 'é'), $matcher->match('/é'));
588588
}
589589

590+
public function testRequirementWithCapturingGroup()
591+
{
592+
$coll = new RouteCollection();
593+
$coll->add('a', new Route('/{a}/{b}', array(), array('a' => '(a|b)')));
594+
595+
$matcher = $this->getUrlMatcher($coll);
596+
$this->assertEquals(array('_route' => 'a', 'a' => 'a', 'b' => 'b'), $matcher->match('/a/b'));
597+
}
598+
590599
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
591600
{
592601
return new UrlMatcher($routes, $context ?: new RequestContext());

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

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ public function provideCompileData()
110110
array(
111111
'Route with an optional variable as the first segment with requirements',
112112
array('/{bar}', array('bar' => 'bar'), array('bar' => '(foo|bar)')),
113-
'', '#^/(?P<bar>(foo|bar))?$#sD', array('bar'), array(
114-
array('variable', '/', '(foo|bar)', 'bar'),
113+
'', '#^/(?P<bar>(?:foo|bar))?$#sD', array('bar'), array(
114+
array('variable', '/', '(?:foo|bar)', 'bar'),
115115
),
116116
),
117117

@@ -146,10 +146,10 @@ public function provideCompileData()
146146
array(
147147
'Route without separator between variables',
148148
array('/{w}{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => '(y|Y)')),
149-
'', '#^/(?P<w>[^/\.]+)(?P<x>[^/\.]+)(?P<y>(y|Y))(?:(?P<z>[^/\.]++)(?:\.(?P<_format>[^/]++))?)?$#sD', array('w', 'x', 'y', 'z', '_format'), array(
149+
'', '#^/(?P<w>[^/\.]+)(?P<x>[^/\.]+)(?P<y>(?:y|Y))(?:(?P<z>[^/\.]++)(?:\.(?P<_format>[^/]++))?)?$#sD', array('w', 'x', 'y', 'z', '_format'), array(
150150
array('variable', '.', '[^/]++', '_format'),
151151
array('variable', '', '[^/\.]++', 'z'),
152-
array('variable', '', '(y|Y)', 'y'),
152+
array('variable', '', '(?:y|Y)', 'y'),
153153
array('variable', '', '[^/\.]+', 'x'),
154154
array('variable', '/', '[^/\.]+', 'w'),
155155
),
@@ -380,6 +380,25 @@ public function testRouteWithTooLongVariableName()
380380
$route = new Route(sprintf('/{%s}', str_repeat('a', RouteCompiler::VARIABLE_MAXIMUM_LENGTH + 1)));
381381
$route->compile();
382382
}
383+
384+
/**
385+
* @dataProvider provideRemoveCapturingGroup
386+
*/
387+
public function testRemoveCapturingGroup($regex, $requirement)
388+
{
389+
$route = new Route('/{foo}', array(), array('foo' => $requirement));
390+
391+
$this->assertSame($regex, $route->compile()->getRegex());
392+
}
393+
394+
public function provideRemoveCapturingGroup()
395+
{
396+
yield array('#^/(?P<foo>a(?:b|c)(?:d|e)f)$#sD', 'a(b|c)(d|e)f');
397+
yield array('#^/(?P<foo>a\(b\)c)$#sD', 'a\(b\)c');
398+
yield array('#^/(?P<foo>(?:b))$#sD', '(?:b)');
399+
yield array('#^/(?P<foo>(?(b)b))$#sD', '(?(b)b)');
400+
yield array('#^/(?P<foo>(*F))$#sD', '(*F)');
401+
}
383402
}
384403

385404
class Utf8RouteCompiler extends RouteCompiler

0 commit comments

Comments
 (0)
0