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 098ab7b

Browse filesBrowse files
vudaltsovvudaltsov
committed
Made important parameters required when matching
1 parent 44aa362 commit 098ab7b
Copy full SHA for 098ab7b

File tree

5 files changed

+53
-49
lines changed
Filter options

5 files changed

+53
-49
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Generator/UrlGenerator.php
+2-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/RouteCompiler.php
+6-12Lines changed: 6 additions & 12 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,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)
183184
$regexp = self::transformCapturingGroupsToNonCapturings($regexp);
184185
}
185186

186-
$tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName);
187-
if ('!' === $varName[0]) {
188-
$varName = substr($varName, 1);
189-
}
187+
$tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName, false, $important);
190188
$variables[] = $varName;
191189
}
192190

@@ -199,7 +197,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)
199197
if (!$isHost) {
200198
for ($i = \count($tokens) - 1; $i >= 0; --$i) {
201199
$token = $tokens[$i];
202-
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
200+
if ('variable' === $token[0] && !$token[5] && $route->hasDefault($token[3])) {
203201
$firstOptional = $i;
204202
} else {
205203
break;
@@ -219,7 +217,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)
219217
$regexp .= 'u';
220218
for ($i = 0, $nbToken = \count($tokens); $i < $nbToken; ++$i) {
221219
if ('variable' === $tokens[$i][0]) {
222-
$tokens[$i][] = true;
220+
$tokens[$i][4] = true;
223221
}
224222
}
225223
}
@@ -286,10 +284,6 @@ private static function computeRegexp(array $tokens, int $index, int $firstOptio
286284
// Text tokens
287285
return preg_quote($token[1], self::REGEX_DELIMITER);
288286
} else {
289-
if ('variable' === $token[0] && '!' === $token[3][0]) {
290-
$token[3] = substr($token[3], 1);
291-
}
292-
293287
// Variable tokens
294288
if (0 === $index && 0 === $firstOptional) {
295289
// When the only token is an optional variable token, the separator is required

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,17 @@ public function testMatchImportantVariable()
186186
$this->assertEquals(array('_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}', array('_format' => 'xml')));
196+
197+
$this->getUrlMatcher($collection)->match('/index');
198+
}
199+
189200
/**
190201
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
191202
*/

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
+33-33Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function provideCompileData()
4747
'Route with a variable',
4848
array('/foo/{bar}'),
4949
'/foo', '#^/foo/(?P<bar>[^/]++)$#sD', array('bar'), array(
50-
array('variable', '/', '[^/]++', 'bar'),
50+
array('variable', '/', '[^/]++', 'bar', false, false),
5151
array('text', '/foo'),
5252
),
5353
),
@@ -56,7 +56,7 @@ public function provideCompileData()
5656
'Route with a variable that has a default value',
5757
array('/foo/{bar}', array('bar' => 'bar')),
5858
'/foo', '#^/foo(?:/(?P<bar>[^/]++))?$#sD', array('bar'), array(
59-
array('variable', '/', '[^/]++', 'bar'),
59+
array('variable', '/', '[^/]++', 'bar', false, false),
6060
array('text', '/foo'),
6161
),
6262
),
@@ -65,8 +65,8 @@ public function provideCompileData()
6565
'Route with several variables',
6666
array('/foo/{bar}/{foobar}'),
6767
'/foo', '#^/foo/(?P<bar>[^/]++)/(?P<foobar>[^/]++)$#sD', array('bar', 'foobar'), array(
68-
array('variable', '/', '[^/]++', 'foobar'),
69-
array('variable', '/', '[^/]++', 'bar'),
68+
array('variable', '/', '[^/]++', 'foobar', false, false),
69+
array('variable', '/', '[^/]++', 'bar', false, false),
7070
array('text', '/foo'),
7171
),
7272
),
@@ -75,8 +75,8 @@ public function provideCompileData()
7575
'Route with several variables that have default values',
7676
array('/foo/{bar}/{foobar}', array('bar' => 'bar', 'foobar' => '')),
7777
'/foo', '#^/foo(?:/(?P<bar>[^/]++)(?:/(?P<foobar>[^/]++))?)?$#sD', array('bar', 'foobar'), array(
78-
array('variable', '/', '[^/]++', 'foobar'),
79-
array('variable', '/', '[^/]++', 'bar'),
78+
array('variable', '/', '[^/]++', 'foobar', false, false),
79+
array('variable', '/', '[^/]++', 'bar', false, false),
8080
array('text', '/foo'),
8181
),
8282
),
@@ -85,8 +85,8 @@ public function provideCompileData()
8585
'Route with several variables but some of them have no default values',
8686
array('/foo/{bar}/{foobar}', array('bar' => 'bar')),
8787
'/foo', '#^/foo/(?P<bar>[^/]++)/(?P<foobar>[^/]++)$#sD', array('bar', 'foobar'), array(
88-
array('variable', '/', '[^/]++', 'foobar'),
89-
array('variable', '/', '[^/]++', 'bar'),
88+
array('variable', '/', '[^/]++', 'foobar', false, false),
89+
array('variable', '/', '[^/]++', 'bar', false, false),
9090
array('text', '/foo'),
9191
),
9292
),
@@ -95,40 +95,40 @@ public function provideCompileData()
9595
'Route with an optional variable as the first segment',
9696
array('/{bar}', array('bar' => 'bar')),
9797
'', '#^/(?P<bar>[^/]++)?$#sD', array('bar'), array(
98-
array('variable', '/', '[^/]++', 'bar'),
98+
array('variable', '/', '[^/]++', 'bar', false, false),
9999
),
100100
),
101101

102102
array(
103103
'Route with a requirement of 0',
104104
array('/{bar}', array('bar' => null), array('bar' => '0')),
105105
'', '#^/(?P<bar>0)?$#sD', array('bar'), array(
106-
array('variable', '/', '0', 'bar'),
106+
array('variable', '/', '0', 'bar', false, false),
107107
),
108108
),
109109

110110
array(
111111
'Route with an optional variable as the first segment with requirements',
112112
array('/{bar}', array('bar' => 'bar'), array('bar' => '(foo|bar)')),
113113
'', '#^/(?P<bar>(?:foo|bar))?$#sD', array('bar'), array(
114-
array('variable', '/', '(?:foo|bar)', 'bar'),
114+
array('variable', '/', '(?:foo|bar)', 'bar', false, false),
115115
),
116116
),
117117

118118
array(
119119
'Route with only optional variables',
120120
array('/{foo}/{bar}', array('foo' => 'foo', 'bar' => 'bar')),
121121
'', '#^/(?P<foo>[^/]++)?(?:/(?P<bar>[^/]++))?$#sD', array('foo', 'bar'), array(
122-
array('variable', '/', '[^/]++', 'bar'),
123-
array('variable', '/', '[^/]++', 'foo'),
122+
array('variable', '/', '[^/]++', 'bar', false, false),
123+
array('variable', '/', '[^/]++', 'foo', false, false),
124124
),
125125
),
126126

127127
array(
128128
'Route with a variable in last position',
129129
array('/foo-{bar}'),
130130
'/foo-', '#^/foo\-(?P<bar>[^/]++)$#sD', array('bar'), array(
131-
array('variable', '-', '[^/]++', 'bar'),
131+
array('variable', '-', '[^/]++', 'bar', false, false),
132132
array('text', '/foo'),
133133
),
134134
),
@@ -138,7 +138,7 @@ public function provideCompileData()
138138
array('/{static{var}static}'),
139139
'/{static', '#^/\{static(?P<var>[^/]+)static\}$#sD', array('var'), array(
140140
array('text', 'static}'),
141-
array('variable', '', '[^/]+', 'var'),
141+
array('variable', '', '[^/]+', 'var', false, false),
142142
array('text', '/{static'),
143143
),
144144
),
@@ -147,20 +147,20 @@ public function provideCompileData()
147147
'Route without separator between variables',
148148
array('/{w}{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => '(y|Y)')),
149149
'', '#^/(?P<w>[^/\.]+)(?P<x>[^/\.]+)(?P<y>(?:y|Y))(?:(?P<z>[^/\.]++)(?:\.(?P<_format>[^/]++))?)?$#sD', array('w', 'x', 'y', 'z', '_format'), array(
150-
array('variable', '.', '[^/]++', '_format'),
151-
array('variable', '', '[^/\.]++', 'z'),
152-
array('variable', '', '(?:y|Y)', 'y'),
153-
array('variable', '', '[^/\.]+', 'x'),
154-
array('variable', '/', '[^/\.]+', 'w'),
150+
array('variable', '.', '[^/]++', '_format', false, false),
151+
array('variable', '', '[^/\.]++', 'z', false, false),
152+
array('variable', '', '(?:y|Y)', 'y', false, false),
153+
array('variable', '', '[^/\.]+', 'x', false, false),
154+
array('variable', '/', '[^/\.]+', 'w', false, false),
155155
),
156156
),
157157

158158
array(
159159
'Route with a format',
160160
array('/foo/{bar}.{_format}'),
161161
'/foo', '#^/foo/(?P<bar>[^/\.]++)\.(?P<_format>[^/]++)$#sD', array('bar', '_format'), array(
162-
array('variable', '.', '[^/]++', '_format'),
163-
array('variable', '/', '[^/\.]++', 'bar'),
162+
array('variable', '.', '[^/]++', '_format', false, false),
163+
array('variable', '/', '[^/\.]++', 'bar', false, false),
164164
array('text', '/foo'),
165165
),
166166
),
@@ -177,7 +177,7 @@ public function provideCompileData()
177177
'Route with an explicit UTF-8 requirement',
178178
array('/{bar}', array('bar' => null), array('bar' => '.'), array('utf8' => true)),
179179
'', '#^/(?P<bar>.)?$#sDu', array('bar'), array(
180-
array('variable', '/', '.', 'bar', true),
180+
array('variable', '/', '.', 'bar', true, false),
181181
),
182182
),
183183
);
@@ -215,7 +215,7 @@ public function provideCompileImplicitUtf8Data()
215215
'Route with an implicit UTF-8 requirement',
216216
array('/{bar}', array('bar' => null), array('bar' => 'é')),
217217
'', '#^/(?P<bar>é)?$#sDu', array('bar'), array(
218-
array('variable', '/', 'é', 'bar', true),
218+
array('variable', '/', 'é', 'bar', true, false),
219219
),
220220
'requirements',
221221
),
@@ -224,7 +224,7 @@ public function provideCompileImplicitUtf8Data()
224224
'Route with a UTF-8 class requirement',
225225
array('/{bar}', array('bar' => null), array('bar' => '\pM')),
226226
'', '#^/(?P<bar>\pM)?$#sDu', array('bar'), array(
227-
array('variable', '/', '\pM', 'bar', true),
227+
array('variable', '/', '\pM', 'bar', true, false),
228228
),
229229
'requirements',
230230
),
@@ -233,8 +233,8 @@ public function provideCompileImplicitUtf8Data()
233233
'Route with a UTF-8 separator',
234234
array('/foo/{bar}§{_format}', array(), array(), array('compiler_class' => Utf8RouteCompiler::class)),
235235
'/foo', '#^/foo/(?P<bar>[^/§]++)§(?P<_format>[^/]++)$#sDu', array('bar', '_format'), array(
236-
array('variable', '§', '[^/]++', '_format', true),
237-
array('variable', '/', '[^/§]++', 'bar', true),
236+
array('variable', '§', '[^/]++', '_format', true, false),
237+
array('variable', '/', '[^/§]++', 'bar', true, false),
238238
array('text', '/foo'),
239239
),
240240
'patterns',
@@ -337,11 +337,11 @@ public function provideCompileWithHostData()
337337
'Route with host pattern and some variables',
338338
array('/hello/{name}', array(), array(), array(), 'www.example.{tld}'),
339339
'/hello', '#^/hello/(?P<name>[^/]++)$#sD', array('tld', 'name'), array('name'), array(
340-
array('variable', '/', '[^/]++', 'name'),
340+
array('variable', '/', '[^/]++', 'name', false, false),
341341
array('text', '/hello'),
342342
),
343343
'#^www\.example\.(?P<tld>[^\.]++)$#sDi', array('tld'), array(
344-
array('variable', '.', '[^\.]++', 'tld'),
344+
array('variable', '.', '[^\.]++', 'tld', false, false),
345345
array('text', 'www.example'),
346346
),
347347
),
@@ -352,9 +352,9 @@ public function provideCompileWithHostData()
352352
array('text', '/hello'),
353353
),
354354
'#^(?P<locale>[^\.]++)\.example\.(?P<tld>[^\.]++)$#sDi', array('locale', 'tld'), array(
355-
array('variable', '.', '[^\.]++', 'tld'),
355+
array('variable', '.', '[^\.]++', 'tld', false, false),
356356
array('text', '.example'),
357-
array('variable', '', '[^\.]++', 'locale'),
357+
array('variable', '', '[^\.]++', 'locale', false, false),
358358
),
359359
),
360360
array(
@@ -364,9 +364,9 @@ public function provideCompileWithHostData()
364364
array('text', '/hello'),
365365
),
366366
'#^(?P<locale>[^\.]++)\.example\.(?P<tld>[^\.]++)$#sDi', array('locale', 'tld'), array(
367-
array('variable', '.', '[^\.]++', 'tld'),
367+
array('variable', '.', '[^\.]++', 'tld', false, false),
368368
array('text', '.example'),
369-
array('variable', '', '[^\.]++', 'locale'),
369+
array('variable', '', '[^\.]++', 'locale', false, false),
370370
),
371371
),
372372
);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/RouteTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ public function testSerializeWhenCompiledWithClass()
261261
*/
262262
public function testSerializedRepresentationKeepsWorking()
263263
{
264-
$serialized = 'C:31:"Symfony\Component\Routing\Route":936:{a:8:{s:4:"path";s:13:"/prefix/{foo}";s:4:"host";s:20:"{locale}.example.net";s:8:"defaults";a:1:{s:3:"foo";s:7:"default";}s:12:"requirements";a:1:{s:3:"foo";s:3:"\d+";}s:7:"options";a:1:{s:14:"compiler_class";s:39:"Symfony\Component\Routing\RouteCompiler";}s:7:"schemes";a:0:{}s:7:"methods";a:0:{}s:8:"compiled";C:39:"Symfony\Component\Routing\CompiledRoute":571:{a:8:{s:4:"vars";a:2:{i:0;s:6:"locale";i:1;s:3:"foo";}s:11:"path_prefix";s:7:"/prefix";s:10:"path_regex";s:31:"#^/prefix(?:/(?P<foo>\d+))?$#sD";s:11:"path_tokens";a:2:{i:0;a:4:{i:0;s:8:"variable";i:1;s:1:"/";i:2;s:3:"\d+";i:3;s:3:"foo";}i:1;a:2:{i:0;s:4:"text";i:1;s:7:"/prefix";}}s:9:"path_vars";a:1:{i:0;s:3:"foo";}s:10:"host_regex";s:40:"#^(?P<locale>[^\.]++)\.example\.net$#sDi";s:11:"host_tokens";a:2:{i:0;a:2:{i:0;s:4:"text";i:1;s:12:".example.net";}i:1;a:4:{i:0;s:8:"variable";i:1;s:0:"";i:2;s:7:"[^\.]++";i:3;s:6:"locale";}}s:9:"host_vars";a:1:{i:0;s:6:"locale";}}}}}';
264+
$serialized = 'C:31:"Symfony\Component\Routing\Route":991:{a:9:{s:4:"path";s:13:"/prefix/{foo}";s:4:"host";s:20:"{locale}.example.net";s:8:"defaults";a:1:{s:3:"foo";s:7:"default";}s:12:"requirements";a:1:{s:3:"foo";s:3:"\d+";}s:7:"options";a:1:{s:14:"compiler_class";s:39:"Symfony\Component\Routing\RouteCompiler";}s:7:"schemes";a:0:{}s:7:"methods";a:0:{}s:9:"condition";s:0:"";s:8:"compiled";C:39:"Symfony\Component\Routing\CompiledRoute":603:{a:8:{s:4:"vars";a:2:{i:0;s:6:"locale";i:1;s:3:"foo";}s:11:"path_prefix";s:7:"/prefix";s:10:"path_regex";s:31:"#^/prefix(?:/(?P<foo>\d+))?$#sD";s:11:"path_tokens";a:2:{i:0;a:6:{i:0;s:8:"variable";i:1;s:1:"/";i:2;s:3:"\d+";i:3;s:3:"foo";i:4;b:0;i:5;b:0;}i:1;a:2:{i:0;s:4:"text";i:1;s:7:"/prefix";}}s:9:"path_vars";a:1:{i:0;s:3:"foo";}s:10:"host_regex";s:40:"#^(?P<locale>[^\.]++)\.example\.net$#sDi";s:11:"host_tokens";a:2:{i:0;a:2:{i:0;s:4:"text";i:1;s:12:".example.net";}i:1;a:6:{i:0;s:8:"variable";i:1;s:0:"";i:2;s:7:"[^\.]++";i:3;s:6:"locale";i:4;b:0;i:5;b:0;}}s:9:"host_vars";a:1:{i:0;s:6:"locale";}}}}}';
265265
$unserialized = unserialize($serialized);
266266

267267
$route = new Route('/prefix/{foo}', array('foo' => 'default'), array('foo' => '\d+'));

0 commit comments

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