Cookie based language detection no longer practical

[Ludwik Trammer]

I use cookies to save user's custom language preference (and that's not just a matter of personal taste - I use Varnish, so it's important for me to defer putting a session cookie on user's computer as long as possible, because it is unique per user, so does not work good with caching).

Up until Django 1.6 I successfully used the following method:

1. Use Django's LocaleMiddleware so the correct locale is detected and used.
2. When user switches languages set a cookie with her new preference, with name based on settings.LANGUAGE_COOKIE_NAME.

I think that sounds pretty responsible and in line with what's in documentation. And it worked as expected. LocaleMiddleware used the cookie's value during language detection.

Unfortunately it all break after upgrading to Django 1.6. In Django 1.6 the following code was added to LocaleMiddleware's process_response() method:

# Store language back into session if it is not present
if hasattr(request, 'session'):
    request.session.setdefault('django_language', language)

It has two unfortunate consequences for me. The former more obvious, the latter more important:

1. Session cookie is now always present on user's computer. A session is created as soon a she opens the site for the first time. That's defeats the whole purpose of using django_language cookie instead of using sessions.

2. When a user opens the site for the first time her current language is saved to her session. She haven't had a chance to manually set her preference yet, so the value set to session in based on request headers or LANGUAGE_CODE settings. From now on django_language cookie value is ignored (and for that matter her changing language preferences in her browser is ignored also), because the value from session has the highest priority.

So, to summarise: cookie value is ignored if session value is present, but starting from Django 1.6 session value is always present.

My code broke after upgrading to Django 1.6, just because I used a cookie-based method, that is part of documented Django futures. The only way to use cookies for setting language now, is to disable sessions completely in your project (not an option for me). That's a really big change, and there is literally nothing about it in "backwards incompatible changes" section of Django 1.6 release notes. I also don't see an easy work around this problem for me.

[Ludwik Trammer]

The change was caused by #14825.

[Ludwik Trammer]

Maybe behaviour requested by #14825 could by a setting, so it stops breaking backward compatibility?

[Anssi Kääriäinen]

The solution in #14825 seems a bit over-aggressive. The implementation in there is "force-assign language back to session for each response". For example this means it is impossible to force-clear the session language if you actually want that. And it also means that the language is stored in the session no matter if you actually want to use session to store the language. As session language overrides cookie language this essentially breaks cookie based language selection. This will also break a view that does this:

def change_language(request, lang):
    request.session['django_language'] = lang
    return HttpResponseRedirect(somepage)

Some proposals for fix:

1. Assign the language back to session in contrib.auth.logout() right after session.flush(). Revert the change to localemiddleware
2. In LocaleMiddleware, store the language back only if it came from session. This can be done by setting request._language_was_from_session = True in process_request and checking against that in process_response.
3. Just revert the change.

I don't particularly like the idea that you can't flush the language code from session, and with option 2 above it will be force-restored on process_response. So, I vote for approach 1. This means that session.flush() will remove the language for next request. But if you store the language in a session and then flush it, isn't that what is supposed to happen?

I haven't actually ran any tests to verify this issue but the analysis in the description are convincing enough for me. Marking this as release blocker as this is a regression in 1.6.

[Claude Paroz]

I'm fine with proposal #1.

[Ludwik Trammer]

I wrote a possible fix. Here's a pull request: ​https://github.com/django/django/pull/1991

This only work on stable/1.6.x branch and won't work on master. I will create a separate version for master, provided this looks ok.

Tests are included.

[Claude Paroz]

Patch looks good (and tests pass).

[Ramiro Morales]

See also #12794 and #15902.

[anonymous]

I really hope Django adopts behaviour described in #12794 or #15902. It will make my life much easier, because the current behaviour is not really compatible with solutions like Varnish, which prevents me from using the built-in @set_language@ view.
However this bigger change surely won't happen in the stable 1.6.x branch, so my patch for this branch still stands.

[Claude Paroz]

I admit I'm struggling porting the patch to master, especially with the i18n.LocaleMiddlewareTests.test_backwards_session_language test.

[Ludwik Trammer]

i18n.LocaleMiddlewareTests.test_backwards_session_language "checks that language is stored in session if missing". I believe that means that it tests for the behavior that was explicitly deemed undesirable (or at least this ticket postulates it is undesirable) - the whole point of this ticket is we don't want a missing language to be automatically add to session. So I believe the test should be simply removed and replaced with i18n.LocaleMiddlewareTests.test_language_not_saved_to_session. The latter tests for a behavior that is the exact opposite of the behavior expected by i18n.LocaleMiddlewareTests.test_backwards_session_language.

[Claude Paroz]

I don't think so, test_backwards_session_language is testing that even with old django_language cookie name, the language is still kept in the session (with the new _language cookie name). However, I admit this is going very far, and I wouldn't oppose to simply removing that test.

[Claude Paroz <claude@…>]

In c558a43fd6bbcea9972b66965f7e8619bc247df1:

[1.6.x] Fixed #21473 -- Limited language preservation to logout

Current language is no longer saved to session by LocaleMiddleware
on every response (the behavior introduced in #14825).
Instead language stored in session is reintroduced into new session
after logout.

[Claude Paroz]

Reopening until fixed in master.

[Ludwik Trammer]

Replying to claudep:

I don't think so, test_backwards_session_language is testing that even with old django_language cookie name, the language is still kept in the session (with the new _language cookie name). However, I admit this is going very far, and I wouldn't oppose to simply removing that test.

You are right, sorry. I got confused by the method description, which turns out to be pretty poor (or maybe it's just me not being a native speaker?). Anyhow. This tests something more specific, but it still just tests a specific part of the behavior that is being removed. More importantly this tests requires a behavior that is entirely unnecessary. There is already ​code for backward compatybility with `django_language` session key in django.utils.translations. There is no point for the locale middleware to get involved and additionaly rename the key on the fly. I'd say you should just remove the test.

[Claude Paroz]

Ludwik, mind having a look at the attached patch?

[Ludwik Trammer]

I think it looks perfect.

[Claude Paroz <claude@…>]

In 9922ed46e2e81845006792310f3f1a3bf0d7b2a6:

Fixed #21473 -- Limited language preservation to logout

Current language is no longer saved to session by LocaleMiddleware
on every response (the behavior introduced in #14825).
Instead language stored in session is reintroduced into new session
after logout.

Forward port of c558a43fd6 to master.

[Sergey Kolosov]

I would like to make an appeal to core developers: all design decisions involving involuntary session creation MUST be made with a great caution.

In case of a high-load project, avoiding to create a session for non-authenticated users is a vital strategy with a critical influence on application performance.

It doesn't really make a big difference, whether you use a database backend, or Redis, or whatever else; eventually, your load would be high enough, and scaling further would not help anymore, so that either network access to the session backend or its “INSERT” performance would become a bottleneck.

In my case, it's an application with 20-25 ms response time under a 20000-30000 RPM load.
Having to create a session for an each session-less request would be critical enough to decide not to upgrade Django, or to fork and rewrite the corresponding components.

Thanks.