From 030ff927209783d782fdc5def137975c741e112e Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 10 Nov 2014 12:05:23 -0700 Subject: [PATCH] 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 --- .../servlets/GzipFilterContentLengthTest.java | 334 ++++++++++-------- .../jetty/servlets/GzipFilterLayeredTest.java | 98 +++++ 2 files changed, 287 insertions(+), 145 deletions(-) diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterContentLengthTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterContentLengthTest.java index 4cd93980c9b..2758f2a8156 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterContentLengthTest.java +++ b/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 java.util.Arrays; -import java.util.List; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; -import javax.servlet.Filter; -import javax.servlet.Servlet; +import java.io.File; +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.Assert; +import org.junit.Ignore; 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. - *

- * 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 http://bugs.eclipse.org/354014 - *

- * 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 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; - - public GzipFilterContentLengthTest(Class testFilter,Class testServlet, String compressionType) + @Parameters(name = "{0} bytes - {1} - compressed: {2} - type: {3} - filter: {4}") + public static List data() { - this.testFilter = testFilter; - this.testServlet = testServlet; - this.compressionType = compressionType; + List ret = new ArrayList(); + + 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 - public TestingDir testingdir = new TestingDir(); - - private Class testFilter; - private Class testServlet; - - private void assertIsGzipCompressed(String filename, int filesize) throws Exception + @Parameter(0) + public int fileSize; + @Parameter(1) + public String fileName; + @Parameter(2) + public boolean expectCompressed; + @Parameter(3) + public String compressionType; + @Parameter(4) + public Class gzipFilterClass; + + private void testWithGzip(Class contentServlet) throws Exception { - GzipTester tester = new GzipTester(testingdir, compressionType); - tester.setGzipFilterClass(testFilter); - - File testfile = tester.prepareServerFile(testServlet.getSimpleName() + "-" + filename,filesize); - - FilterHolder holder = tester.setContentServlet(testServlet); - holder.setInitParameter("mimeTypes","text/plain"); + GzipTester tester = new GzipTester(testingdir, GzipFilter.GZIP); + + // Add AsyncGzip Filter + FilterHolder gzipHolder = new FilterHolder(gzipFilterClass); + 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 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()); - } - finally - { - tester.stop(); - } - } - - private void assertIsNotGzipCompressed(String filename, int filesize) throws Exception - { - GzipTester tester = new GzipTester(testingdir, compressionType); - tester.setGzipFilterClass(testFilter); - - File testfile = tester.prepareServerFile(testServlet.getSimpleName() + "-" + filename,filesize); - - FilterHolder holder = tester.setContentServlet(testServlet); - holder.setInitParameter("mimeTypes","text/plain"); - - try - { - tester.start(); - HttpTester.Response response = tester.assertIsResponseNotGzipCompressed("GET",testfile.getName(),filesize,HttpStatus.OK_200); - Assert.assertThat(response.get("ETAG"),Matchers.startsWith("W/etag-")); + + 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 { @@ -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 - public void testIsGzipCompressedMedium() throws Exception - { - 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 - * + * Test with content servlet that does: + * 1) setHeader(content-length) + * 2) getOutputStream() + * 3) setHeader(content-type) + * 4) outputStream.write() + * * @see http://bugs.eclipse.org/354014 */ @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 - * that are not being compressed encountered when using GzipFilter - * + * Test with content servlet that does: + * 1) setHeader(content-length) + * 2) setHeader(content-type) + * 3) getOutputStream() + * 4) outputStream.write() + * * @see http://bugs.eclipse.org/354014 */ @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 - * that are not being compressed encountered when using GzipFilter - * + * Test with content servlet that does: + * 1) getOutputStream() + * 2) setHeader(content-length) + * 3) setHeader(content-type) + * 4) outputStream.write() + * * @see http://bugs.eclipse.org/354014 */ @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 - * that were not being compressed encountered when using GzipFilter - * + * Test with content servlet that does: + * 1) getOutputStream() + * 2) setHeader(content-length) + * 3) setHeader(content-type) + * 4) outputStream.write() (with frequent response flush) + * * @see http://bugs.eclipse.org/354014 */ @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 http://bugs.eclipse.org/354014 + */ + @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 http://bugs.eclipse.org/354014 + */ + @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 http://bugs.eclipse.org/354014 + */ + @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 http://bugs.eclipse.org/450873 + */ + @Test + @Ignore + public void testHttpOutputWrite() throws Exception + { + testWithGzip(TestServletBufferTypeLengthWrite.class); } } diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterLayeredTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterLayeredTest.java index c36ac1a546d..dc3eaa80f68 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipFilterLayeredTest.java +++ b/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(); + } + } }