Skip to content

Commit

Permalink
feature #35857 [Routing] deprecate RouteCompiler::REGEX_DELIMITER (ni…
Browse files Browse the repository at this point in the history
…colas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Routing] deprecate RouteCompiler::REGEX_DELIMITER

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | -
| License       | MIT
| Doc PR        | -

This should be mostly internal cleanup.

Commits
-------

21b8a64 [Routing] deprecate RouteCompiler::REGEX_DELIMITER
  • Loading branch information
fabpot committed Feb 25, 2020
2 parents 28a249f + 21b8a64 commit 8867f57
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 53 deletions.
1 change: 1 addition & 0 deletions UPGRADE-5.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Routing

* Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
* Added argument `$priority` to `RouteCollection::add()`
* Deprecated the `RouteCompiler::REGEX_DELIMITER` constant

Yaml
----
Expand Down
1 change: 1 addition & 0 deletions UPGRADE-6.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ Routing

* Removed `RouteCollectionBuilder`.
* Added argument `$priority` to `RouteCollection::add()`
* Removed the `RouteCompiler::REGEX_DELIMITER` constant
1 change: 1 addition & 0 deletions src/Symfony/Component/Routing/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
* added "priority" option to annotated routes
* added argument `$priority` to `RouteCollection::add()`
* deprecated the `RouteCompiler::REGEX_DELIMITER` constant

5.0.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use Symfony\Component\Routing\Annotation\Route as RouteAnnotation;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCompiler;

/**
* AnnotationClassLoader loads routing information from a PHP class and its methods.
Expand Down Expand Up @@ -206,7 +205,7 @@ protected function addRoute(RouteCollection $collection, $annot, array $globals,
$this->configureRoute($route, $class, $method, $annot);
if (0 !== $locale) {
$route->setDefault('_locale', $locale);
$route->setRequirement('_locale', preg_quote($locale, RouteCompiler::REGEX_DELIMITER));
$route->setRequirement('_locale', preg_quote($locale));
$route->setDefault('_canonical_route', $name);
$collection->add($name.'.'.$locale, $route, $priority);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCompiler;

/**
* @internal
Expand Down Expand Up @@ -64,7 +63,7 @@ final protected function createLocalizedRoute(RouteCollection $collection, strin
$routes->add($name.'.'.$locale, $route = $this->createRoute($path));
$collection->add($namePrefix.$name.'.'.$locale, $route);
$route->setDefault('_locale', $locale);
$route->setRequirement('_locale', preg_quote($locale, RouteCompiler::REGEX_DELIMITER));
$route->setRequirement('_locale', preg_quote($locale));
$route->setDefault('_canonical_route', $namePrefix.$name);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
}

if ($requiredHost) {
if ('#' !== $requiredHost[0] ? $requiredHost !== $host : !preg_match($requiredHost, $host, $hostMatches)) {
if ('{' !== $requiredHost[0] ? $requiredHost !== $host : !preg_match($requiredHost, $host, $hostMatches)) {
continue;
}
if ('#' === $requiredHost[0] && $hostMatches) {
if ('{' === $requiredHost[0] && $hostMatches) {
$hostMatches['_route'] = $ret['_route'];
$ret = $this->mergeDefaults($hostMatches, $ret);
}
Expand Down
15 changes: 9 additions & 6 deletions src/Symfony/Component/Routing/RouteCompiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
*/
class RouteCompiler implements RouteCompilerInterface
{
/**
* @deprecated since Symfony 5.1, to be removed in 6.0
*/
const REGEX_DELIMITER = '#';

/**
Expand Down Expand Up @@ -161,8 +164,8 @@ private static function compilePattern(Route $route, string $pattern, bool $isHo
$nextSeparator = self::findNextSeparator($followingPattern, $useUtf8);
$regexp = sprintf(
'[^%s%s]+',
preg_quote($defaultSeparator, self::REGEX_DELIMITER),
$defaultSeparator !== $nextSeparator && '' !== $nextSeparator ? preg_quote($nextSeparator, self::REGEX_DELIMITER) : ''
preg_quote($defaultSeparator),
$defaultSeparator !== $nextSeparator && '' !== $nextSeparator ? preg_quote($nextSeparator) : ''
);
if (('' !== $nextSeparator && !preg_match('#^\{\w+\}#', $followingPattern)) || '' === $followingPattern) {
// When we have a separator, which is disallowed for the variable, we can optimize the regex with a possessive
Expand Down Expand Up @@ -217,7 +220,7 @@ private static function compilePattern(Route $route, string $pattern, bool $isHo
for ($i = 0, $nbToken = \count($tokens); $i < $nbToken; ++$i) {
$regexp .= self::computeRegexp($tokens, $i, $firstOptional);
}
$regexp = self::REGEX_DELIMITER.'^'.$regexp.'$'.self::REGEX_DELIMITER.'sD'.($isHost ? 'i' : '');
$regexp = '{^'.$regexp.'$}sD'.($isHost ? 'i' : '');

// enable Utf8 matching if really required
if ($needsUtf8) {
Expand Down Expand Up @@ -289,14 +292,14 @@ private static function computeRegexp(array $tokens, int $index, int $firstOptio
$token = $tokens[$index];
if ('text' === $token[0]) {
// Text tokens
return preg_quote($token[1], self::REGEX_DELIMITER);
return preg_quote($token[1]);
} else {
// Variable tokens
if (0 === $index && 0 === $firstOptional) {
// When the only token is an optional variable token, the separator is required
return sprintf('%s(?P<%s>%s)?', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]);
return sprintf('%s(?P<%s>%s)?', preg_quote($token[1]), $token[3], $token[2]);
} else {
$regexp = sprintf('%s(?P<%s>%s)', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]);
$regexp = sprintf('%s(?P<%s>%s)', preg_quote($token[1]), $token[3], $token[2]);
if ($index >= $firstOptional) {
// Enclose each optional token in a subpattern to make it optional.
// "?:" means it is non-capturing, i.e. the portion of the subject string that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
'/c2/route3' => [[['_route' => 'route3'], 'b.example.com', null, null, false, false, null]],
'/route5' => [[['_route' => 'route5'], 'c.example.com', null, null, false, false, null]],
'/route6' => [[['_route' => 'route6'], null, null, null, false, false, null]],
'/route11' => [[['_route' => 'route11'], '#^(?P<var1>[^\\.]++)\\.example\\.com$#sDi', null, null, false, false, null]],
'/route12' => [[['_route' => 'route12', 'var1' => 'val'], '#^(?P<var1>[^\\.]++)\\.example\\.com$#sDi', null, null, false, false, null]],
'/route11' => [[['_route' => 'route11'], '{^(?P<var1>[^\\.]++)\\.example\\.com$}sDi', null, null, false, false, null]],
'/route12' => [[['_route' => 'route12', 'var1' => 'val'], '{^(?P<var1>[^\\.]++)\\.example\\.com$}sDi', null, null, false, false, null]],
'/route17' => [[['_route' => 'route17'], null, null, null, false, false, null]],
],
[ // $regexpList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
'/c2/route3' => [[['_route' => 'route3'], 'b.example.com', null, null, false, false, null]],
'/route5' => [[['_route' => 'route5'], 'c.example.com', null, null, false, false, null]],
'/route6' => [[['_route' => 'route6'], null, null, null, false, false, null]],
'/route11' => [[['_route' => 'route11'], '#^(?P<var1>[^\\.]++)\\.example\\.com$#sDi', null, null, false, false, null]],
'/route12' => [[['_route' => 'route12', 'var1' => 'val'], '#^(?P<var1>[^\\.]++)\\.example\\.com$#sDi', null, null, false, false, null]],
'/route11' => [[['_route' => 'route11'], '{^(?P<var1>[^\\.]++)\\.example\\.com$}sDi', null, null, false, false, null]],
'/route12' => [[['_route' => 'route12', 'var1' => 'val'], '{^(?P<var1>[^\\.]++)\\.example\\.com$}sDi', null, null, false, false, null]],
'/route17' => [[['_route' => 'route17'], null, null, null, false, false, null]],
'/secure' => [[['_route' => 'secure'], null, null, ['https' => 0], false, false, null]],
'/nonsecure' => [[['_route' => 'nonsecure'], null, null, ['http' => 0], false, false, null]],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
true, // $matchHost
[ // $staticRoutes
'/' => [
[['_route' => 'a'], '#^(?P<d>[^\\.]++)\\.e\\.c\\.b\\.a$#sDi', null, null, false, false, null],
[['_route' => 'c'], '#^(?P<e>[^\\.]++)\\.e\\.c\\.b\\.a$#sDi', null, null, false, false, null],
[['_route' => 'a'], '{^(?P<d>[^\\.]++)\\.e\\.c\\.b\\.a$}sDi', null, null, false, false, null],
[['_route' => 'c'], '{^(?P<e>[^\\.]++)\\.e\\.c\\.b\\.a$}sDi', null, null, false, false, null],
[['_route' => 'b'], 'd.c.b.a', null, null, false, false, null],
],
],
Expand Down
Loading

0 comments on commit 8867f57

Please sign in to comment.