Page MenuHomePhabricator

Accept-Language is missing in Vary header when it's used to find out user's preferred variant
Closed, ResolvedPublic

Description

Add Accept-Language to Vary header

It can cause some problems when responses are cached by squid etc.


Version: 1.16.x
Severity: normal

attachment vary.diff ignored as obsolete

Details

Reference
bz21672

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 10:54 PM
bzimport set Reference to bz21672.
bzimport added a subscriber: Unknown Object (MLST).

Why did you change the case in Accept-Encoding to accept-encoding (same for Cookie)? I'm not sure if these headers are case-sensitive, but it's best to leave them in their current case. Also, for this to work, OutputPage::getXVO() would have to be changed. Finally, the idea of adding this option inside LanguageConverter doesn't look optimal to me, but I'm not familiar enough with this code to come up with a better idea offhand.

HTTP header names seem to be case-insensitive ( see http://tools.ietf.org/html/rfc2616#section-4.2 ). I changed the case to avoid duplicates. XVO is not a part of HTTP standard, and I know nothing about it... Is it required?

XVO stands for X-Vary-Options: , which is a header Squid uses to determine if something is cacheable. However, I don't believe caching multiple versions of the same page and choosing the one to serve based on a header (like Accept-Language) is possible at the moment.

PhiLiP.NPC wrote:

I have commit this patch to svn with a few changes have been made:

  1. Change the lowercase accept-encoding back to Accept-Encoding.
  2. Change getXVO() to include the Accept-Language header, base on Tim Starling's note on http://old.nabble.com/X-Vary-Options-patch-p15349937.html.
  3. Only add Accept-Language when a valid variant has been found out. In zh case, it's about 7 variants.

If I did something wrong, please tell me.

PhiLiP.NPC wrote:

Oops, about 8 variants: zh-hans, zh-hant, zh-cn, zh-hk, zh-mo, zh-my, zh-sg, zh-tw. But only zh-cn, zh-hk, zh-tw are commonly used.

PhiLiP.NPC wrote:

Forgot to include rev id: r59522, r59523, and r59527.

Created attachment 6832
Add all available variants to Accept-Language in XVO

I don't agree with r59541 (a follow-up) about what the content in X-Vary-Options should be. See my comment there.

Here is my patch. A new way to add stuff to these headers is provided.

I can't find anything about what if no *-contains exists with a single header name...

Attached:

sumanah wrote:

Liangent, I'm sorry, but over time trunk has changed such that your patch no longer applies. If this issue is still a problem, can you update your patch and submit it in Gerrit? Thanks.

A similar implementation is already in codebase. Dunno when it was added.