381521 Only set Vary header when content could be compressed

6 files changed, 92 insertions, 33 deletions

diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java
index 086e7a4f35..ae56132cec 100644
--- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java
+++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java
@@ -429,44 +429,37 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
         String pathInContext=URIUtil.addPaths(servletPath,pathInfo);
         boolean endsWithSlash=(pathInfo==null?request.getServletPath():pathInfo).endsWith(URIUtil.SLASH);
         
-        // Can we gzip this request?
-        String pathInContextGz=null;
-        boolean gzip=false;
-        if (!included.booleanValue() && _gzip && reqRanges==null && !endsWithSlash )
-        {
-            // Tell caches that response may vary by accept-encoding
-            response.setHeader(HttpHeaders.VARY,HttpHeaders.ACCEPT_ENCODING);
-            // Should we vary this response according to accept-encoding?
-            String accept=request.getHeader(HttpHeaders.ACCEPT_ENCODING);
-            if (accept!=null && accept.indexOf("gzip")>=0)
-                gzip=true;
-        }
 
         // Find the resource and content
         Resource resource=null;
         HttpContent content=null;
-
         try
         {
-            // Try gzipped content first
-            if (gzip)
+            // is gzip enabled?
+            String pathInContextGz=null;
+            boolean gzip=false;
+            if (!included.booleanValue() && _gzip && reqRanges==null && !endsWithSlash )
             {
+                // Look for a gzip resource
                 pathInContextGz=pathInContext+".gz";
-
                 if (_cache==null)
-                {
                     resource=getResource(pathInContextGz);
-                }
                 else
                 {
                     content=_cache.lookup(pathInContextGz);
                     resource=(content==null)?null:content.getResource();
                 }
 
-                if (resource==null || !resource.exists() || resource.isDirectory())
+                // Does a gzip resource exist?
+                if (resource!=null && resource.exists() && !resource.isDirectory())
                 {
-                    gzip=false;
-                    pathInContextGz=null;
+                    // Tell caches that response may vary by accept-encoding
+                    response.setHeader(HttpHeaders.VARY,HttpHeaders.ACCEPT_ENCODING);
+                    
+                    // Does the client accept gzip?
+                    String accept=request.getHeader(HttpHeaders.ACCEPT_ENCODING);
+                    if (accept!=null && accept.indexOf("gzip")>=0)
+                        gzip=true;
                 }
             }
 
diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
index 0a837f58a9..d444f1bf0e 100644
--- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
+++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
@@ -641,6 +641,42 @@ public class DefaultServletTest
 
 
     @Test
+    public void testGzip() throws Exception
+    {
+        testdir.ensureEmpty();
+        File resBase = testdir.getFile("docroot");
+        FS.ensureDirExists(resBase);
+        File file0 = new File(resBase, "data0.txt");
+        createFile(file0, "Hello Text 0");
+        File file0gz = new File(resBase, "data0.txt.gz");
+        createFile(file0gz, "fake gzip");
+
+        String resBasePath = resBase.getAbsolutePath();
+
+        ServletHolder defholder = context.addServlet(DefaultServlet.class, "/");
+        defholder.setInitParameter("dirAllowed", "false");
+        defholder.setInitParameter("redirectWelcome", "false");
+        defholder.setInitParameter("welcomeServlets", "false");
+        defholder.setInitParameter("gzip", "true");
+        defholder.setInitParameter("resourceBase", resBasePath);
+
+        String response = connector.getResponses("GET /context/data0.txt HTTP/1.1\r\nHost:localhost:8080\r\n\r\n");
+        assertResponseContains("Content-Length: 12", response);
+        assertResponseContains("Hello Text 0",response);
+        assertResponseContains("Vary: Accept-Encoding",response);
+        assertResponseNotContains("Content-Encoding: gzip",response);
+        
+        response = connector.getResponses("GET /context/data0.txt HTTP/1.1\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\n\r\n");
+        assertResponseContains("Content-Length: 9", response);
+        assertResponseContains("fake gzip",response);
+        assertResponseContains("Vary: Accept-Encoding",response);
+        assertResponseContains("Content-Encoding: gzip",response);
+        
+    }
+
+
+
+    @Test
     public void testIfModifiedSmall() throws Exception
     {
         testIfModified("Hello World");
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 352faf22fc..c99319d03b 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
@@ -30,6 +30,7 @@ import java.util.zip.GZIPOutputStream;
 
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
+import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
@@ -111,6 +112,7 @@ public class GzipFilter extends UserAgentFilter
     public final static String ETAG_DEFLATE="-deflate\"";
     public final static String ETAG="o.e.j.s.GzipFilter.ETag";
 
+    protected ServletContext _context;
     protected Set<String> _mimeTypes;
     protected int _bufferSize=8192;
     protected int _minGzipSize=256;
@@ -136,6 +138,8 @@ public class GzipFilter extends UserAgentFilter
     {
         super.init(filterConfig);
         
+        _context=filterConfig.getServletContext();
+        
         String tmp=filterConfig.getInitParameter("bufferSize");
         if (tmp!=null)
             _bufferSize=Integer.parseInt(tmp);
@@ -217,6 +221,20 @@ public class GzipFilter extends UserAgentFilter
         HttpServletRequest request=(HttpServletRequest)req;
         HttpServletResponse response=(HttpServletResponse)res;
         
+        
+        // Check if mime type of request can ever be compressed.
+        if (_mimeTypes!=null && _mimeTypes.size()>0)
+        {
+            String mimeType = _context.getMimeType(request.getRequestURI());
+            
+            if (mimeType!=null && !_mimeTypes.contains(mimeType))
+            {
+                // handle normally without setting vary header
+                super.doFilter(request,response,chain);
+                return;
+            }
+        }
+        
         // Inform caches that responses may vary according to Accept-Encoding
         response.setHeader("Vary","Accept-Encoding");
 
diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterDefaultTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterDefaultTest.java
index 04bbe5cc0e..1b948734f5 100644
--- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterDefaultTest.java
+++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterDefaultTest.java
@@ -27,11 +27,14 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import junit.framework.Assert;
+
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.gzip.CompressedResponseWrapper;
 import org.eclipse.jetty.servlet.DefaultServlet;
 import org.eclipse.jetty.servlet.FilterHolder;
 import org.eclipse.jetty.servlets.gzip.GzipTester;
+import org.eclipse.jetty.testing.HttpTester;
 import org.eclipse.jetty.toolchain.test.TestingDir;
 import org.junit.Rule;
 import org.junit.Test;
@@ -115,7 +118,8 @@ public class GzipFilterDefaultTest
         try
         {
             tester.start();
-            tester.assertIsResponseGzipCompressed("file.txt");
+            HttpTester http = tester.assertIsResponseGzipCompressed("file.txt");
+            Assert.assertEquals("Accept-Encoding",http.getHeader("Vary"));
         }
         finally
         {
@@ -138,7 +142,8 @@ public class GzipFilterDefaultTest
         try
         {
             tester.start();
-            tester.assertIsResponseGzipCompressed("file.txt");
+            HttpTester http = tester.assertIsResponseGzipCompressed("file.txt");
+            Assert.assertEquals("Accept-Encoding",http.getHeader("Vary"));
         }
         finally
         {
@@ -161,7 +166,8 @@ public class GzipFilterDefaultTest
         try
         {
             tester.start();
-            tester.assertIsResponseGzipCompressed("file.txt");
+            HttpTester http = tester.assertIsResponseGzipCompressed("file.txt");
+            Assert.assertEquals("Accept-Encoding",http.getHeader("Vary"));
         }
         finally
         {
@@ -184,7 +190,8 @@ public class GzipFilterDefaultTest
         try
         {
             tester.start();
-            tester.assertIsResponseGzipCompressed("file.txt");
+            HttpTester http = tester.assertIsResponseGzipCompressed("file.txt");
+            Assert.assertEquals("Accept-Encoding",http.getHeader("Vary"));
         }
         finally
         {
@@ -206,7 +213,8 @@ public class GzipFilterDefaultTest
         try
         {
             tester.start();
-            tester.assertIsResponseNotGzipCompressed("file.txt", filesize, HttpStatus.OK_200);
+            HttpTester http = tester.assertIsResponseNotGzipCompressed("file.txt", filesize, HttpStatus.OK_200);
+            Assert.assertEquals("Accept-Encoding",http.getHeader("Vary"));
         }
         finally
         {
@@ -219,7 +227,6 @@ public class GzipFilterDefaultTest
     {
         GzipTester tester = new GzipTester(testingdir, compressionType);
 
-        // Test content that is smaller than the buffer.
         int filesize = CompressedResponseWrapper.DEFAULT_BUFFER_SIZE * 4;
         tester.prepareServerFile("file.mp3",filesize);
         
@@ -229,7 +236,8 @@ public class GzipFilterDefaultTest
         try
         {
             tester.start();
-            tester.assertIsResponseNotGzipCompressed("file.mp3", filesize, HttpStatus.OK_200);
+            HttpTester http = tester.assertIsResponseNotGzipCompressed("file.mp3", filesize, HttpStatus.OK_200);
+            Assert.assertNull(http.getHeader("Vary"));
         }
         finally
         {
diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludableGzipFilterMinSizeTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludableGzipFilterMinSizeTest.java
index fba8bb2126..9dbfc1f30b 100644
--- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludableGzipFilterMinSizeTest.java
+++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/IncludableGzipFilterMinSizeTest.java
@@ -99,7 +99,7 @@ public class IncludableGzipFilterMinSizeTest
         tester.setGzipFilterClass(IncludableGzipFilter.class);
 
         FilterHolder holder = tester.setContentServlet(testServlet);
-        holder.setInitParameter("mimeTypes","application/soap+xml,text/javascript");
+        holder.setInitParameter("mimeTypes","application/soap+xml,text/javascript,application/x-javascript");
         holder.setInitParameter("minGzipSize", "2048");
         holder.setInitParameter("uncheckedPrintWriter","true");
 
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 de3aa98fb3..059d5b3a45 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
@@ -73,12 +73,12 @@ public class GzipTester
         // DOES NOT WORK IN WINDOWS - this.testdir.ensureEmpty();
     }
 
-    public void assertIsResponseGzipCompressed(String filename) throws Exception
+    public HttpTester assertIsResponseGzipCompressed(String filename) throws Exception
     {
-        assertIsResponseGzipCompressed(filename,filename);
+        return assertIsResponseGzipCompressed(filename,filename);
     }
 
-    public void assertIsResponseGzipCompressed(String requestedFilename, String serverFilename) throws Exception
+    public HttpTester assertIsResponseGzipCompressed(String requestedFilename, String serverFilename) throws Exception
     {
         System.err.printf("[GzipTester] requesting /context/%s%n",requestedFilename);
         HttpTester request = new HttpTester();
@@ -139,6 +139,8 @@ public class GzipTester
             IO.close(in);
             IO.close(bais);
         }
+        
+        return response;
     }
 
     /**
@@ -243,7 +245,7 @@ public class GzipTester
      *            passing -1 will disable the Content-Length assertion)
      * @throws Exception
      */
-    public void assertIsResponseNotGzipCompressed(String filename, int expectedFilesize, int status) throws Exception
+    public HttpTester assertIsResponseNotGzipCompressed(String filename, int expectedFilesize, int status) throws Exception
     {
         String uri = "/context/"+filename;
         HttpTester response = executeRequest(uri);
@@ -258,6 +260,8 @@ public class GzipTester
             String actual = readResponse(response);
             Assert.assertEquals("Expected response equals actual response",expectedResponse,actual);
         }
+        
+        return response;
     }