8000 bug #29770 [Routing] Make important parameters required when matching… · symfony/symfony@a25daa8 · GitHub
[go: up one dir, main page]
Skip to content

Commit a25daa8

Browse files
nicolas-grekasnicolas-grekas
committed
bug #29770 [Routing] Make important parameters required when matching (vudaltsov)
This PR was merged into the 4.3-dev branch. Discussion ---------- [Routing] Make important parameters required when matching | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29763 | License | MIT | Doc PR | n/a 1. This PR improves "important" route parameters implementation. Instead of considering `!slug` to be a variable name (which is not correct from my POV and leads to a lot of `'!' === $varName[0]` and `substr($varName, 1)` snippets), I took advantage of the `$token` array and used offset `[5]` for keeping boolean importance flag. This approach improved and simplified code. 1. This PR makes important parameters required when matching according to #29763 Commits ------- 2c3ab22 Made important parameters required when matching
2 parents 9876e3a + 2c3ab22 commit a25daa8

File tree

3 files changed

+25
-14
lines changed

3 files changed

+25
-14
lines changed

‎src/Symfony/Component/Routing/Generator/UrlGenerator.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,8 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
157157
foreach ($tokens as $token) {
158158
if ('variable' === $token[0]) {
159159
$varName = $token[3];
160-
if ($important = ('!' === $varName[0])) {
161-
$varName = substr($varName, 1);
162-
}
160+
// variable is not important by default
161+
$important = $token[5] ?? false;
163162

164163
if (!$optional || $important || !array_key_exists($varName, $defaults) || (null !== $mergedParams[$varName] && (string) $mergedParams[$varName] !== (string) $defaults[$varName])) {
165164
// check requirement

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,10 @@ private static function compilePattern(Route $route, $pattern, $isHost)
111111

112112
// Match all variables enclosed in "{}" and iterate over them. But we only want to match the innermost variable
113113
// in case of nested "{}", e.g. {foo{bar}}. This in ensured because \w does not match "{" or "}" itself.
114-
preg_match_all('#\{!?\w+\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
114+
preg_match_all('#\{(!)?(\w+)\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
115115
foreach ($matches as $match) {
116-
$varName = substr($match[0][0], 1, -1);
116+
$important = $match[1][1] >= 0;
117+
$varName = $match[2][0];
117118
// get all static text preceding the current variable
118119
$precedingText = substr($pattern, $pos, $match[0][1] - $pos);
119120
$pos = $match[0][1] + \strlen($match[0][0]);
@@ -183,10 +184,13 @@ private static function compilePattern(Route $route, $pattern, $isHost)
183184
$regexp = self::transformCapturingGroupsToNonCapturings($regexp);
184185
}
185186

186-
$tokens[] = ['variable', $isSeparator ? $precedingChar : '', $regexp, $varName];
187-
if ('!' === $varName[0]) {
188-
$varName = substr($varName, 1);
187+
if ($important) {
188+
$token = ['variable', $isSeparator ? $precedingChar : '', $regexp, $varName, false, true];
189+
} else {
190+
$token = ['variable', $isSeparator ? $precedingChar : '', $regexp, $varName];
189191
}
192+
193+
$tokens[] = $token;
190194
$variables[] = $varName;
191195
}
192196

@@ -199,7 +203,8 @@ private static function compilePattern(Route $route, $pattern, $isHost)
199203
if (!$isHost) {
200204
for ($i = \count($tokens) - 1; $i >= 0; --$i) {
201205
$token = $tokens[$i];
202-
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
206+
// variable is optional when it is not important and has a default value
207+
if ('variable' === $token[0] && !($token[5] ?? false) && $route->hasDefault($token[3])) {
203208
$firstOptional = $i;
204209
} else {
205210
break;
@@ -219,7 +224,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)
219224
$regexp .= 'u';
220225
for ($i = 0, $nbToken = \count($tokens); $i < $nbToken; ++$i) {
221226
if ('variable' === $tokens[$i][0]) {
222-
$tokens[$i][] = true;
227+
$tokens[$i][4] = true;
223228
}
224229
}
225230
}
@@ -286,10 +291,6 @@ private static function computeRegexp(array $tokens, int $index, int $firstOptio
286291
// Text tokens
287292
return preg_quote($token[1], self::REGEX_DELIMITER);
288293
} else {
289-
if ('variable' === $token[0] && '!' === $token[3][0]) {
290-
$token[3] = substr($token[3], 1);
291-
}
292-
293294
// Variable tokens
294295
if (0 === $index && 0 === $firstOptional) {
295296
// When the only token is an optional variable token, the separator is required

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,17 @@ public function testMatchImportantVariable()
186186
$this->assertEquals(['_route' => 'index', '_format' => 'xml'], $matcher->match('/index.xml'));
187187
}
188188

189+
/**
190+
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
191+
*/
192+
public function testShortPathDoesNotMatchImportantVariable()
193+
{
194+
$collection = new RouteCollection();
195+
$collection->add('index', new Route('/index.{!_format}', ['_format' => 'xml']));
196+
197+
$this->getUrlMatcher($collection)->match('/index');
198+
}
199+
189200
/**
190201
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
191202
*/

0 commit comments

Comments
 (0)
0