Skip to content

Commit

Permalink
bug #35735 [Routing] Add locale requirement for localized routes (mta…
Browse files Browse the repository at this point in the history
…rld)

This PR was merged into the 4.4 branch.

Discussion
----------

[Routing] Add locale requirement for localized routes

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| License       | MIT

4.4 version of #35692

If you're using localized routes, you expect to have these kind of routes available:
- `/fr/accueil`
- `/en/home`

But nowadays, these routes are unexpectedly available:
- `/en/accueil`
- `/fr/home`

When importing routes like that:
- `prefix: "/{_locale}"`
- `@Route({"en": "/home", "fr": "/accueil"}, name="home")`

This PR proposes to add a strict locale requirement for localized so that the above routes won't be available.

Commits
-------

50d7445 [Routing] Add locale requirement for localized routes
  • Loading branch information
fabpot committed Feb 20, 2020
2 parents 12f1c7c + 50d7445 commit 88b89c9
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 5 deletions.
Expand Up @@ -18,6 +18,7 @@
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 @@ -211,6 +212,7 @@ protected function addRoute(RouteCollection $collection, $annot, $globals, \Refl
$this->configureRoute($route, $class, $method, $annot);
if (0 !== $locale) {
$route->setDefault('_locale', $locale);
$route->setRequirement('_locale', preg_quote($locale, RouteCompiler::REGEX_DELIMITER));
$route->setDefault('_canonical_route', $name);
$collection->add($name.'.'.$locale, $route);
} else {
Expand Down
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Routing\Loader\Configurator\RouteConfigurator;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCompiler;

trait AddTrait
{
Expand Down Expand Up @@ -67,6 +68,7 @@ final public function add(string $name, $path): RouteConfigurator
$routes->add($name.'.'.$locale, $route = $this->createRoute($path));
$this->collection->add($this->name.$name.'.'.$locale, $route);
$route->setDefault('_locale', $locale);
$route->setRequirement('_locale', preg_quote($locale, RouteCompiler::REGEX_DELIMITER));
$route->setDefault('_canonical_route', $this->name.$name);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Routing/Loader/XmlFileLoader.php
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\Config\Util\XmlUtils;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCompiler;

/**
* XmlFileLoader loads XML routing files.
Expand Down Expand Up @@ -129,6 +130,7 @@ protected function parseRoute(RouteCollection $collection, \DOMElement $node, $p
foreach ($paths as $locale => $p) {
$defaults['_locale'] = $locale;
$defaults['_canonical_route'] = $id;
$requirements['_locale'] = preg_quote($locale, RouteCompiler::REGEX_DELIMITER);
$route = new Route($p, $defaults, $requirements, $options, $node->getAttribute('host'), $schemes, $methods, $condition);
$collection->add($id.'.'.$locale, $route);
}
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Routing/Loader/YamlFileLoader.php
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCompiler;
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Parser as YamlParser;
use Symfony\Component\Yaml\Yaml;
Expand Down Expand Up @@ -140,6 +141,7 @@ protected function parseRoute(RouteCollection $collection, $name, array $config,
foreach ($config['path'] as $locale => $path) {
$localizedRoute = clone $route;
$localizedRoute->setDefault('_locale', $locale);
$localizedRoute->setRequirement('_locale', preg_quote($locale, RouteCompiler::REGEX_DELIMITER));
$localizedRoute->setDefault('_canonical_route', $name);
$localizedRoute->setPath($path);
$collection->add($name.'.'.$locale, $localizedRoute);
Expand Down
Expand Up @@ -125,6 +125,9 @@ public function testLocalizedPathRoutes()
$this->assertCount(2, $routes);
$this->assertEquals('/path', $routes->get('action.en')->getPath());
$this->assertEquals('/pad', $routes->get('action.nl')->getPath());

$this->assertEquals('nl', $routes->get('action.nl')->getRequirement('_locale'));
$this->assertEquals('en', $routes->get('action.en')->getRequirement('_locale'));
}

public function testLocalizedPathRoutesWithExplicitPathPropety()
Expand Down
10 changes: 5 additions & 5 deletions src/Symfony/Component/Routing/Tests/Loader/PhpFileLoaderTest.php
Expand Up @@ -229,11 +229,11 @@ public function testRoutingI18nConfigurator()

$expectedCollection = new RouteCollection();

$expectedCollection->add('foo.en', (new Route('/glish/foo'))->setDefaults(['_locale' => 'en', '_canonical_route' => 'foo']));
$expectedCollection->add('bar.en', (new Route('/glish/bar'))->setDefaults(['_locale' => 'en', '_canonical_route' => 'bar']));
$expectedCollection->add('baz.en', (new Route('/baz'))->setDefaults(['_locale' => 'en', '_canonical_route' => 'baz']));
$expectedCollection->add('c_foo.fr', (new Route('/ench/pub/foo'))->setDefaults(['_locale' => 'fr', '_canonical_route' => 'c_foo']));
$expectedCollection->add('c_bar.fr', (new Route('/ench/pub/bar'))->setDefaults(['_locale' => 'fr', '_canonical_route' => 'c_bar']));
$expectedCollection->add('foo.en', (new Route('/glish/foo'))->setDefaults(['_locale' => 'en', '_canonical_route' => 'foo'])->setRequirement('_locale', 'en'));
$expectedCollection->add('bar.en', (new Route('/glish/bar'))->setDefaults(['_locale' => 'en', '_canonical_route' => 'bar'])->setRequirement('_locale', 'en'));
$expectedCollection->add('baz.en', (new Route('/baz'))->setDefaults(['_locale' => 'en', '_canonical_route' => 'baz'])->setRequirement('_locale', 'en'));
$expectedCollection->add('c_foo.fr', (new Route('/ench/pub/foo'))->setDefaults(['_locale' => 'fr', '_canonical_route' => 'c_foo'])->setRequirement('_locale', 'fr'));
$expectedCollection->add('c_bar.fr', (new Route('/ench/pub/bar'))->setDefaults(['_locale' => 'fr', '_canonical_route' => 'c_bar'])->setRequirement('_locale', 'fr'));

$expectedCollection->addResource(new FileResource(realpath(__DIR__.'/../Fixtures/php_dsl_sub_i18n.php')));
$expectedCollection->addResource(new FileResource(realpath(__DIR__.'/../Fixtures/php_dsl_i18n.php')));
Expand Down
Expand Up @@ -185,6 +185,9 @@ public function testLocalizedImports()

$this->assertEquals('/le-prefix/le-suffix', $routeCollection->get('imported.fr')->getPath());
$this->assertEquals('/the-prefix/suffix', $routeCollection->get('imported.en')->getPath());

$this->assertEquals('fr', $routeCollection->get('imported.fr')->getRequirement('_locale'));
$this->assertEquals('en', $routeCollection->get('imported.en')->getRequirement('_locale'));
}

public function testLocalizedImportsOfNotLocalizedRoutes()
Expand Down
Expand Up @@ -321,6 +321,9 @@ public function testImportingRoutesWithLocales()
$this->assertCount(2, $routes);
$this->assertEquals('/nl/voorbeeld', $routes->get('imported.nl')->getPath());
$this->assertEquals('/en/example', $routes->get('imported.en')->getPath());

$this->assertEquals('nl', $routes->get('imported.nl')->getRequirement('_locale'));
$this->assertEquals('en', $routes->get('imported.en')->getRequirement('_locale'));
}

public function testImportingNonLocalizedRoutesWithLocales()
Expand Down

0 comments on commit 88b89c9

Please sign in to comment.