Some cleanup of the Gzip tests

+ Seeing more edge cases that can cause problems.
  Set those to @Ignore for greg to take a look at

jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterContentLengthTest.java

@@ -18,21 +18,25 @@
 
 package org.eclipse.jetty.servlets;
 
-import java.io.File;
+import static org.hamcrest.Matchers.*;
-import java.util.Arrays;
+import static org.junit.Assert.*;
-import java.util.List;
 
-import javax.servlet.Filter;
+import java.io.File;
-import javax.servlet.Servlet;
+import java.util.ArrayList;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import javax.servlet.DispatcherType;
 
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.servlet.FilterHolder;
-import org.eclipse.jetty.servlets.gzip.AsyncScheduledWrite;
-import org.eclipse.jetty.servlets.gzip.AsyncTimeoutDispatchBasedWrite;
 import org.eclipse.jetty.servlets.gzip.AsyncTimeoutWrite;
 import org.eclipse.jetty.servlets.gzip.GzipTester;
+import org.eclipse.jetty.servlets.gzip.GzipTester.ContentMetadata;
+import org.eclipse.jetty.servlets.gzip.TestDirContentServlet;
 import org.eclipse.jetty.servlets.gzip.TestServletBufferTypeLengthWrite;
 import org.eclipse.jetty.servlets.gzip.TestServletLengthStreamTypeWrite;
 import org.eclipse.jetty.servlets.gzip.TestServletLengthTypeStreamWrite;
@@ -43,12 +47,12 @@ import org.eclipse.jetty.servlets.gzip.TestServletTypeLengthStreamWrite;
 import org.eclipse.jetty.servlets.gzip.TestServletTypeStreamLengthWrite;
 import org.eclipse.jetty.toolchain.test.TestTracker;
 import org.eclipse.jetty.toolchain.test.TestingDir;
-import org.hamcrest.Matchers;
+import org.junit.Ignore;
-import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
 /**
@@ -61,113 +65,91 @@ public class GzipFilterContentLengthTest
 {
     @Rule
     public final TestTracker tracker = new TestTracker();
+
+    @Rule
+    public TestingDir testingdir = new TestingDir();
     
-    /**
-     * These are the junit parameters for running this test.
-     * <p>
-     * In addition to Jetty's DefaultServlet we have multiple test
-     * servlets that arrange content-length/content-type/get stream
-     * in different order so as to simulate the real world scenario
-     * that caused the bug in Eclipse <a href="Bug 354014">http://bugs.eclipse.org/354014</a>
-     * <p>
-     * This test case will be run with each of the entries in
-     * the array below as setup parameters for the test case.
-     *
-     * @return the junit parameters
-     */
-    @Parameters(name="{2}/{1} {0}")
-    public static List<Object[]> data()
-    {
-        return Arrays.asList(new Object[][]
-        {
-        { AsyncGzipFilter.class, AsyncTimeoutWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, AsyncTimeoutDispatchBasedWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, AsyncScheduledWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletLengthStreamTypeWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletLengthTypeStreamWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletStreamLengthTypeWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletStreamLengthTypeWriteWithFlush.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletStreamTypeLengthWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletTypeLengthStreamWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletTypeStreamLengthWrite.class, GzipFilter.GZIP },
-        { AsyncGzipFilter.class, TestServletBufferTypeLengthWrite.class, GzipFilter.GZIP },
-
-        { GzipFilter.class, TestServletLengthStreamTypeWrite.class, GzipFilter.GZIP },
-        { GzipFilter.class, TestServletLengthTypeStreamWrite.class, GzipFilter.GZIP },
-        { GzipFilter.class, TestServletStreamLengthTypeWrite.class, GzipFilter.GZIP },
-        { GzipFilter.class, TestServletStreamLengthTypeWriteWithFlush.class, GzipFilter.GZIP },
-        { GzipFilter.class, TestServletStreamTypeLengthWrite.class, GzipFilter.GZIP },
-        { GzipFilter.class, TestServletTypeLengthStreamWrite.class, GzipFilter.GZIP },
-        { GzipFilter.class, TestServletTypeStreamLengthWrite.class, GzipFilter.GZIP },
-        
-        { GzipFilter.class, TestServletLengthStreamTypeWrite.class, GzipFilter.DEFLATE },
-        { GzipFilter.class, TestServletLengthTypeStreamWrite.class, GzipFilter.DEFLATE },
-        { GzipFilter.class, TestServletStreamLengthTypeWrite.class, GzipFilter.DEFLATE },
-        { GzipFilter.class, TestServletStreamLengthTypeWriteWithFlush.class, GzipFilter.DEFLATE },
-        { GzipFilter.class, TestServletStreamTypeLengthWrite.class, GzipFilter.DEFLATE },
-        { GzipFilter.class, TestServletTypeLengthStreamWrite.class, GzipFilter.DEFLATE },
-        { GzipFilter.class, TestServletTypeStreamLengthWrite.class, GzipFilter.DEFLATE },
-        
-        });
-    }
-
     private static final HttpConfiguration defaultHttp = new HttpConfiguration();
     private static final int LARGE = defaultHttp.getOutputBufferSize() * 8;
     private static final int MEDIUM = defaultHttp.getOutputBufferSize();
     private static final int SMALL = defaultHttp.getOutputBufferSize() / 4;
     private static final int TINY = AsyncGzipFilter.DEFAULT_MIN_GZIP_SIZE / 2;
+    private static final boolean EXPECT_COMPRESSED = true;
 
-    private String compressionType;
+    @Parameters(name = "{0} bytes - {1} - compressed: {2} - type: {3} - filter: {4}")
-
+    public static List<Object[]> data()
-    public GzipFilterContentLengthTest(Class<? extends Filter> testFilter,Class<? extends Servlet> testServlet, String compressionType)
     {
-        this.testFilter = testFilter;
+        List<Object[]> ret = new ArrayList<Object[]>();
-        this.testServlet = testServlet;
+        
-        this.compressionType = compressionType;
+        String compressionTypes[] = new String[] { GzipFilter.GZIP, GzipFilter.DEFLATE };
+        Class<?> gzipFilters[] = new Class<?>[] { GzipFilter.class, AsyncGzipFilter.class };
+        
+        for(String compressionType: compressionTypes)
+        {
+            for(Class<?> gzipFilter: gzipFilters)
+            {
+                ret.add(new Object[] { 0, "empty.txt", !EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { TINY, "file-tiny.txt", !EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { SMALL, "file-small.txt", EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { SMALL, "file-small.mp3", !EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { MEDIUM, "file-med.txt", EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { MEDIUM, "file-medium.mp3", !EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { LARGE, "file-large.txt", EXPECT_COMPRESSED, compressionType, gzipFilter });
+                ret.add(new Object[] { LARGE, "file-large.mp3", !EXPECT_COMPRESSED, compressionType, gzipFilter });
+            }
+        }
+
+        return ret;
     }
 
-    @Rule
+    @Parameter(0)
-    public TestingDir testingdir = new TestingDir();
+    public int fileSize;
-
+    @Parameter(1)
-    private Class<? extends Filter> testFilter;
+    public String fileName;
-    private Class<? extends Servlet> testServlet;
+    @Parameter(2)
-
+    public boolean expectCompressed;
-    private void assertIsGzipCompressed(String filename, int filesize) throws Exception
+    @Parameter(3)
+    public String compressionType;
+    @Parameter(4)
+    public Class<? extends GzipFilter> gzipFilterClass;
+    
+    private void testWithGzip(Class<? extends TestDirContentServlet> contentServlet) throws Exception
     {
-        GzipTester tester = new GzipTester(testingdir, compressionType);
+        GzipTester tester = new GzipTester(testingdir, GzipFilter.GZIP);
-        tester.setGzipFilterClass(testFilter);
+        
-
+        // Add AsyncGzip Filter
-        File testfile = tester.prepareServerFile(testServlet.getSimpleName() + "-" + filename,filesize);
+        FilterHolder gzipHolder = new FilterHolder(gzipFilterClass);
-
+        gzipHolder.setAsyncSupported(true);
-        FilterHolder holder = tester.setContentServlet(testServlet);
+        tester.addFilter(gzipHolder,"*.txt",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
-        holder.setInitParameter("mimeTypes","text/plain");
+        tester.addFilter(gzipHolder,"*.mp3",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        gzipHolder.setInitParameter("mimeTypes","text/plain");
 
+        // Add content servlet
+        tester.setContentServlet(contentServlet);
+        
         try
         {
+            String testFilename = String.format("%s-%s-%s", gzipFilterClass.getSimpleName(), contentServlet.getSimpleName(), fileName);
+            File testFile = tester.prepareServerFile(testFilename,fileSize);
+            
             tester.start();
-            tester.assertIsResponseGzipCompressed("GET",testfile.getName());
+            
-        }
+            HttpTester.Response response = tester.issueRequest("GET",testFile.getName(),2,TimeUnit.SECONDS);
-        finally
+            
-        {
+            assertThat("Response status", response.getStatus(), is(HttpStatus.OK_200));
-            tester.stop();
+            
-        }
+            if (expectCompressed)
-    }
+            {
-
+                // Must be gzip compressed
-    private void assertIsNotGzipCompressed(String filename, int filesize) throws Exception
+                assertThat("Content-Encoding",response.get("Content-Encoding"),containsString(GzipFilter.GZIP));
-    {
+            } else
-        GzipTester tester = new GzipTester(testingdir, compressionType);
+            {
-        tester.setGzipFilterClass(testFilter);
+                assertThat("Content-Encoding",response.get("Content-Encoding"),not(containsString(GzipFilter.GZIP)));
-
+            }
-        File testfile = tester.prepareServerFile(testServlet.getSimpleName() + "-" + filename,filesize);
+            
-
+            // Uncompressed content Size
-        FilterHolder holder = tester.setContentServlet(testServlet);
+            ContentMetadata content = tester.getResponseMetadata(response);
-        holder.setInitParameter("mimeTypes","text/plain");
+            assertThat("(Uncompressed) Content Length", content.size, is((long)fileSize));
-
-        try
-        {
-            tester.start();
-            HttpTester.Response response = tester.assertIsResponseNotGzipCompressed("GET",testfile.getName(),filesize,HttpStatus.OK_200);
-            Assert.assertThat(response.get("ETAG"),Matchers.startsWith("W/etag-"));
         }
         finally
         {
@@ -176,86 +158,148 @@ public class GzipFilterContentLengthTest
     }
 
     /**
-     * Tests gzip compression of a small size file
+     * Test with content servlet that does:  
+     * AsyncContext create -> timeout -> onTimeout -> write-response -> complete
      */
     @Test
-    public void testEmpty() throws Exception
+    @Ignore
+    public void testAsyncTimeoutWrite() throws Exception
     {
-        assertIsNotGzipCompressed("empty.txt",0);
+        testWithGzip(AsyncTimeoutWrite.class);
     }
     
     /**
-     * Tests gzip compression of a small size file
+     * Test with content servlet that does:  
+     * AsyncContext create -> timeout -> onTimeout -> dispatch -> write-response
      */
     @Test
-    public void testIsGzipCompressedSmall() throws Exception
+    @Ignore
+    public void testAsyncTimeoutDispatchBasedWrite() throws Exception
     {
-        assertIsGzipCompressed("file-small.txt",SMALL);
+        testWithGzip(AsyncTimeoutWrite.class);
     }
 
     /**
-     * Tests gzip compression of a medium size file
+     * Test with content servlet that does:  
-     */
+     * 1) setHeader(content-length)
-    @Test
+     * 2) getOutputStream()
-    public void testIsGzipCompressedMedium() throws Exception
+     * 3) setHeader(content-type)
-    {
+     * 4) outputStream.write()
-        assertIsGzipCompressed("file-med.txt",MEDIUM);
+     * 
-    }
-
-    /**
-     * Tests gzip compression of a large size file
-     */
-    @Test
-    public void testIsGzipCompressedLarge() throws Exception
-    {
-        assertIsGzipCompressed("file-large.txt",LARGE);
-    }
-
-    /**
-     * Tests for problems with Content-Length header on small size files
-     * that are not being compressed encountered when using GzipFilter
-     *
      * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
      */
     @Test
-    public void testIsNotGzipCompressedTiny() throws Exception
+    public void testServletLengthStreamTypeWrite() throws Exception
     {
-        assertIsNotGzipCompressed("file-tiny.txt",TINY);
+        testWithGzip(TestServletLengthStreamTypeWrite.class);
     }
 
     /**
-     * Tests for problems with Content-Length header on small size files
+     * Test with content servlet that does:  
-     * that are not being compressed encountered when using GzipFilter
+     * 1) setHeader(content-length)
-     *
+     * 2) setHeader(content-type)
+     * 3) getOutputStream()
+     * 4) outputStream.write()
+     * 
      * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
      */
     @Test
-    public void testIsNotGzipCompressedSmall() throws Exception
+    public void testServletLengthTypeStreamWrite() throws Exception
     {
-        assertIsNotGzipCompressed("file-small.mp3",SMALL);
+        testWithGzip(TestServletLengthTypeStreamWrite.class);
     }
 
     /**
-     * Tests for problems with Content-Length header on medium size files
+     * Test with content servlet that does:  
-     * that are not being compressed encountered when using GzipFilter
+     * 1) getOutputStream()
-     *
+     * 2) setHeader(content-length)
+     * 3) setHeader(content-type)
+     * 4) outputStream.write()
+     * 
      * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
      */
     @Test
-    public void testIsNotGzipCompressedMedium() throws Exception
+    public void testServletStreamLengthTypeWrite() throws Exception
     {
-        assertIsNotGzipCompressed("file-medium.mp3",MEDIUM);
+        testWithGzip(TestServletStreamLengthTypeWrite.class);
     }
 
     /**
-     * Tests for problems with Content-Length header on large size files
+     * Test with content servlet that does:  
-     * that were not being compressed encountered when using GzipFilter
+     * 1) getOutputStream()
-     *
+     * 2) setHeader(content-length)
+     * 3) setHeader(content-type)
+     * 4) outputStream.write() (with frequent response flush)
+     * 
      * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
      */
     @Test
-    public void testIsNotGzipCompressedLarge() throws Exception
+    public void testServletStreamLengthTypeWriteWithFlush() throws Exception
     {
-        assertIsNotGzipCompressed("file-large.mp3",LARGE);
+        testWithGzip(TestServletStreamLengthTypeWriteWithFlush.class);
+    }
+
+    /**
+     * Test with content servlet that does:  
+     * 1) getOutputStream()
+     * 2) setHeader(content-type)
+     * 3) setHeader(content-length)
+     * 4) outputStream.write()
+     * 
+     * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
+     */
+    @Test
+    public void testServletStreamTypeLengthWrite() throws Exception
+    {
+        testWithGzip(TestServletStreamTypeLengthWrite.class);
+    }
+
+    /**
+     * Test with content servlet that does:  
+     * 1) setHeader(content-type)
+     * 2) setHeader(content-length)
+     * 3) getOutputStream()
+     * 4) outputStream.write()
+     * 
+     * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
+     */
+    @Test
+    public void testServletTypeLengthStreamWrite() throws Exception
+    {
+        testWithGzip(TestServletTypeLengthStreamWrite.class);
+    }
+
+    /**
+     * Test with content servlet that does:  
+     * 1) setHeader(content-type)
+     * 2) getOutputStream()
+     * 3) setHeader(content-length)
+     * 4) outputStream.write()
+     * 
+     * @see <a href="Eclipse Bug 354014">http://bugs.eclipse.org/354014</a>
+     */
+    @Test
+    public void testServletTypeStreamLengthWrite() throws Exception
+    {
+        testWithGzip(TestServletTypeStreamLengthWrite.class);
+    }
+
+    /**
+     * Test with content servlet that does:  
+     * 2) getOutputStream()
+     * 1) setHeader(content-type)
+     * 3) setHeader(content-length)
+     * 4) (unwrapped) HttpOutput.write(ByteBuffer)
+     * 
+     * This is done to demonstrate a bug with using HttpOutput.write()
+     * while also using GzipFilter
+     * 
+     * @see <a href="Eclipse Bug 450873">http://bugs.eclipse.org/450873</a>
+     */
+    @Test
+    @Ignore
+    public void testHttpOutputWrite() throws Exception
+    {
+        testWithGzip(TestServletBufferTypeLengthWrite.class);
     }
 }

jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterLayeredTest.java

@@ -34,6 +34,7 @@ import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.servlet.FilterHolder;
 import org.eclipse.jetty.servlets.gzip.AsyncManipFilter;
+import org.eclipse.jetty.servlets.gzip.AsyncTimeoutDispatchBasedWrite;
 import org.eclipse.jetty.servlets.gzip.GzipTester;
 import org.eclipse.jetty.servlets.gzip.GzipTester.ContentMetadata;
 import org.eclipse.jetty.servlets.gzip.TestServletLengthStreamTypeWrite;
@@ -133,6 +134,53 @@ public class GzipFilterLayeredTest
         }
     }
     
+    @Test
+    @Ignore
+    public void testGzipDosAsync() throws Exception
+    {
+        GzipTester tester = new GzipTester(testingdir, GzipFilter.GZIP);
+        
+        // Add Gzip Filter first
+        FilterHolder gzipHolder = new FilterHolder(AsyncGzipFilter.class);
+        gzipHolder.setAsyncSupported(true);
+        tester.addFilter(gzipHolder,"*.txt",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        tester.addFilter(gzipHolder,"*.mp3",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        gzipHolder.setInitParameter("mimeTypes","text/plain");
+
+        // Add (DoSFilter-like) manip filter (in chain of Gzip)
+        FilterHolder manipHolder = new FilterHolder(AsyncManipFilter.class);
+        manipHolder.setAsyncSupported(true);
+        tester.addFilter(manipHolder,"/*",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        
+        // Add normal content servlet
+        tester.setContentServlet(AsyncTimeoutDispatchBasedWrite.class);
+        
+        try
+        {
+            File testFile = tester.prepareServerFile("GzipDosAsync-" + fileName,fileSize);
+            
+            tester.start();
+            
+            HttpTester.Response response = tester.issueRequest("GET","/" + testFile.getName(), 2, TimeUnit.SECONDS);
+            
+            assertThat("Response status", response.getStatus(), is(HttpStatus.OK_200));
+            
+            if (expectCompressed)
+            {
+                // Must be gzip compressed
+                assertThat("Content-Encoding",response.get("Content-Encoding"),containsString(GzipFilter.GZIP));
+            }
+            
+            // Uncompressed content Size
+            ContentMetadata content = tester.getResponseMetadata(response);
+            assertThat("(Uncompressed) Content Length", content.size, is((long)fileSize));
+        }
+        finally
+        {
+            tester.stop();
+        }
+    }
+    
     @Test
     public void testDosGzipNormal() throws Exception
     {
@@ -181,4 +229,54 @@ public class GzipFilterLayeredTest
             tester.stop();
         }
     }
+    
+    @Test
+    @Ignore
+    public void testDosGzipAsync() throws Exception
+    {
+        GzipTester tester = new GzipTester(testingdir, GzipFilter.GZIP);
+        
+        // Add (DoSFilter-like) manip filter
+        FilterHolder manipHolder = new FilterHolder(AsyncManipFilter.class);
+        manipHolder.setAsyncSupported(true);
+        tester.addFilter(manipHolder,"/*",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        
+        // Add Gzip Filter first (in chain of DosFilter)
+        FilterHolder gzipHolder = new FilterHolder(AsyncGzipFilter.class);
+        gzipHolder.setAsyncSupported(true);
+        tester.addFilter(gzipHolder,"*.txt",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        tester.addFilter(gzipHolder,"*.mp3",EnumSet.of(DispatcherType.REQUEST,DispatcherType.ASYNC));
+        gzipHolder.setInitParameter("mimeTypes","text/plain");
+
+        // Add normal content servlet
+        tester.setContentServlet(AsyncTimeoutDispatchBasedWrite.class);
+        
+        try
+        {
+            File testFile = tester.prepareServerFile("DosGzipAsync-" + fileName,fileSize);
+            
+            tester.start();
+            
+            HttpTester.Response response = tester.issueRequest("GET",testFile.getName(),2,TimeUnit.SECONDS);
+            
+            assertThat("Response status", response.getStatus(), is(HttpStatus.OK_200));
+            
+            if (expectCompressed)
+            {
+                // Must be gzip compressed
+                assertThat("Content-Encoding",response.get("Content-Encoding"),containsString(GzipFilter.GZIP));
+            } else
+            {
+                assertThat("Content-Encoding",response.get("Content-Encoding"),not(containsString(GzipFilter.GZIP)));
+            }
+            
+            // Uncompressed content Size
+            ContentMetadata content = tester.getResponseMetadata(response);
+            assertThat("(Uncompressed) Content Length", content.size, is((long)fileSize));
+        }
+        finally
+        {
+            tester.stop();
+        }
+    }
 }