From 8243bd60b759d15ce9ab08a1b7aa9a2d6de4a27b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 9 Dec 2013 18:18:48 +1100 Subject: [PATCH] 423392 - GzipFilter without wrapping or blocking Handle the case where a gzip flush requires multiple iterations to go from finish() to finished() Better handling of exceptional cases --- .../jetty/servlets/AsyncGzipFilter.java | 14 +-- .../jetty/servlets/gzip/GzipHttpOutput.java | 85 ++++++++++++++----- .../org/eclipse/jetty/util/BufferUtil.java | 2 + .../eclipse/jetty/webapp/WebAppContext.java | 4 +- .../src/main/webapp/WEB-INF/web.xml | 5 +- 5 files changed, 78 insertions(+), 32 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 eb7df127944..eb383e11cf6 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 @@ -295,6 +295,7 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException { + LOG.debug("{} doFilter {}",this,req); HttpServletRequest request=(HttpServletRequest)req; HttpServletResponse response=(HttpServletResponse)res; @@ -364,21 +365,20 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory GzipHttpOutput cout=(GzipHttpOutput)out; - boolean exceptional=true; try { cout.mightCompress(this); super.doFilter(request,response,chain); - exceptional=false; } - finally + catch(Throwable e) { - LOG.debug("{} excepted {}",this,request); - if (exceptional && !response.isCommitted()) + LOG.debug("{} excepted {}",this,request,e); + if (!response.isCommitted()) { cout.resetBuffer(); - cout.noCompression(); + cout.noCompressionIfPossible(); } + throw e; } } @@ -459,7 +459,6 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory @Override public Deflater getDeflater(Request request, long content_length) { - String ua = getUserAgent(request); if (ua!=null && isExcludedAgent(ua)) { @@ -514,6 +513,7 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory @Override public void recycle(Deflater deflater) { + deflater.reset(); if (_deflater.get()==null) _deflater.set(deflater); 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 29f969e6301..2464924cc10 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 @@ -35,9 +35,12 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingNestedCallback; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; public class GzipHttpOutput extends HttpOutput { + public static Logger LOG = Log.getLogger(GzipHttpOutput.class); private final static HttpGenerator.CachedHttpField CONTENT_ENCODING_GZIP=new HttpGenerator.CachedHttpField(HttpHeader.CONTENT_ENCODING,"gzip"); private final static byte[] GZIP_HEADER = new byte[] { (byte)0x1f, (byte)0x8b, Deflater.DEFLATED, 0, 0, 0, 0, 0, 0, 0 }; @@ -49,7 +52,6 @@ public class GzipHttpOutput extends HttpOutput private GzipFactory _factory; private ByteBuffer _buffer; - public GzipHttpOutput(HttpChannel channel) { super(channel); @@ -58,6 +60,7 @@ public class GzipHttpOutput extends HttpOutput @Override public void reset() { + _state.set(GZState.NOT_COMPRESSING); super.reset(); } @@ -152,6 +155,7 @@ public class GzipHttpOutput extends HttpOutput int sc = response.getStatus(); if (sc>0 && (sc<200 || sc==204 || sc==205 || sc>=300)) { + LOG.debug("{} exclude by status {}",this,sc); noCompression(); super.write(content,complete,callback); return; @@ -164,6 +168,7 @@ public class GzipHttpOutput extends HttpOutput ct=MimeTypes.getContentTypeWithoutCharset(ct); if (_factory.isExcludedMimeType(StringUtil.asciiToLowerCase(ct))) { + LOG.debug("{} exclude by mimeType {}",this,ct); noCompression(); super.write(content,complete,callback); return; @@ -181,10 +186,11 @@ public class GzipHttpOutput extends HttpOutput if (content_length<0 && complete) content_length=content.remaining(); - _deflater = content.isDirect()?null:_factory.getDeflater(getHttpChannel().getRequest(),content_length); + _deflater = _factory.getDeflater(getHttpChannel().getRequest(),content_length); if (_deflater==null) { + LOG.debug("{} exclude no deflater",this); _state.set(GZState.NOT_COMPRESSING); super.write(content,complete,callback); return; @@ -201,6 +207,7 @@ public class GzipHttpOutput extends HttpOutput if (etag!=null) fields.put(HttpHeader.ETAG,etag.substring(0,etag.length()-1)+"--gzip\""); + LOG.debug("{} compressing {}",this,_deflater); _state.set(GZState.COMPRESSING); gzip(content,complete,callback); @@ -227,6 +234,28 @@ public class GzipHttpOutput extends HttpOutput } } + public void noCompressionIfPossible() + { + while (true) + { + switch (_state.get()) + { + case COMPRESSING: + case NOT_COMPRESSING: + return; + + case MIGHT_COMPRESS: + if (_state.compareAndSet(GZState.MIGHT_COMPRESS,GZState.NOT_COMPRESSING)) + return; + break; + + default: + throw new IllegalStateException(_state.get().toString()); + } + } + } + + public void mightCompress(GzipFactory factory) { while (true) @@ -236,12 +265,15 @@ public class GzipHttpOutput extends HttpOutput case NOT_COMPRESSING: _factory=factory; if (_state.compareAndSet(GZState.NOT_COMPRESSING,GZState.MIGHT_COMPRESS)) + { + LOG.debug("{} might compress",this); return; + } _factory=null; break; default: - throw new IllegalStateException(); + throw new IllegalStateException(_state.get().toString()); } } } @@ -275,24 +307,25 @@ public class GzipHttpOutput extends HttpOutput _deflater=null; getHttpChannel().getByteBufferPool().release(_buffer); _buffer=null; + return State.SUCCEEDED; } - return State.SUCCEEDED; + + if (!_complete) + return State.SUCCEEDED; } + BufferUtil.compact(_buffer); int off=_buffer.arrayOffset()+_buffer.limit(); int len=_buffer.capacity()-_buffer.limit()- (_complete?8:0); int produced=_deflater.deflate(_buffer.array(),off,len,Deflater.NO_FLUSH); _buffer.limit(_buffer.limit()+produced); boolean complete=_deflater.finished(); if (complete) - { addTrailer(); - _deflater.end(); // TODO recycle - } + superWrite(_buffer,complete,this); return State.SCHEDULED; } - } private class GzipBufferCB extends IteratingNestedCallback @@ -321,30 +354,40 @@ public class GzipHttpOutput extends HttpOutput _deflater=null; getHttpChannel().getByteBufferPool().release(_buffer); _buffer=null; + return State.SUCCEEDED; } - return State.SUCCEEDED; + + if (!_complete) + return State.SUCCEEDED; } - - BufferUtil.clearToFill(_input); - BufferUtil.put(_content,_input); - BufferUtil.flipToFlush(_input,0); + else + { + BufferUtil.clearToFill(_input); + BufferUtil.put(_content,_input); + BufferUtil.flipToFlush(_input,0); - byte[] array=_input.array(); - int off=_input.arrayOffset()+_input.position(); - int len=_input.remaining(); - _crc.update(array,off,len); - _deflater.setInput(array,off,len); - if (_complete && BufferUtil.isEmpty(_content)) - _deflater.finish(); + byte[] array=_input.array(); + int off=_input.arrayOffset()+_input.position(); + int len=_input.remaining(); + + _crc.update(array,off,len); + _deflater.setInput(array,off,len); + if (_complete && BufferUtil.isEmpty(_content)) + _deflater.finish(); + } } - + + BufferUtil.compact(_buffer); int off=_buffer.arrayOffset()+_buffer.limit(); int len=_buffer.capacity()-_buffer.limit() - (_complete?8:0); int produced=_deflater.deflate(_buffer.array(),off,len,Deflater.NO_FLUSH); + _buffer.limit(_buffer.limit()+produced); boolean complete=_deflater.finished(); + if (complete) addTrailer(); + superWrite(_buffer,complete,this); return State.SCHEDULED; } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java index a947c360a3b..7eb4f68c0dc 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java @@ -285,6 +285,8 @@ public class BufferUtil */ public static boolean compact(ByteBuffer buffer) { + if (buffer.position()==0) + return false; boolean full = buffer.limit() == buffer.capacity(); buffer.compact().flip(); return full && buffer.limit() < buffer.capacity(); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index 11e862e28c9..165972ff79c 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -115,7 +115,8 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL "org.eclipse.jetty.jndi.", // webapp cannot change naming classes "org.eclipse.jetty.jaas.", // webapp cannot change jaas classes "org.eclipse.jetty.websocket.", // webapp cannot change / replace websocket classes - "org.eclipse.jetty.servlet.DefaultServlet" // webapp cannot change default servlets + "org.eclipse.jetty.servlet.DefaultServlet", // webapp cannot change default servlets + "org.eclipse.jetty.servlets.AsyncGzipFilter" // special case for AsyncGzipFilter } ; // Server classes are classes that are hidden from being @@ -129,6 +130,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL "-org.eclipse.jetty.jaas.", // don't hide jaas classes "-org.eclipse.jetty.servlets.", // don't hide jetty servlets "-org.eclipse.jetty.servlet.DefaultServlet", // don't hide default servlet + "-org.eclipse.jetty.servlets.AsyncGzipFilter", // special case for AsyncGzipFilter "-org.eclipse.jetty.servlet.listener.", // don't hide useful listeners "-org.eclipse.jetty.websocket.", // don't hide websocket classes from webapps (allow webapp to use ones from system classloader) "org.eclipse.jetty." // hide other jetty classes diff --git a/tests/test-webapps/test-jetty-webapp/src/main/webapp/WEB-INF/web.xml b/tests/test-webapps/test-jetty-webapp/src/main/webapp/WEB-INF/web.xml index 3fd7646d942..9095a617f4f 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/webapp/WEB-INF/web.xml +++ b/tests/test-webapps/test-jetty-webapp/src/main/webapp/WEB-INF/web.xml @@ -55,7 +55,7 @@ GzipFilter - org.eclipse.jetty.servlets.IncludableGzipFilter + org.eclipse.jetty.servlets.AsyncGzipFilter true bufferSize @@ -63,7 +63,7 @@ mimeTypes - text/plain,application/xml + text/plain,application/xml,text/html minGzipSize @@ -92,7 +92,6 @@ *.txt - Login com.acme.LoginServlet