Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#17866 closed Uncategorized (fixed)

locale middleware adds Vary Accept-Language when language prefix used

Reported by: sero Owned by: Łukasz Langa
Component: HTTP handling Version: 1.4-beta-1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When i18n_patterns are in use, locale middleware does pretty nice job and redirects to proper url.
But after redirection to localized site, "Vary: Accept-Language" forces (squid/varnish) to cache a lot of versions of this prefixed url, which isn't necessary in my opinion.

Attachments (1)

language_prefix_vary.patch (621 bytes ) - added by sero 13 years ago.

Download all attachments as: .zip

Change History (8)

by sero, 13 years ago

Attachment: language_prefix_vary.patch added

comment:1 by sero, 13 years ago

Has patch: set

comment:2 by sero, 13 years ago

Sorry, but my patch doesn't work correctly. It removed "Vary: Accept-Language" also on admin site, but it shouldn't.

comment:3 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

We should include the "Vary: Accept-Language" header if and only if this header was actually used to determine the language used for rendering the page.

comment:4 by Łukasz Langa, 12 years ago

Owner: changed from nobody to Łukasz Langa
Status: newassigned

comment:5 by Łukasz Langa, 12 years ago

Pull request:

https://github.com/django/django/pull/796

Tests pass on 2.7 and 3.3.

Remarks:

  • we cannot restrict including "Accept-Language" to only the case when it was used in page rendering. The language might be taken from the session or a client cookie. In that case the user gets that specific language but it doesn't mean the server doesn't vary content depending on Accept-Language (e.g. if the session would be empty and the cookie not set). Such a thing would break caching proxies. Taking that into account, I leave out "Vary: Accept-Language" on i18n_patterns URLs only.
  • the patch reuses supported languages across requests to speed up computation, trans_null had to learn the supported argument to get_language_from_path() (already in trans_real`).
  • now LocaleMiddleware.is_language_prefix_pattern_used() would be computed in every response so I moved this computation to __init__ since it's not going to change between restarts
  • on a similar note, django.utils.translation.trans_real.check_for_language is needlessly computed for every get_language_from_path(). It won't change between restarts of the application, shouldn't we memoize it?

comment:6 by Łukasz Langa <lukasz@…>, 12 years ago

Resolution: fixed
Status: assignedclosed

In 539900f117dce5d9b51e2b9e8ff225c423060d52:

Fixes #17866: Vary: Accept-Language header when language prefix used

comment:7 by Honza Král <Honza.Kral@…>, 12 years ago

In c2a045198c1c006cbb6c1e141221e78bdde9a78f:

Merge pull request #796 from ambv/vary

Fixes #17866: Vary: Accept-Language header when language prefix used

Note: See TracTickets for help on using tickets.
Back to Top