Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#17866 closed Uncategorized (fixed)

locale middleware adds Vary Accept-Language when language prefix used

Reported by: sero Owned by: ambv
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 3 years ago.

Download all attachments as: .zip

Change History (8)

Changed 3 years ago by sero

comment:1 Changed 3 years ago by sero

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by sero

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

comment:3 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

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 Changed 3 years ago by ambv

  • Owner changed from nobody to ambv
  • Status changed from new to assigned

comment:5 Changed 3 years ago by ambv

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 Changed 3 years ago by Łukasz Langa <lukasz@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 539900f117dce5d9b51e2b9e8ff225c423060d52:

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

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

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