From 5165d4c7ade39b4107d4f09bf78ea79a1815fe30 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 12 Nov 2014 10:20:06 +1100 Subject: [PATCH 1/3] 450855 GzipFilter MIGHT_COMPRESS exception Disabled tests that tried to apply the GzipFilter to default async content generation. These tests will never work as GzipFilter wraps the response and the default async does not preserve wrappers. For the MIGHT_COMPRESS issue, improved the filter to handle the case that it has already been applied to a request/response and that the state is already mightCompress. --- .../eclipse/jetty/servlets/AsyncGzipFilter.java | 15 ++++++++++++++- .../jetty/servlets/gzip/GzipHttpOutput.java | 4 ++++ .../servlets/GzipFilterContentLengthTest.java | 8 +++++++- .../servlets/gzip/AsyncTimeoutCompleteWrite.java | 2 +- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/AsyncGzipFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/AsyncGzipFilter.java index 2cdeb9c60c9..7cc75b8c6d5 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/AsyncGzipFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/AsyncGzipFilter.java @@ -28,6 +28,7 @@ import java.util.StringTokenizer; import java.util.regex.Pattern; import java.util.zip.Deflater; +import javax.servlet.DispatcherType; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletContext; @@ -302,6 +303,19 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory LOG.debug("{} doFilter {}",this,req); HttpServletRequest request=(HttpServletRequest)req; HttpServletResponse response=(HttpServletResponse)res; + HttpChannel channel = HttpChannel.getCurrentHttpChannel(); + + // Have we already started compressing this response? + if (req.getDispatcherType()!=DispatcherType.REQUEST) + { + HttpOutput out = channel.getResponse().getHttpOutput(); + if (out instanceof GzipHttpOutput && ((GzipHttpOutput)out).mightCompress()) + { + LOG.debug("{} already might compress {}",this,request); + super.doFilter(request,response,chain); + return; + } + } // If not a supported method or it is an Excluded URI or an excluded UA - no Vary because no matter what client, this URI is always excluded String requestURI = request.getRequestURI(); @@ -368,7 +382,6 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory request.setAttribute(ETAG,etag.replace(ETAG_GZIP,"")); } - HttpChannel channel = HttpChannel.getCurrentHttpChannel(); HttpOutput out = channel.getResponse().getHttpOutput(); if (!(out instanceof GzipHttpOutput)) { diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/GzipHttpOutput.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/GzipHttpOutput.java index d56b2e55f5d..9337b6ccedb 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/GzipHttpOutput.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/gzip/GzipHttpOutput.java @@ -248,6 +248,10 @@ public class GzipHttpOutput extends HttpOutput } } + public boolean mightCompress() + { + return _state.get()==GZState.MIGHT_COMPRESS; + } public void mightCompress(GzipFactory factory) { 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 3c1f5d89125..0d0e5cf79e6 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 @@ -49,6 +49,7 @@ 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.junit.Assert; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -63,7 +64,6 @@ import org.junit.runners.Parameterized.Parameters; * @see http://bugs.eclipse.org/354014 */ @RunWith(Parameterized.class) -@Ignore public class GzipFilterContentLengthTest { @Rule @@ -167,6 +167,8 @@ public class GzipFilterContentLengthTest @Test public void testAsyncTimeoutCompleteWrite_Default() throws Exception { + if (expectCompressed && gzipFilterClass==GzipFilter.class) + return; // Default startAsync will never work with GzipFilter, which needs wrapping testWithGzip(AsyncTimeoutCompleteWrite.Default.class); } @@ -187,6 +189,8 @@ public class GzipFilterContentLengthTest @Test public void testAsyncTimeoutDispatchWrite_Default() throws Exception { + if (expectCompressed && gzipFilterClass==GzipFilter.class) + return; // Default startAsync will never work with GzipFilter, which needs wrapping testWithGzip(AsyncTimeoutDispatchWrite.Default.class); } @@ -207,6 +211,8 @@ public class GzipFilterContentLengthTest @Test public void testAsyncScheduledDispatchWrite_Default() throws Exception { + if (expectCompressed && gzipFilterClass==GzipFilter.class) + return; // Default startAsync will never work with GzipFilter, which needs wrapping testWithGzip(AsyncScheduledDispatchWrite.Default.class); } diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/AsyncTimeoutCompleteWrite.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/AsyncTimeoutCompleteWrite.java index a9a3811818c..197f140b643 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/AsyncTimeoutCompleteWrite.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/AsyncTimeoutCompleteWrite.java @@ -90,7 +90,7 @@ public abstract class AsyncTimeoutCompleteWrite extends TestDirContentServlet im String fileName = request.getServletPath(); request.setAttribute("filename",fileName); ctx.addListener(this); - ctx.setTimeout(200); + ctx.setTimeout(20); // Setup indication of a redispatch (which this scenario shouldn't do) request.setAttribute(this.getClass().getName(),ctx); From 85811dad9b0bcfe0685ed1f021a986a229294c81 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 12 Nov 2014 10:41:48 +1100 Subject: [PATCH 2/3] 450873 Disable tests that downcaste wrapped GzipFilterResponses --- .../eclipse/jetty/servlets/GzipFilterContentLengthTest.java | 3 ++- .../jetty/servlets/gzip/TestServletBufferTypeLengthWrite.java | 2 +- 2 files changed, 3 insertions(+), 2 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 0d0e5cf79e6..2a4c8eb456d 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 @@ -344,9 +344,10 @@ public class GzipFilterContentLengthTest * @see http://bugs.eclipse.org/450873 */ @Test - @Ignore public void testHttpOutputWrite() throws Exception { + if (gzipFilterClass == GzipFilter.class) + return; // Can't downcaste output stream when wrapper is used testWithGzip(TestServletBufferTypeLengthWrite.class); } } diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/TestServletBufferTypeLengthWrite.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/TestServletBufferTypeLengthWrite.java index 16a66ddce11..80f5f133667 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/TestServletBufferTypeLengthWrite.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/gzip/TestServletBufferTypeLengthWrite.java @@ -61,7 +61,7 @@ public class TestServletBufferTypeLengthWrite extends TestDirContentServlet response.setHeader("ETag","W/etag-"+fileName); response.setContentLength(dataBytes.length); - + ((HttpOutput)out).write(ByteBuffer.wrap(dataBytes).asReadOnlyBuffer()); } } From 5fe992bf04bb79e03c2cd83b1cf92aaf951b415e Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 12 Nov 2014 10:46:14 +1100 Subject: [PATCH 3/3] 450855 GzipFilter MIGHT_COMPRESS exception Re-enabled some default GzipFilter tests. These will work because the filter rewraps the response on the dispatch. It is only the timeout test that will not work with default. --- .../eclipse/jetty/servlets/GzipFilterContentLengthTest.java | 4 ---- 1 file changed, 4 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 2a4c8eb456d..ba9a6912122 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 @@ -189,8 +189,6 @@ public class GzipFilterContentLengthTest @Test public void testAsyncTimeoutDispatchWrite_Default() throws Exception { - if (expectCompressed && gzipFilterClass==GzipFilter.class) - return; // Default startAsync will never work with GzipFilter, which needs wrapping testWithGzip(AsyncTimeoutDispatchWrite.Default.class); } @@ -211,8 +209,6 @@ public class GzipFilterContentLengthTest @Test public void testAsyncScheduledDispatchWrite_Default() throws Exception { - if (expectCompressed && gzipFilterClass==GzipFilter.class) - return; // Default startAsync will never work with GzipFilter, which needs wrapping testWithGzip(AsyncScheduledDispatchWrite.Default.class); }