381521 Only set Vary header when content could be compressed

3 files changed, 102 insertions, 57 deletions

diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java
index 36ecb1fd35..e49e9f1ee2 100644
--- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java
+++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java
@@ -104,15 +104,21 @@ import org.eclipse.jetty.util.log.Logger;
  *                            instead.
  * 
  * excludePathPatterns        Same as excludePath, but accepts regex patterns for more complex matching.
+ * 
+ * vary                       Set to the value of the Vary header sent with responses that could be compressed.  By default it is 
+ *                            set to 'Vary: Accept-Encoding, User-Agent' since IE6 is excluded by default from the excludedAgents. 
+ *                            If user-agents are not to be excluded, then this can be set to 'Vary: Accept-Encoding'.  Note also 
+ *                            that shared caches may cache copies of a resource that is varied by User-Agent - one per variation of 
+ *                            the User-Agent, unless the cache does some normalization of the UA string.
  * </PRE>
  */
 public class GzipFilter extends UserAgentFilter
 {
     private static final Logger LOG = Log.getLogger(GzipFilter.class);
     public final static String GZIP="gzip";
-    public final static String ETAG_GZIP="-gzip\"";
+    public final static String ETAG_GZIP="--gzip\"";
     public final static String DEFLATE="deflate";
-    public final static String ETAG_DEFLATE="-deflate\"";
+    public final static String ETAG_DEFLATE="--deflate\"";
     public final static String ETAG="o.e.j.s.GzipFilter.ETag";
 
     protected ServletContext _context;
@@ -125,6 +131,7 @@ public class GzipFilter extends UserAgentFilter
     protected Set<Pattern> _excludedAgentPatterns;
     protected Set<String> _excludedPaths;
     protected Set<Pattern> _excludedPathPatterns;
+    protected String _vary="Accept-Encoding, User-Agent";
     
     private static final int STATE_SEPARATOR = 0;
     private static final int STATE_Q = 1;
@@ -202,6 +209,10 @@ public class GzipFilter extends UserAgentFilter
             while (tok.hasMoreTokens())
                 _excludedPathPatterns.add(Pattern.compile(tok.nextToken()));            
         }       
+        
+        tmp=filterConfig.getInitParameter("vary");
+        if (tmp!=null)
+            _vary=tmp;
     }
 
     /* ------------------------------------------------------------ */
@@ -224,9 +235,9 @@ public class GzipFilter extends UserAgentFilter
         HttpServletRequest request=(HttpServletRequest)req;
         HttpServletResponse response=(HttpServletResponse)res;
 
-        // Exclude URIs - no Vary because no matter what client, this URI is always excluded
+        // If not a GET or an Excluded URI - no Vary because no matter what client, this URI is always excluded
         String requestURI = request.getRequestURI();
-        if (isExcludedPath(requestURI))
+        if (!HttpMethods.GET.equalsIgnoreCase(request.getMethod()) || isExcludedPath(requestURI))
         {
             super.doFilter(request,response,chain);
             return;
@@ -244,54 +255,45 @@ public class GzipFilter extends UserAgentFilter
                 return;
             }
         }
-
-        // Inform caches that responses may vary according to Accept-Encoding (this may be nulled later)
-        response.setHeader("Vary","Accept-Encoding");
         
-        // Exclude User-Agents
+        // Excluded User-Agents
         String ua = getUserAgent(request);
-        String compressionType = selectCompression(request.getHeader("accept-encoding"));
+        boolean ua_excluded=ua!=null&&isExcludedAgent(ua);
+        
+        // Acceptable compression type
+        String compressionType = ua_excluded?null:selectCompression(request.getHeader("accept-encoding"));
         
-        // If this request is not excluded by agent and if it can be compressed
-        if (!isExcludedAgent(ua) && compressionType!=null && !response.containsHeader("Content-Encoding") && !HttpMethods.HEAD.equalsIgnoreCase(request.getMethod()))
+        // Special handling for etags
+        String etag = request.getHeader("If-None-Match"); 
+        if (etag!=null)
         {
-            // Special handling for etags
-            String etag = request.getHeader("If-None-Match"); 
-            if (etag!=null)
-            {
-                if (etag.endsWith(ETAG_GZIP))
-                    request.setAttribute(ETAG,etag.substring(0,etag.length()-ETAG_GZIP.length())+'"');
-                else if (etag.endsWith(ETAG_DEFLATE))
-                    request.setAttribute(ETAG,etag.substring(0,etag.length()-ETAG_DEFLATE.length())+'"');
-            }
-            
-            CompressedResponseWrapper wrappedResponse = createWrappedResponse(request,response,compressionType);
-            
-            boolean exceptional=true;
-            try
+            int dd=etag.indexOf("--");
+            if (dd>0)
+                request.setAttribute(ETAG,etag.substring(0,dd)+(etag.endsWith("\"")?"\"":""));
+        }
+
+        CompressedResponseWrapper wrappedResponse = createWrappedResponse(request,response,compressionType);
+
+        boolean exceptional=true;
+        try
+        {
+            super.doFilter(request,wrappedResponse,chain);
+            exceptional=false;
+        }
+        finally
+        {
+            Continuation continuation = ContinuationSupport.getContinuation(request);
+            if (continuation.isSuspended() && continuation.isResponseWrapped())   
             {
-                super.doFilter(request,wrappedResponse,chain);
-                exceptional=false;
+                continuation.addContinuationListener(new ContinuationListenerWaitingForWrappedResponseToFinish(wrappedResponse));
             }
-            finally
+            else if (exceptional && !response.isCommitted())
             {
-                Continuation continuation = ContinuationSupport.getContinuation(request);
-                if (continuation.isSuspended() && continuation.isResponseWrapped())   
-                {
-                    continuation.addContinuationListener(new ContinuationListenerWaitingForWrappedResponseToFinish(wrappedResponse));
-                }
-                else if (exceptional && !response.isCommitted())
-                {
-                    wrappedResponse.resetBuffer();
-                    wrappedResponse.noCompression();
-                }
-                else
-                    wrappedResponse.finish();
+                wrappedResponse.resetBuffer();
+                wrappedResponse.noCompression();
             }
-        }
-        else
-        {
-            super.doFilter(request,new VaryResponseWrapper(response),chain);
+            else
+                wrappedResponse.finish();
         }
     }
 
@@ -387,14 +389,32 @@ public class GzipFilter extends UserAgentFilter
     protected CompressedResponseWrapper createWrappedResponse(HttpServletRequest request, HttpServletResponse response, final String compressionType)
     {
         CompressedResponseWrapper wrappedResponse = null;
-        if (compressionType.equals(GZIP))
+        if (compressionType==null)
         {
             wrappedResponse = new CompressedResponseWrapper(request,response)
             {
                 @Override
                 protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
                 {
-                    return new AbstractCompressedStream(compressionType,request,this)
+                    return new AbstractCompressedStream(null,request,this,_vary)
+                    {
+                        @Override
+                        protected DeflaterOutputStream createStream() throws IOException
+                        {
+                            return null;
+                        }
+                    };
+                }
+            };
+        }
+        else if (compressionType.equals(GZIP))
+        {
+            wrappedResponse = new CompressedResponseWrapper(request,response)
+            {
+                @Override
+                protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
+                {
+                    return new AbstractCompressedStream(compressionType,request,this,_vary)
                     {
                         @Override
                         protected DeflaterOutputStream createStream() throws IOException
@@ -412,7 +432,7 @@ public class GzipFilter extends UserAgentFilter
                 @Override
                 protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
                 {
-                    return new AbstractCompressedStream(compressionType,request,this)
+                    return new AbstractCompressedStream(compressionType,request,this,_vary)
                     {
                         @Override
                         protected DeflaterOutputStream createStream() throws IOException
@@ -422,7 +442,7 @@ public class GzipFilter extends UserAgentFilter
                     };
                 }
             };
-        }
+        } 
         else
         {
             throw new IllegalStateException(compressionType + " not supported");
@@ -572,11 +592,8 @@ public class GzipFilter extends UserAgentFilter
                     ct=ct.substring(0,colon);
             }
         
-            if (_mimeTypes!=null && !_mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))
-                // Remove the vary header, because of content type.
-                super.setHeader("Vary",null);
-            else
-                super.setHeader("Vary","Accept-Encoding");
+            if (_mimeTypes!=null && _mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))
+                super.setHeader("Vary",_vary);
                 
         }
     }
diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludableGzipFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludableGzipFilter.java
index 8ff4608a70..5ba2be0022 100644
--- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludableGzipFilter.java
+++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/IncludableGzipFilter.java
@@ -70,14 +70,41 @@ public class IncludableGzipFilter extends GzipFilter
     protected CompressedResponseWrapper createWrappedResponse(HttpServletRequest request, HttpServletResponse response, final String compressionType)
     {
         CompressedResponseWrapper wrappedResponse = null;
-        if (compressionType.equals(GZIP))
+        if (compressionType==null)
         {
             wrappedResponse = new IncludableResponseWrapper(request,response)
             {
                 @Override
                 protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
                 {
-                    return new AbstractCompressedStream(compressionType,request,this)
+                    return new AbstractCompressedStream(null,request,this,_vary)
+                    {
+                        @Override
+                        protected DeflaterOutputStream createStream() throws IOException
+                        {
+                            return null;
+                        }
+
+                        @Override
+                        protected void setHeader(String name, String value)
+                        {
+                            super.setHeader(name, value);
+                            HttpServletResponse response = (HttpServletResponse)getResponse();
+                            if (!response.containsHeader(name))
+                                response.setHeader("org.eclipse.jetty.server.include." + name, value);
+                        }
+                    };
+                }
+            };
+        }
+        else if (compressionType.equals(GZIP))
+        {
+            wrappedResponse = new IncludableResponseWrapper(request,response)
+            {
+                @Override
+                protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
+                {
+                    return new AbstractCompressedStream(compressionType,request,this,_vary)
                     {
                         @Override
                         protected DeflaterOutputStream createStream() throws IOException
@@ -104,7 +131,7 @@ public class IncludableGzipFilter extends GzipFilter
                 @Override
                 protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
                 {
-                    return new AbstractCompressedStream(compressionType,request,this)
+                    return new AbstractCompressedStream(compressionType,request,this,_vary)
                     {
                         @Override
                         protected DeflaterOutputStream createStream() throws IOException
diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/GzipTester.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/GzipTester.java
index 7146a34e67..7cac0d3afd 100644
--- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/GzipTester.java
+++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/GzipTester.java
@@ -306,13 +306,13 @@ public class GzipTester
     {
         Assert.assertThat("Response.method",response.getMethod(),nullValue());
         Assert.assertThat("Response.status",response.getStatus(),is(status));
+        Assert.assertThat("Response.header[Content-Encoding]",response.getHeader("Content-Encoding"),not(containsString(compressionType)));
         if (expectedFilesize != (-1))
         {
             Assert.assertThat("Response.header[Content-Length]",response.getHeader("Content-Length"),notNullValue());
             int serverLength = Integer.parseInt(response.getHeader("Content-Length"));
             Assert.assertThat("Response.header[Content-Length]",serverLength,is(expectedFilesize));
         }
-        Assert.assertThat("Response.header[Content-Encoding]",response.getHeader("Content-Encoding"),not(containsString(compressionType)));
     }
 
     private HttpTester executeRequest(String uri) throws IOException, Exception
@@ -465,6 +465,7 @@ public class GzipTester
         ServletHolder servletHolder = servletTester.addServlet(servletClass,"/");
         servletHolder.setInitParameter("baseDir",testdir.getDir().getAbsolutePath());
         FilterHolder holder = servletTester.addFilter(gzipFilterClass,"/*",0);
+        holder.setInitParameter("vary","Accept-Encoding");
         return holder;
     }