381521 Only set Vary header when content could be compressed

jetty-http/src/main/java/org/eclipse/jetty/http/gzip/AbstractCompressedStream.java

@@ -40,6 +40,7 @@ import org.eclipse.jetty.util.ByteArrayOutputStream2;
 public abstract class AbstractCompressedStream extends ServletOutputStream 
 {
     private final String _encoding;
+    protected final String _vary;
     protected final CompressedResponseWrapper _wrapper;
     protected final HttpServletResponse _response;
     protected OutputStream _out;
@@ -52,12 +53,13 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
      * Instantiates a new compressed stream.
      * 
      */
-    public AbstractCompressedStream(String encoding,HttpServletRequest request, CompressedResponseWrapper wrapper)
+    public AbstractCompressedStream(String encoding,HttpServletRequest request, CompressedResponseWrapper wrapper,String vary)
             throws IOException
     {
         _encoding=encoding;
         _wrapper = wrapper;
         _response = (HttpServletResponse)wrapper.getResponse();
+        _vary=vary;
         
         if (_wrapper.getMinCompressSize()==0)
             doCompress();
@@ -106,7 +108,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
         {
             long length=_wrapper.getContentLength();
             if (length > 0 && length < _wrapper.getMinCompressSize())
-                doNotCompress();
+                doNotCompress(false);
             else
                 doCompress();
         }
@@ -137,13 +139,14 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
                     _wrapper.setContentLength(length);
                 }
                 if (length < _wrapper.getMinCompressSize())
-                    doNotCompress();
+                    doNotCompress(false);
                 else
                     doCompress();
             }
             else if (_out == null)
             {
-                doNotCompress();
+                // No output
+                doNotCompress(false);
             }
 
             if (_compressedOutputStream != null)
@@ -168,7 +171,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
             {
                 long length=_wrapper.getContentLength();
                 if (length > 0 && length < _wrapper.getMinCompressSize())
-                    doNotCompress();
+                    doNotCompress(false);
                 else
                     doCompress();
             }
@@ -226,11 +229,15 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
             if (_response.isCommitted())
                 throw new IllegalStateException();
             
+            if (_encoding!=null)
+            {
                 setHeader("Content-Encoding", _encoding);            
                 if (_response.containsHeader("Content-Encoding"))
                 {
+                    setHeader("Vary",_vary);
                     _out=_compressedOutputStream=createStream();
-
+                    if (_out!=null)
+                    {
                         if (_bOut!=null)
                         {
                             _out.write(_bOut.getBuf(),0,_bOut.getCount());
@@ -240,9 +247,12 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
                         String etag=_wrapper.getETag();
                         if (etag!=null)
                             setHeader("ETag",etag.substring(0,etag.length()-1)+'-'+_encoding+'"');
+                        return;
                     }
-            else 
+                }
-                doNotCompress();
+            }
+            
+            doNotCompress(true); // Send vary as it could have been compressed if encoding was present
         }
     }
 
@@ -252,12 +262,14 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
      * @throws IOException
      *             Signals that an I/O exception has occurred.
      */
-    public void doNotCompress() throws IOException
+    public void doNotCompress(boolean sendVary) throws IOException
     {
         if (_compressedOutputStream != null)
             throw new IllegalStateException("Compressed output stream is already assigned.");
         if (_out == null || _bOut != null)
         {
+            if (sendVary)
+                setHeader("Vary",_vary);
             if (_wrapper.getETag()!=null)
                 setHeader("ETag",_wrapper.getETag());
                 
@@ -289,7 +301,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
         {
             long length=_wrapper.getContentLength();
             if (_response.isCommitted() || (length >= 0 && length < _wrapper.getMinCompressSize()))
-                doNotCompress();
+                doNotCompress(false);
             else if (lengthToWrite > _wrapper.getMinCompressSize())
                 doCompress();
             else
@@ -299,7 +311,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
         {
             long length=_wrapper.getContentLength();
             if (_response.isCommitted() || (length >= 0 && length < _wrapper.getMinCompressSize()))
-                doNotCompress();
+                doNotCompress(false);
             else if (lengthToWrite >= (_bOut.getBuf().length - _bOut.getCount()))
                 doCompress();
         }

jetty-http/src/main/java/org/eclipse/jetty/http/gzip/CompressedResponseWrapper.java

@@ -138,8 +138,6 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
             (_mimeTypes==null && ct!=null && ct.contains("gzip") ||
              _mimeTypes!=null && (ct==null||!_mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))))
         {
-            // Remove the vary header, because of content type.
-            setHeader("Vary",null);
             noCompression();
         }
     }
@@ -318,7 +316,7 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
         {
             try
             {
-                _compressedStream.doNotCompress();
+                _compressedStream.doNotCompress(false);
             }
             catch (IOException e)
             {

jetty-server/src/main/java/org/eclipse/jetty/server/handler/GzipHandler.java

@@ -68,6 +68,7 @@ public class GzipHandler extends HandlerWrapper
     protected Set<String> _excluded;
     protected int _bufferSize = 8192;
     protected int _minGzipSize = 256;
+    protected String _vary = "Accept-Encoding, User-Agent";
 
     /* ------------------------------------------------------------ */
     /**
@@ -161,6 +162,31 @@ public class GzipHandler extends HandlerWrapper
         }
     }
 
+    /* ------------------------------------------------------------ */
+    /**
+     * @return The value of the Vary header set if a response can be compressed.
+     */
+    public String getVary()
+    {
+        return _vary;
+    }
+
+    /* ------------------------------------------------------------ */
+    /**
+     * Set the value of the Vary header sent with responses that could be compressed.  
+     * <p>
+     * By default it is set to '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 'Accept-Encoding'.  Note also that shared caches may cache 
+     * many 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.
+     * @param vary The value of the Vary header set if a response can be compressed.
+     */
+    public void setVary(String vary)
+    {
+        _vary = vary;
+    }
+
     /* ------------------------------------------------------------ */
     /**
      * Get the buffer size.
@@ -297,7 +323,7 @@ public class GzipHandler extends HandlerWrapper
             @Override
             protected AbstractCompressedStream newCompressedStream(HttpServletRequest request,HttpServletResponse response) throws IOException
             {
-                return new AbstractCompressedStream("gzip",request,this)
+                return new AbstractCompressedStream("gzip",request,this,_vary)
                 {
                     @Override
                     protected DeflaterOutputStream createStream() throws IOException

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;
@@ -245,24 +256,20 @@ public class GzipFilter extends UserAgentFilter
             }
         }
         
-        // Inform caches that responses may vary according to Accept-Encoding (this may be nulled later)
+        // Excluded User-Agents
-        response.setHeader("Vary","Accept-Encoding");
-        
-        // Exclude 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)
         {
-                if (etag.endsWith(ETAG_GZIP))
+            int dd=etag.indexOf("--");
-                    request.setAttribute(ETAG,etag.substring(0,etag.length()-ETAG_GZIP.length())+'"');
+            if (dd>0)
-                else if (etag.endsWith(ETAG_DEFLATE))
+                request.setAttribute(ETAG,etag.substring(0,dd)+(etag.endsWith("\"")?"\"":""));
-                    request.setAttribute(ETAG,etag.substring(0,etag.length()-ETAG_DEFLATE.length())+'"');
         }
 
         CompressedResponseWrapper wrappedResponse = createWrappedResponse(request,response,compressionType);
@@ -289,11 +296,6 @@ public class GzipFilter extends UserAgentFilter
                 wrappedResponse.finish();
         }
     }
-        else
-        {
-            super.doFilter(request,new VaryResponseWrapper(response),chain);
-        }
-    }
 
     /* ------------------------------------------------------------ */
     private String selectCompression(String encodingHeader)
@@ -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
@@ -572,11 +592,8 @@ public class GzipFilter extends UserAgentFilter
                     ct=ct.substring(0,colon);
             }
         
-            if (_mimeTypes!=null && !_mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))
+            if (_mimeTypes!=null && _mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))
-                // Remove the vary header, because of content type.
+                super.setHeader("Vary",_vary);
-                super.setHeader("Vary",null);
-            else
-                super.setHeader("Vary","Accept-Encoding");
                 
         }
     }

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

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;
     }