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

[Routing] fix matching host patterns, utf8 prefixes and non-capturing groups #27511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 10, 2018
Merged

[Routing] fix matching host patterns, utf8 prefixes and non-capturing groups #27511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions 7 src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,10 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
if ($hostRegex) {
preg_match('#^.\^(.*)\$.[a-zA-Z]*$#', $hostRegex, $rx);
$state->vars = array();
$hostRegex = '(?i:'.preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]).')';
$hostRegex = '(?i:'.preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]).')\.';
$state->hostVars = $state->vars;
} else {
$hostRegex = '[^/]*+';
$hostRegex = '(?:(?:[^.]*+\.)++)';
$state->hostVars = array();
}
$state->mark += strlen($rx = ($prev ? ')' : '')."|{$hostRegex}(?");
Expand All @@ -406,6 +406,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
$rx = ")$}{$modifiers}";
$code .= "\n .'{$rx}',";
$state->regex .= $rx;
$state->markTail = 0;

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

$matchedPathinfo = $matchHost ? '$host.$pathinfo' : '$pathinfo';
$matchedPathinfo = $matchHost ? '$host.\'.\'.$pathinfo' : '$pathinfo';
unset($state->getVars);

return <<<EOF
Expand Down
16 changes: 10 additions & 6 deletions 16 src/Symfony/Component/Routing/Matcher/Dumper/StaticPrefixCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,9 @@ public function getRoutes(): array
*
* @param array|self $route
*/
public function addRoute(string $prefix, $route, string $staticPrefix = null)
public function addRoute(string $prefix, $route)
{
if (null === $staticPrefix) {
list($prefix, $staticPrefix) = $this->getCommonPrefix($prefix, $prefix);
}
list($prefix, $staticPrefix) = $this->getCommonPrefix($prefix, $prefix);

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

if ($item instanceof self && $this->prefixes[$i] === $commonPrefix) {
// the new route is a child of a previous one, let's nest it
$item->addRoute($prefix, $route, $staticPrefix);
$item->addRoute($prefix, $route);
} else {
// the new route and a previous one have a common prefix, let's merge them
$child = new self($commonPrefix);
Expand Down Expand Up @@ -176,7 +174,7 @@ private function getCommonPrefix(string $prefix, string $anotherPrefix): array
break;
}
$subPattern = substr($prefix, $i, $j - $i);
if ($prefix !== $anotherPrefix && !preg_match('{(?<!'.$subPattern.')}', '')) {
if ($prefix !== $anotherPrefix && !preg_match('/^\(\[[^\]]++\]\+\+\)$/', $subPattern) && !preg_match('{(?<!'.$subPattern.')}', '')) {
// sub-patterns of variable length are not considered as common prefixes because their greediness would break in-order matching
break;
}
Expand All @@ -187,6 +185,12 @@ private function getCommonPrefix(string $prefix, string $anotherPrefix): array
}
}
restore_error_handler();
if ($i < $end && 0b10 === (\ord($prefix[$i]) >> 6) && preg_match('//u', $prefix.' '.$anotherPrefix)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a comment here explaining what that snippet of code do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment added

do {
// Prevent cutting in the middle of an UTF-8 characters
--$i;
} while (0b10 === (\ord($prefix[$i]) >> 6));
}

return array(substr($prefix, 0, $i), substr($prefix, 0, $staticLength ?? $i));
}
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Routing/RouteCompiler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ private static function transformCapturingGroupsToNonCapturings(string $regexp):
continue;
}
$regexp = substr_replace($regexp, '?:', $i, 0);
$i += 2;
++$i;
}

return $regexp;
Expand Down
96 changes: 48 additions & 48 deletions 96 src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,50 +79,50 @@ public function match($rawPathinfo)
return $ret;
}

$matchedPathinfo = $host.$pathinfo;
$matchedPathinfo = $host.'.'.$pathinfo;
$regexList = array(
0 => '{^(?'
.'|[^/]*+(?'
.'|/foo/(baz|symfony)(*:34)'
.'|(?:(?:[^.]*+\\.)++)(?'
.'|/foo/(baz|symfony)(*:46)'
.'|/bar(?'
.'|/([^/]++)(*:57)'
.'|head/([^/]++)(*:77)'
.'|/([^/]++)(*:69)'
.'|head/([^/]++)(*:89)'
.')'
.'|/test/([^/]++)/(?'
.'|(*:103)'
.'|(*:115)'
.')'
.'|/([\']+)(*:119)'
.'|/([\']+)(*:131)'
.'|/a/(?'
.'|b\'b/([^/]++)(?'
.'|(*:148)'
.'|(*:156)'
.'|(*:160)'
.'|(*:168)'
.')'
.'|(.*)(*:169)'
.'|(.*)(*:181)'
.'|b\'b/([^/]++)(?'
.'|(*:192)'
.'|(*:200)'
.'|(*:204)'
.'|(*:212)'
.')'
.')'
.'|/multi/hello(?:/([^/]++))?(*:236)'
.'|/multi/hello(?:/([^/]++))?(*:248)'
.'|/([^/]++)/b/([^/]++)(?'
.'|(*:267)'
.'|(*:275)'
.'|(*:279)'
.'|(*:287)'
.')'
.'|/aba/([^/]++)(*:297)'
.')|(?i:([^\\.]++)\\.example\\.com)(?'
.'|/aba/([^/]++)(*:309)'
.')|(?i:([^\\.]++)\\.example\\.com)\\.(?'
.'|/route1(?'
.'|3/([^/]++)(*:357)'
.'|4/([^/]++)(*:375)'
.'|3/([^/]++)(*:371)'
.'|4/([^/]++)(*:389)'
.')'
.')|(?i:c\\.example\\.com)(?'
.'|/route15/([^/]++)(*:425)'
.')|[^/]*+(?'
.'|/route16/([^/]++)(*:460)'
.')|(?i:c\\.example\\.com)\\.(?'
.'|/route15/([^/]++)(*:441)'
.')|(?:(?:[^.]*+\\.)++)(?'
.'|/route16/([^/]++)(*:488)'
.'|/a/(?'
.'|a\\.\\.\\.(*:481)'
.'|a\\.\\.\\.(*:509)'
.'|b/(?'
.'|([^/]++)(*:502)'
.'|c/([^/]++)(*:520)'
.'|([^/]++)(*:530)'
.'|c/([^/]++)(*:548)'
.')'
.')'
.')'
Expand All @@ -132,7 +132,7 @@ public function match($rawPathinfo)
foreach ($regexList as $offset => $regex) {
while (preg_match($regex, $matchedPathinfo, $matches)) {
switch ($m = (int) $matches['MARK']) {
case 103:
case 115:
$matches = array('foo' => $matches[1] ?? null);

// baz4
Expand All @@ -159,7 +159,7 @@ public function match($rawPathinfo)
not_bazbaz6:

break;
case 148:
case 160:
$matches = array('foo' => $matches[1] ?? null);

// foo1
Expand All @@ -173,14 +173,14 @@ public function match($rawPathinfo)
not_foo1:

break;
case 192:
case 204:
$matches = array('foo1' => $matches[1] ?? null);

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

break;
case 267:
case 279:
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);

// foo3
Expand All @@ -189,23 +189,23 @@ public function match($rawPathinfo)
break;
default:
$routes = array(
34 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
57 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
77 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
119 => array(array('_route' => 'quoter'), array('quoter'), null, null),
156 => array(array('_route' => 'bar1'), array('bar'), null, null),
169 => array(array('_route' => 'overridden'), array('var'), null, null),
200 => array(array('_route' => 'bar2'), array('bar1'), null, null),
236 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
275 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
297 => array(array('_route' => 'foo4'), array('foo'), null, null),
357 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
375 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
425 => array(array('_route' => 'route15'), array('name'), null, null),
460 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
481 => array(array('_route' => 'a'), array(), null, null),
502 => array(array('_route' => 'b'), array('var'), null, null),
520 => array(array('_route' => 'c'), array('var'), null, null),
46 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
69 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
89 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
131 => array(array('_route' => 'quoter'), array('quoter'), null, null),
168 => array(array('_route' => 'bar1'), array('bar'), null, null),
181 => array(array('_route' => 'overridden'), array('var'), null, null),
212 => array(array('_route' => 'bar2'), array('bar1'), null, null),
248 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
287 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
309 => array(array('_route' => 'foo4'), array('foo'), null, null),
371 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
389 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
441 => array(array('_route' => 'route15'), array('name'), null, null),
488 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
509 => array(array('_route' => 'a'), array(), null, null),
530 => array(array('_route' => 'b'), array('var'), null, null),
548 => array(array('_route' => 'c'), array('var'), null, null),
);

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

if (520 === $m) {
if (548 === $m) {
break;
}
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.