Skip to content

Commit bc30d61

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] fix matching host patterns, utf8 prefixes and non-capturing groups
1 parent 22b8756 commit bc30d61

File tree

11 files changed

+1185
-1134
lines changed

11 files changed

+1185
-1134
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,10 +376,10 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
376376
if ($hostRegex) {
377377
preg_match('#^.\^(.*)\$.[a-zA-Z]*$#', $hostRegex, $rx);
378378
$state->vars = array();
379-
$hostRegex = '(?i:'.preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]).')';
379+
$hostRegex = '(?i:'.preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]).')\.';
380380
$state->hostVars = $state->vars;
381381
} else {
382-
$hostRegex = '[^/]*+';
382+
$hostRegex = '(?:(?:[^.]*+\.)++)';
383383
$state->hostVars = array();
384384
}
385385
$state->mark += strlen($rx = ($prev ? ')' : '')."|{$hostRegex}(?");
@@ -406,6 +406,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
406406
$rx = ")$}{$modifiers}";
407407
$code .= "\n .'{$rx}',";
408408
$state->regex .= $rx;
409+
$state->markTail = 0;
409410

410411
// if the regex is too large, throw a signaling exception to recompute with smaller chunk size
411412
set_error_handler(function ($type, $message) { throw 0 === strpos($message, $this->signalingException->getMessage()) ? $this->signalingException : new \ErrorException($message); });
@@ -427,7 +428,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
427428
EOF;
428429
}
429430

430-
$matchedPathinfo = $matchHost ? '$host.$pathinfo' : '$pathinfo';
431+
$matchedPathinfo = $matchHost ? '$host.\'.\'.$pathinfo' : '$pathinfo';
431432
unset($state->getVars);
432433

433434
return <<<EOF

‎Matcher/Dumper/StaticPrefixCollection.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,9 @@ public function getRoutes(): array
6363
*
6464
* @param array|self $route
6565
*/
66-
public function addRoute(string $prefix, $route, string $staticPrefix = null)
66+
public function addRoute(string $prefix, $route)
6767
{
68-
if (null === $staticPrefix) {
69-
list($prefix, $staticPrefix) = $this->getCommonPrefix($prefix, $prefix);
70-
}
68+
list($prefix, $staticPrefix) = $this->getCommonPrefix($prefix, $prefix);
7169

7270
for ($i = \count($this->items) - 1; 0 <= $i; --$i) {
7371
$item = $this->items[$i];
@@ -102,7 +100,7 @@ public function addRoute(string $prefix, $route, string $staticPrefix = null)
102100

103101
if ($item instanceof self && $this->prefixes[$i] === $commonPrefix) {
104102
// the new route is a child of a previous one, let's nest it
105-
$item->addRoute($prefix, $route, $staticPrefix);
103+
$item->addRoute($prefix, $route);
106104
} else {
107105
// the new route and a previous one have a common prefix, let's merge them
108106
$child = new self($commonPrefix);
@@ -176,7 +174,7 @@ private function getCommonPrefix(string $prefix, string $anotherPrefix): array
176174
break;
177175
}
178176
$subPattern = substr($prefix, $i, $j - $i);
179-
if ($prefix !== $anotherPrefix && !preg_match('{(?<!'.$subPattern.')}', '')) {
177+
if ($prefix !== $anotherPrefix && !preg_match('/^\(\[[^\]]++\]\+\+\)$/', $subPattern) && !preg_match('{(?<!'.$subPattern.')}', '')) {
180178
// sub-patterns of variable length are not considered as common prefixes because their greediness would break in-order matching
181179
break;
182180
}
@@ -187,6 +185,12 @@ private function getCommonPrefix(string $prefix, string $anotherPrefix): array
187185
}
188186
}
189187
restore_error_handler();
188+
if ($i < $end && 0b10 === (\ord($prefix[$i]) >> 6) && preg_match('//u', $prefix.' '.$anotherPrefix)) {
189+
do {
190+
// Prevent cutting in the middle of an UTF-8 characters
191+
--$i;
192+
} while (0b10 === (\ord($prefix[$i]) >> 6));
193+
}
190194

191195
return array(substr($prefix, 0, $i), substr($prefix, 0, $staticLength ?? $i));
192196
}

‎RouteCompiler.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ private static function transformCapturingGroupsToNonCapturings(string $regexp):
321321
continue;
322322
}
323323
$regexp = substr_replace($regexp, '?:', $i, 0);
324-
$i += 2;
324+
++$i;
325325
}
326326

327327
return $regexp;

‎Tests/Fixtures/dumper/url_matcher1.php

Lines changed: 48 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -79,50 +79,50 @@ public function match($rawPathinfo)
7979
return $ret;
8080
}
8181

82-
$matchedPathinfo = $host.$pathinfo;
82+
$matchedPathinfo = $host.'.'.$pathinfo;
8383
$regexList = array(
8484
0 => '{^(?'
85-
.'|[^/]*+(?'
86-
.'|/foo/(baz|symfony)(*:34)'
85+
.'|(?:(?:[^.]*+\\.)++)(?'
86+
.'|/foo/(baz|symfony)(*:46)'
8787
.'|/bar(?'
88-
.'|/([^/]++)(*:57)'
89-
.'|head/([^/]++)(*:77)'
88+
.'|/([^/]++)(*:69)'
89+
.'|head/([^/]++)(*:89)'
9090
.')'
9191
.'|/test/([^/]++)/(?'
92-
.'|(*:103)'
92+
.'|(*:115)'
9393
.')'
94-
.'|/([\']+)(*:119)'
94+
.'|/([\']+)(*:131)'
9595
.'|/a/(?'
9696
.'|b\'b/([^/]++)(?'
97-
.'|(*:148)'
98-
.'|(*:156)'
97+
.'|(*:160)'
98+
.'|(*:168)'
9999
.')'
100-
.'|(.*)(*:169)'
100+
.'|(.*)(*:181)'
101101
.'|b\'b/([^/]++)(?'
102-
.'|(*:192)'
103-
.'|(*:200)'
102+
.'|(*:204)'
103+
.'|(*:212)'
104104
.')'
105105
.')'
106-
.'|/multi/hello(?:/([^/]++))?(*:236)'
106+
.'|/multi/hello(?:/([^/]++))?(*:248)'
107107
.'|/([^/]++)/b/([^/]++)(?'
108-
.'|(*:267)'
109-
.'|(*:275)'
108+
.'|(*:279)'
109+
.'|(*:287)'
110110
.')'
111-
.'|/aba/([^/]++)(*:297)'
112-
.')|(?i:([^\\.]++)\\.example\\.com)(?'
111+
.'|/aba/([^/]++)(*:309)'
112+
.')|(?i:([^\\.]++)\\.example\\.com)\\.(?'
113113
.'|/route1(?'
114-
.'|3/([^/]++)(*:357)'
115-
.'|4/([^/]++)(*:375)'
114+
.'|3/([^/]++)(*:371)'
115+
.'|4/([^/]++)(*:389)'
116116
.')'
117-
.')|(?i:c\\.example\\.com)(?'
118-
.'|/route15/([^/]++)(*:425)'
119-
.')|[^/]*+(?'
120-
.'|/route16/([^/]++)(*:460)'
117+
.')|(?i:c\\.example\\.com)\\.(?'
118+
.'|/route15/([^/]++)(*:441)'
119+
.')|(?:(?:[^.]*+\\.)++)(?'
120+
.'|/route16/([^/]++)(*:488)'
121121
.'|/a/(?'
122-
.'|a\\.\\.\\.(*:481)'
122+
.'|a\\.\\.\\.(*:509)'
123123
.'|b/(?'
124-
.'|([^/]++)(*:502)'
125-
.'|c/([^/]++)(*:520)'
124+
.'|([^/]++)(*:530)'
125+
.'|c/([^/]++)(*:548)'
126126
.')'
127127
.')'
128128
.')'
@@ -132,7 +132,7 @@ public function match($rawPathinfo)
132132
foreach ($regexList as $offset => $regex) {
133133
while (preg_match($regex, $matchedPathinfo, $matches)) {
134134
switch ($m = (int) $matches['MARK']) {
135-
case 103:
135+
case 115:
136136
$matches = array('foo' => $matches[1] ?? null);
137137

138138
// baz4
@@ -159,7 +159,7 @@ public function match($rawPathinfo)
159159
not_bazbaz6:
160160

161161
break;
162-
case 148:
162+
case 160:
163163
$matches = array('foo' => $matches[1] ?? null);
164164

165165
// foo1
@@ -173,14 +173,14 @@ public function match($rawPathinfo)
173173
not_foo1:
174174

175175
break;
176-
case 192:
176+
case 204:
177177
$matches = array('foo1' => $matches[1] ?? null);
178178

179179
// foo2
180180
return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());
181181

182182
break;
183-
case 267:
183+
case 279:
184184
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
185185

186186
// foo3
@@ -189,23 +189,23 @@ public function match($rawPathinfo)
189189
break;
190190
default:
191191
$routes = array(
192-
34 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
193-
57 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
194-
77 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
195-
119 => array(array('_route' => 'quoter'), array('quoter'), null, null),
196-
156 => array(array('_route' => 'bar1'), array('bar'), null, null),
197-
169 => array(array('_route' => 'overridden'), array('var'), null, null),
198-
200 => array(array('_route' => 'bar2'), array('bar1'), null, null),
199-
236 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
200-
275 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
201-
297 => array(array('_route' => 'foo4'), array('foo'), null, null),
202-
357 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
203-
375 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
204-
425 => array(array('_route' => 'route15'), array('name'), null, null),
205-
460 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
206-
481 => array(array('_route' => 'a'), array(), null, null),
207-
502 => array(array('_route' => 'b'), array('var'), null, null),
208-
520 => array(array('_route' => 'c'), array('var'), null, null),
192+
46 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
193+
69 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
194+
89 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
195+
131 => array(array('_route' => 'quoter'), array('quoter'), null, null),
196+
168 => array(array('_route' => 'bar1'), array('bar'), null, null),
197+
181 => array(array('_route' => 'overridden'), array('var'), null, null),
198+
212 => array(array('_route' => 'bar2'), array('bar1'), null, null),
199+
248 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
200+
287 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
201+
309 => array(array('_route' => 'foo4'), array('foo'), null, null),
202+
371 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
203+
389 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
204+
441 => array(array('_route' => 'route15'), array('name'), null, null),
205+
488 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
206+
509 => array(array('_route' => 'a'), array(), null, null),
207+
530 => array(array('_route' => 'b'), array('var'), null, null),
208+
548 => array(array('_route' => 'c'), array('var'), null, null),
209209
);
210210

211211
list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
@@ -231,7 +231,7 @@ public function match($rawPathinfo)
231231
return $ret;
232232
}
233233

234-
if (520 === $m) {
234+
if (548 === $m) {
235235
break;
236236
}
237237
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));

0 commit comments

Comments
 (0)