Incompatibility between the set_language() view, LocaleMiddleware, and i18n_patterns() when prefix_default_language=False.

[George Tantiras]

Using python 3.4.

In a fresh Django 1.11 install, I configured 2 languages: English (default Language) and Greek.

When I implemented the example ​HTML template code to add a change language drop down in admin, everything worked as expected if prefix_default_language=True

However, the following configuration in urls.py although it works when I change from English to Greek, it will stay to the Greek page when I try to change from Greek to English:

clean = [
    url(r'^i18n/', include('django.conf.urls.i18n')),
]

admin = i18n_patterns(
    url(r'^admin/', admin.site.urls),
    prefix_default_language=False
)


urlpatterns = admin + clean

[Tim Graham]

Can you provide a test that demonstrates the problem? Ideally, as part of Django's test suite. b8a815e9dfea89034ede7ff786551f89af84a31b may give you an idea of where to add a test.

[George Tantiras]

I have written a test class in my project, for the time being. It is a challenge to integrate it to Django, so I am pasting it here until then:

from django.test import TestCase

class setlangTest(TestCase):
    def test_en2gr(self):
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'el'},
            follow=False,
            HTTP_REFERER='/admin/',
        )
        self.assertEqual(response.url, '/el/admin/', 'Did not change from English to Greek')

    def test_gr2en(self):
        # This will fail if prefix_default_language=False
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'en'},
            follow=False,
            HTTP_REFERER='/el/admin/',
        )
        self.assertEqual(response.url, '/admin/', 'Did not change from Greek to English')

[George Tantiras]

I have created a relevant ​pull request which includes the above tests.

[Tomer Chachamu]

Thanks for that test, it really clarified it.

In the HTML in the documentation you mentioned, it mentions the redirect_to context variable, but doesn't really explain what to set it to. In your case, it is empty, so the set_language view has started looking at the HTTP_REFERRER as shown in your tests.

You need to set redirect_to to the current page URL, without the language prefix. For example, you could add this context processor to your settings:

from django.urls import translate_url

def redirect_path_context_processor(request):
    return {'language_select_redirect_to': translate_url(request.path, settings.LANGUAGE_CODE)}

The set_language view will then translate the URL again to the user's selected language.

Also, if you *do* use prefix_default_language=True, then the URL for the set_language view should also be prefixed. I don't think this is documented anywhere.

[Tomer Chachamu]

Here are some passing tests showing how the view will behave when you have set next correctly.

@override_settings(
    USE_I18N=True,
    LANGUAGES=[
        ('en', 'English'),
        ('fr', 'French'),
        ('de', 'German'),
    ],
    MIDDLEWARE=[
        'django.contrib.sessions.middleware.SessionMiddleware',
        'django.contrib.auth.middleware.AuthenticationMiddleware',
        'django.middleware.locale.LocaleMiddleware',
        'django.middleware.common.CommonMiddleware',
    ],
    ROOT_URLCONF='i18n.i18n_unprefixed_urls',
    LANGUAGE_CODE='en',
)
class UnprefixedI18nSetLang(TestCase):
    def test_en2fr(self):
        self.client.post('/i18n/setlang/', data={'language': 'en'})
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'fr', 'next': '/admin/'},
            follow=True,
        )
        self.assertEqual(
            response.redirect_chain,
            [('/fr/admin/', 302), ('/fr/admin/login/?next=/fr/admin/', 302)]
        )

    def test_fr2en(self):
        self.client.post('/i18n/setlang/', data={'language': 'fr'})
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'en', 'next': '/admin/'},
            follow=True,
        )
        self.assertEqual(
            response.redirect_chain,
            [('/admin/', 302), ('/admin/login/?next=/admin/', 302)]
            )

    def test_fr2de(self):
        self.client.post('/i18n/setlang/', data={'language': 'fr'})
        response = self.client.post(
            '/i18n/setlang/',
            data={'language': 'de', 'next': '/admin/'},
            follow=True,
        )
        self.assertEqual(
            response.redirect_chain,
            [('/de/admin/', 302), ('/de/admin/login/?next=/de/admin/', 302)]
            )

[George Tantiras]

Thank you, indeed. The docs ​https://docs.djangoproject.com/en/1.11/topics/i18n/translation/#the-set-language-redirect-view read:

After setting the language choice, Django looks for a next parameter in the POST or GET data. If that is found and Django considers it to be a safe URL (i.e. it doesn’t point to a different host and uses a safe scheme), a redirect to that URL will be performed. Otherwise, Django may fall back to redirecting the user to the URL from the Referer header or, if it is not set, to /, depending on the nature of the request:

And the example -I am using in my code where the problem appeared- has the following line:

input name="next" type="hidden" value="{{ redirect_to }}" />

However, it is not clear how can the variabe redirect_to be set (in the base.html admin template) and it stays empty.

[Tomer Chachamu]

Hi George, try adding the above context processor in your settings.TEMPLATES['OPTIONS']['context_processors'], see ​https://docs.djangoproject.com/en/1.11/topics/templates/#django.template.backends.django.DjangoTemplates

[George Tantiras]

Bingo!

I managed to add it, and it works as expected.

Thank you!

[George Tantiras]

​PR

[Carlton Gibson]

This looks like a bug to me, rather than a documentation issue.

You shouldn't have to set the next parameter, manually calling translate_url() to get the value, since ​the `set_language()` view does that itself:

                next_trans = translate_url(next, lang_code)
                if next_trans != next:
                    response = HttpResponseRedirect(next_trans)

A breakpoint inserted here, in the failing test case George provided (​Original PR), shows the translate_url() call failing.

Furthermore, as per the report, the translate_url() call works when prefix_default_language=True.

The issue lies in the relation between the set_language() view, LocaleMiddleware, and i18n_patterns() when prefix_default_language=False.

If you disable LocaleMiddleware and set the the request language using translation.override() (to, e.g. 'el' here) the test passes.

The issue is in ​`LocaleMiddle.process_request()`:

    def process_request(self, request):
        urlconf = getattr(request, 'urlconf', settings.ROOT_URLCONF)
        i18n_patterns_used, prefixed_default_language = is_language_prefix_patterns_used(urlconf)
        language = translation.get_language_from_request(request, check_path=i18n_patterns_used)
        language_from_path = translation.get_language_from_path(request.path_info)
        if not language_from_path and i18n_patterns_used and not prefixed_default_language:
            language = settings.LANGUAGE_CODE
        translation.activate(language)
        request.LANGUAGE_CODE = translation.get_language()

Here, because the set_language() view is routed outside of i18n_patterns(), not language_from_path and i18n_patterns_used and not prefixed_default_language is True and so the language is always set to settings.LANGUAGE_CODE. This means that translate_url() never gets a match trying to resolve the url to translate (because the wrong language is activated).

IF you route the set_language() view inside i18n_patterns() then the test passes but you can't do that because it's ​explicitly warned against in the docs:

Warning
Make sure that you don’t include the above URL within i18n_patterns() - it needs to be language-independent itself to work correctly.

Two thoughts:

• I'm not sure right now why that warning is required. (Could it be changed?)
• translate_url() only functions on URLs coming from the currently active language. Would it be feasible to have set_language() redetermine the request language before calling translate_url()?
• Is LocaleMiddleware's not language_from_path and i18n_patterns_used and not prefixed_default_language` correct? (I mean it seems it but...)

Option 2 seems the most likely. None of them look that nice.

Claude: I'd be grateful if you could check my reasoning here. 😬 Thanks.

[Carlton Gibson]

#30272 is related here.

[George Tantiras]

A decision is pending about the suited course of action for this case.

[Mariusz Felisiak]

#35009 was a duplicate.

[Mariusz Felisiak]

#35034 was a duplicate.

[Eric Soroos]

#35034 was marked as a duplicate -- however it's a little different, with no relationship to the prefix_default_language:
(note, this is in 4.2 as of the time of posting, but bug history suggests 1.x through 5.0)

When posting to /i18n/setlang/:

• with a referrer in a language that's not the current django cooke language like /km/admin/foo/bar,
• with a current language cookie of django_language=en
• and a post content of next=&language=en

django.urls.translate_urls attempts to resolve the url using the current request language (in this case, en) and fails, because the url is in a different language (km).

i18n.set_language then returns the original referrer in the location header of the redirect, and the language doesn't apparently change, confusing the user.

This is approximately option 2 from https://code.djangoproject.com/ticket/28567#comment:14, but with the resolution entirely in translate_urls instead of set_language. The patch for translate_urls to handle alternate locales is approximately:

from django.utils.translation import override, check_for_language, get_language_from_path

...

def translate_url(url, lang_code):
    """                                                                                                                                                                            
    Given a URL (absolute or relative), try to get its translated version in                                                                                                       
    the `lang_code` language (either by i18n_patterns or by translated regex).                                                                                                     
    Return the original URL if no translated version is found.                                                                                                                     
    """
    parsed = urlsplit(url)
    try:
        # URL may be encoded.                                                                                                                                                      
        try:
            match = resolve(unquote(parsed.path))
        except Resolver404:
            url_lang_code = get_language_from_path(unquote(parsed.path))
            if url_lang_code and check_for_language(url_lang_code):
                with override(url_lang_code):
                    match = resolve(unquote(parsed.path))
            else:
                raise
    except Resolver404:
        pass
    else:
...

​https://github.com/django/django/compare/main...EricSoroos:django:35034-translate-url

[Jason Judkins]

I've picked this up (PR ​21240) and want to summarize where the ticket stands, since the history is spread across several comments and duplicates.

The root cause was diagnosed by Carlton Gibson in comment:14: translate_url() resolves the URL using whatever language is currently active, rather than the language encoded in the URL itself. When LocaleMiddleware activates a language that differs from the URL's prefix — which happens whenever the language cookie and the URL prefix are out of sync — resolve() raises Resolver404, translate_url() silently swallows it, and the original URL is returned unchanged. comment:14 laid out three possible fix locations (translate_url, set_language, LocaleMiddleware) and suggested Option 2 (have the resolution account for the URL's own language) as most likely, while noting none looked especially clean.

Eric Soroos independently arrived at the same conclusion in comment:20, posting a translate_url()-based patch that resolves with the active language first and falls back to the URL-prefix language. PR 21240 converges on that same approach: source-language detection now lives inside translate_url() itself, so the function derives the correct language from the URL it is given rather than depending on caller-set context.
Two things PR 21240 adds beyond the prior patches:

A regression test for the set_language integration path (cookie language ≠ URL prefix language), plus direct translate_url() unit tests covering both prefixed and unprefixed translated URLs — including the unprefixed case raised in review, which the first iteration of the patch missed.
Full-suite verification. The i18n, view_tests, urlpatterns, and urlpatterns_reverse suites pass with no regressions, which I think directly addresses the "none of them look that nice" hesitation in comment:14 — fixing this in translate_url() does not break any existing caller.

The remaining open question from comment:14 is whether translate_url() is the agreed-upon home for the fix, versus set_language or LocaleMiddleware. Three independent contributors have now landed on translate_url(), and the empirical results suggest it's safe, but a decision from someone with authority over i18n would help unblock a ticket that has been pending one since 2018. Given Claude Paroz is cc'd here as the i18n domain expert, his read would be especially valuable.

[Jason Judkins]

@EricSoroos — flagging that I've picked up the translate_url() approach you proposed in comment:20 over in PR 21240, with a regression test and the unprefixed-URL case added. Credited in the PR commit and description. Happy to have your input if you'd like to weigh in.

[Claude Paroz]

Thanks Jason for this impressive work!

I would say that generally I'm trusting the test suite a lot, so if you added a regression test and all other tests pass, that's a very good sign. However I'm seeing tests failing in the current version of your patch, so it may be that some language setting is leaking to unrelated tests. Something to fix.

Note also that this will not enter the stable release, so you can safely drop any release note about this, in my opinion (we are not mentioning every bug fix).

[Jason Judkins]

Updated PR #21240:

Fixed parallel test isolation issue: test_setlang_with_mismatched_cookie_and_next_url_prefix was leaving language state activated in the worker process after the test completed, causing failures in unrelated auth tests when run in parallel. Fixed by calling deactivate_all() at the end of the test.
Removed release notes entry per comment:25.