8000 CompiledUrlGenerator & UrlGenerator - Passes wrong locale as parameter · Issue #35915 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

CompiledUrlGenerator & UrlGenerator - Passes wrong locale as parameter #35915

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

Jump to bottom
Closed
christoph-pflueger opened this issue Mar 1, 2020 · 3 comments
Closed

CompiledUrlGenerator & UrlGenerator - Passes wrong locale as parameter #35915

christoph-pflueger opened this issue Mar 1, 2020 · 3 comments
Labels
Bug Routing Status: Reviewed

Comments

@christoph-pflueger
Copy link

Symfony version(s) affected: All

Description

When using internationalized routes with _locale as a prefix, the UrlGenerator creates routes with the correct route name but the wrong _locale.

For example:

When there are two routes available:

en: /bug
de: /fehler

Then the CompiledUrlGenerator and UrlGenerator generate /en/fehler for the German route when en is the default locale.

How to reproduce

https://github.com/git-chrisp/symfony-routing-bug

Visit the path /de/fehler. It should show /de/fehler but shows /en/fehler instead as en is the default locale.

This repository is the website skeleton with the following additions:

  1. DefaultController with one route
  2. The prefix has been configured in config/routes/annotations.yaml
  3. One template to show the bug

Possible Solution

In line 59 of the CompiledUrlGenerator (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Generator/CompiledUrlGenerator.php):

$parameters['_locale'] = $locale ?? $defaults['_locale'];

Same has to be done in the UrlGenerator: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Routing/Generator/UrlGenerator.php

Additional context

Please correct me if I'm wrong or my proposed fix doesn't work. Otherwise, I'd gladly create a pull request.
@xabbuh xabbuh added the Routing label Mar 1, 2020
@fancyweb
Copy link
Contributor
fancyweb commented Mar 2, 2020

I'm going to look at this one.

@fancyweb
Copy link
Contributor
fancyweb commented Mar 2, 2020 •
edited
Loading

Status: works for me but can be improved

IMO, this is a case of misconfiguration. The problem is not on the generation side. Requirements and defaults are added automatically when using i18n routes. A generated i18n route is supposed to have the right _locale as default which is not the case here because prefix defaults and requirements override the right values in the end (see Routing YamlFileLoader), thus allowing /de/bug, /en/fehler and not fallbacking on the good _locale parameter default value. I think we don't support this kind of configuration, see https://symfony.com/doc/current/routing.html#localized-routes-i18n for how we support it.

However, I guess we can improve the DX.

$parameters['_locale'] = $locale ?? $defaults['_locale'];

We can't do this because we need to be able to generate a foo.fr route even when the current context locale is en. There is a test asserting that.

@Tobion
Copy link
Contributor
Tobion commented Mar 2, 2020 •
edited
Loading

/en/fehler shouldn't even be possible. See #35735

@fancyweb fancyweb mentioned this issue Mar 2, 2020
Merged
nicolas-grekas added a commit that referenced this issue Mar 2, 2020
@nicolas-grekas
bug #35928 [Routing] Prevent localized routes _locale default & requi…
afdd507 
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Routing Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@nicolas-grekas @Tobion @xabbuh @fancyweb @christoph-pflueger @carsonbot
0