Code

Opened 4 years ago

Closed 4 years ago

Last modified 18 months ago

#13217 closed Uncategorized (fixed)

LocaleMiddleware breaks caching. (Vary: Cookie)

Reported by: harm Owned by: nobody
Component: Uncategorized Version: 1.2-beta
Severity: Normal Keywords: session accessed vary cookie
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Enabling django.middleware.locale.LocaleMiddleware causes that django adds a 'Vary: Cookie' header to every reponse.

The net effects of responses with Vary: Cookie, is that they are not properly cached by reverse proxies, and worse, never cached by end browsers. (Browser may cache requests with the same cookie, but in reality NO browser is implemented like that,

And this happens for _every_ response

Is there a way to achieve this without always touching the session ? So that we just get a Vary : Accept-Language

similar tickets with other middleware #6552

Attachments (0)

Change History (14)

comment:1 Changed 4 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

harm,

Are you really reporting this against 1.1-beta1?. If the answer is no, we need the exact release/beta/SVN revision at which you are seeing this.

#6552 was about a context processor not a middleware. Are you sure you are not using the same custom context processor that led you to erroneously reopen that ticket?

comment:2 Changed 4 years ago by russellm

  • Resolution set to worksforme
  • Status changed from new to closed

I can't reproduce this. According to my testing, the locale middleware adds the "Vary: Accept-Language" and "Content-Language" headers. From a visual inspection of the code, I can't see anything that would modify the Cookie header.

If you insist this is happening, you'll need to provide a specific example of a view and configuaration that doesn't set Vary: Cookie until you turn on the Locale middleware.

comment:3 Changed 4 years ago by harm

  • Version changed from 1.1-beta-1 to 1.2-beta

I selected the incorrect version -> 1.2-beta

The problem is that the LocaleMiddleware reads the request.session. The Session Middleware later detects that the session was touched (session.accessed) and appends the Vary: Cookie header.

class LocaleMiddleware(object):
 def process_request(self, request):
        language = translation.get_language_from_request(request)   <----- accesses session.

comment:4 Changed 4 years ago by harm

here is a reproducible testcase.

Start with a new project
all default, no custom context processors, no custom middleware

views.y

from django.http import HttpResponse
def hi(request):
     return HttpResponse("hi")

tests.py

from django.test import TestCase
from views import hi

class SimpleTest(TestCase):
    def test_hi(self):
        response = self.client.get('/hi')
                                                                                                                                                       vary = response.get('vary',None)
  if vary:
     print "Vary: %s" % (vary)  
  else:                        
     print " - no vary headers "

without django.middleware.locale.LocaleMiddleware issue python manage.py test . The result is

- no vary headers

add 'django.middleware.locale.LocaleMiddleware' to the MIDDLEWARE_CLASSES in settings.py and issue python manage.py test again. Now you get the incorrect result. A Cookie is added

Vary: Accept-Language, Cookie


comment:5 Changed 4 years ago by harm

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I made a copy/past error in the testcase tests.py above. (missing vary = response.get('vary',None) )

def test_hi(self):
        response = self.client.get('/hi')

        vary = response.get('vary',None)
        if vary:
            print "Vary: %s" % (vary)
        else:
            print " - no vary headers "

comment:6 Changed 4 years ago by ramiro

I don't have time to set up an environment to tests this right now, but just wanted to point that 1.2-beta 1 got bundled on Feb 06 and the [12546] and [12624] changes were committed after that (Feb 23 and March 01 respectively). If you are effectively using the beta1 tarball, can you retry with a newer SVN checkout?

comment:7 Changed 4 years ago by russellm

Ok - I can now reproduce. The problem was that I had USE_I18N=False in my settings, which meant that the dummy translation functions were being used.

I can verify this also occurs on 1.1, so it isn't a recent change.

comment:8 follow-up: Changed 4 years ago by harm

I can't test trunk right now.

But reading [12546] and [12624] i don't think they are related. They touch django caching stuff. Not the logic when to add Vary: headers

Thinking about this problem more, its actually quite hard to get this right. Where else as from the session can LocaleMiddleware his info ? With the current code ,if it reads the session, than automatically session.accessed gets set. If session.accessed is set -> Vary: Cookie is added.

Should we allow LocalMiddleware to read out a session via a backdoor? (because the LocalMiddleware _knows_ that it only looks at the language, and that it adds the correct Vary: Content-Language anyway. So that as fas as LocalMiddleware knows, Vary: cookie should not be set.

comment:9 in reply to: ↑ 8 Changed 4 years ago by russellm

  • Resolution set to wontfix
  • Status changed from reopened to closed

Replying to harm:

Should we allow LocalMiddleware to read out a session via a backdoor? (because the LocalMiddleware _knows_ that it only looks at the language, and that it adds the correct Vary: Content-Language anyway. So that as fas as LocalMiddleware knows, Vary: cookie should not be set.

Incorrect. It would be true if Django were *only* using the Accept-Language header to choose the page language. However, the session *is* being read to determine the site language, and a user can modify the site language using the i18n views. As a result, this means that Vary: Cookie is actually correct. The page *is* varying based on what is in the cookie, and as a result, it isn't cacheable independent of the cookie.

I completely agree that this is annoying from a page caching perspective, but I'm not sure I see what we can do about it, other than changing the way i18n works so that it doesn't have any dependency on the cookie -- but this would mean (1) breaking backwards compatibility, and/or (2) removing the ability for users to select their language.

Regrettably, I'm going to close this wontfix.

comment:10 Changed 4 years ago by harm

Yeah, I get (now) that this can't be solved for views that actually require translation.

Its rather unfortunate though , that using i18n renders the complete site 0% cachable. (except media/) Even parts of the site that are not related to i18n at all.
I really feel this is an undesired situation....

I still wonder if adding some settings/parameters. to LocaleMiddleWare can improve the current situation.
For example: the ability to exclude/include apps (by means of urls) from LocaleMiddleware. this prevents LocaleMiddleware from touching the session for urls where you as site builder you don't want translations.

That would give the freedom to explicitly control (if desired) what parts of your site you want to i18n enable. (or the reverse which parts of you site you want to exclude). All possible with backwards compatibility.

Any thoughts ?

comment:11 Changed 4 years ago by russellm

In short, no, I don't have any ideas. If you want to follow this up, I suggest starting a thread in django-developers. It's a non-trivial topic, and it's going to require a lot of thought and discussion if we're going to get it right. Trac isn't the best place to have this discussion.

comment:13 Changed 4 years ago by gabrielhurley

  • Keywords session accessed vary cookie added

comment:14 Changed 18 months ago by aaugustin

  • Easy pickings unset
  • Resolution changed from wontfix to fixed
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

If you're using i18n_patterns (introduced in Django 1.4) and the URL contains a language prefix, get_language_from_request will select the active language without reading the session.

So, in practice, this is fixed.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.