Skip to content

Commit

Permalink
bug #35928 [Routing] Prevent localized routes _locale default & requi…
Browse files Browse the repository at this point in the history
…rement from being overridden (fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[Routing] Prevent localized routes _locale default & requirement from being overridden

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

When we have configured a localized route, its default _locale and _locale requirement should not be modified to ensure it works as expected.

Commits
-------

096dc0a [Routing] Prevent localized routes _locale default & requirement from being overridden
  • Loading branch information
nicolas-grekas committed Mar 2, 2020
2 parents 4ab8774 + 096dc0a commit afdd507
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 0 deletions.
21 changes: 21 additions & 0 deletions src/Symfony/Component/Routing/Route.php
Expand Up @@ -376,6 +376,10 @@ public function setDefaults(array $defaults)
*/
public function addDefaults(array $defaults)
{
if (isset($defaults['_locale']) && $this->isLocalized()) {
unset($defaults['_locale']);
}

foreach ($defaults as $name => $default) {
$this->defaults[$name] = $default;
}
Expand Down Expand Up @@ -418,6 +422,10 @@ public function hasDefault($name)
*/
public function setDefault($name, $default)
{
if ('_locale' === $name && $this->isLocalized()) {
return $this;
}

$this->defaults[$name] = $default;
$this->compiled = null;

Expand Down Expand Up @@ -461,6 +469,10 @@ public function setRequirements(array $requirements)
*/
public function addRequirements(array $requirements)
{
if (isset($requirements['_locale']) && $this->isLocalized()) {
unset($requirements['_locale']);
}

foreach ($requirements as $key => $regex) {
$this->requirements[$key] = $this->sanitizeRequirement($key, $regex);
}
Expand Down Expand Up @@ -503,6 +515,10 @@ public function hasRequirement($key)
*/
public function setRequirement($key, $regex)
{
if ('_locale' === $key && $this->isLocalized()) {
return $this;
}

$this->requirements[$key] = $this->sanitizeRequirement($key, $regex);
$this->compiled = null;

Expand Down Expand Up @@ -577,4 +593,9 @@ private function sanitizeRequirement(string $key, $regex)

return $regex;
}

private function isLocalized(): bool
{
return isset($this->defaults['_locale']) && isset($this->defaults['_canonical_route']) && ($this->requirements['_locale'] ?? null) === preg_quote($this->defaults['_locale'], RouteCompiler::REGEX_DELIMITER);
}
}
61 changes: 61 additions & 0 deletions src/Symfony/Component/Routing/Tests/RouteTest.php
Expand Up @@ -271,4 +271,65 @@ public function testSerializedRepresentationKeepsWorking()
$this->assertEquals($route, $unserialized);
$this->assertNotSame($route, $unserialized);
}

/**
* @dataProvider provideNonLocalizedRoutes
*/
public function testLocaleDefaultWithNonLocalizedRoutes(Route $route)
{
$this->assertNotSame('fr', $route->getDefault('_locale'));
$route->setDefault('_locale', 'fr');
$this->assertSame('fr', $route->getDefault('_locale'));
}

/**
* @dataProvider provideLocalizedRoutes
*/
public function testLocaleDefaultWithLocalizedRoutes(Route $route)
{
$expected = $route->getDefault('_locale');
$this->assertIsString($expected);
$this->assertNotSame('fr', $expected);
$route->setDefault('_locale', 'fr');
$this->assertSame($expected, $route->getDefault('_locale'));
}

/**
* @dataProvider provideNonLocalizedRoutes
*/
public function testLocaleRequirementWithNonLocalizedRoutes(Route $route)
{
$this->assertNotSame('fr', $route->getRequirement('_locale'));
$route->setRequirement('_locale', 'fr');
$this->assertSame('fr', $route->getRequirement('_locale'));
}

/**
* @dataProvider provideLocalizedRoutes
*/
public function testLocaleRequirementWithLocalizedRoutes(Route $route)
{
$expected = $route->getRequirement('_locale');
$this->assertIsString($expected);
$this->assertNotSame('fr', $expected);
$route->setRequirement('_locale', 'fr');
$this->assertSame($expected, $route->getRequirement('_locale'));
}

public function provideNonLocalizedRoutes()
{
return [
[(new Route('/foo'))],
[(new Route('/foo'))->setDefault('_locale', 'en')],
[(new Route('/foo'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'foo')],
[(new Route('/foo'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'foo')->setRequirement('_locale', 'foobar')],
];
}

public function provideLocalizedRoutes()
{
return [
[(new Route('/foo'))->setDefault('_locale', 'en')->setDefault('_canonical_route', 'foo')->setRequirement('_locale', 'en')],
];
}
}

0 comments on commit afdd507

Please sign in to comment.