Bug 49570 - The CompressionFilter example should support HTTP proxies to cache gzipped content better by sending Vary: Accept-Encoding header
The CompressionFilter example should support HTTP proxies to cache gzipped co...
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Examples
trunk
All All
: P2 enhancement (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-07-08 02:11 UTC by Thai Ha
Modified: 2010-07-23 08:01 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thai Ha 2010-07-08 02:11:36 UTC
At the moment the Compression Filter example doesn't send the header "Vary: Accept-Encoding" with the compressed content (see method writeToGZip(..) in http://svn.apache.org/repos/asf/tomcat/trunk/webapps/examples/WEB-INF/classes/compressionFilters/CompressionResponseStream.java). 

From many sources like

1. http://httpd.apache.org/docs/2.0/mod/mod_deflate.html#proxies
2. http://developer.yahoo.net/blog/archives/2007/07/high_performanc_3.html

it looks like a "Vary: Accept-Encoding" header should be sent with the compressed content to make sure proxy servers can serve gzipped content correctly.

To enhance this example, the method writeToGzip(..) should be updated to send Vary: Accept-Encoding header like below:

public void writeToGZip(byte b[], int off, int len) throws IOException {
...
   response.addHeader("Content-Encoding", "gzip");
   response.addHeader("Vary", "Accept-Encoding");
   gzipstream = new GZIPOutputStream(output);
...
}
Comment 1 Mark Thomas 2010-07-10 13:21:15 UTC
The current approach to gzip compression is fundamentally flawed. See bug46538 and bug39727 for an explanation of why. Transfer-Encoding is the correct way to go but browser support is still patchy.

That said, the majority of the current botched solutions - Tomcat included - do set the Vary header. The proposed patch isn't quite right - see bug 48660 for details.

I have applied a corrected patch to to 7.0.x and it will be included in 7.0.1 onwards.
Comment 2 Thai Ha 2010-07-12 23:39:11 UTC
The patch in SVN doesn't work for me because there is no method HttpServletResponse.getHeader()


protected HttpServletResponse response = null;
....
response.addHeader("Content-Encoding", "gzip");
String vary = response.getHeader("Vary");
if (vary == null) {
    // Add a new Vary header
    response.setHeader("Vary", "Accept-Encoding");
} else if (vary.equals("*")) {
    // No action required
} else {
    // Merge into current header
    response.setHeader("Vary", vary + ",Accept-Encoding");
}
Comment 3 Thai Ha 2010-07-15 22:12:30 UTC
A proposed patch is below:

if (response.containsHeader("Vary")) {
  response.addHeader("Vary", "Accept-Encoding");
} else {
  response.setHeader("Vary", "Accept-Encoding");
}
Comment 4 Mark Thomas 2010-07-23 08:01:54 UTC
(In reply to comment #2)
> The patch in SVN doesn't work for me because there is no method
> HttpServletResponse.getHeader()

That is a Servlet 3.0 method and this is Tomcat 7 so that method is available.

A patch similar to what you describe would be required for Tomcat 6 but the patch you propose still suffers from the issues described in bug 48660. Note I do not propose back-porting this fix to Tomcat 6.