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
This commit is contained in:
Greg Wilkins 2013-12-09 18:18:48 +11:00
parent 39d21aee0a
commit 8243bd60b7
5 changed files with 78 additions and 32 deletions

View File

@ -295,6 +295,7 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory
public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
throws IOException, ServletException throws IOException, ServletException
{ {
LOG.debug("{} doFilter {}",this,req);
HttpServletRequest request=(HttpServletRequest)req; HttpServletRequest request=(HttpServletRequest)req;
HttpServletResponse response=(HttpServletResponse)res; HttpServletResponse response=(HttpServletResponse)res;
@ -364,21 +365,20 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory
GzipHttpOutput cout=(GzipHttpOutput)out; GzipHttpOutput cout=(GzipHttpOutput)out;
boolean exceptional=true;
try try
{ {
cout.mightCompress(this); cout.mightCompress(this);
super.doFilter(request,response,chain); super.doFilter(request,response,chain);
exceptional=false;
} }
finally catch(Throwable e)
{ {
LOG.debug("{} excepted {}",this,request); LOG.debug("{} excepted {}",this,request,e);
if (exceptional && !response.isCommitted()) if (!response.isCommitted())
{ {
cout.resetBuffer(); cout.resetBuffer();
cout.noCompression(); cout.noCompressionIfPossible();
} }
throw e;
} }
} }
@ -459,7 +459,6 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory
@Override @Override
public Deflater getDeflater(Request request, long content_length) public Deflater getDeflater(Request request, long content_length)
{ {
String ua = getUserAgent(request); String ua = getUserAgent(request);
if (ua!=null && isExcludedAgent(ua)) if (ua!=null && isExcludedAgent(ua))
{ {
@ -514,6 +513,7 @@ public class AsyncGzipFilter extends UserAgentFilter implements GzipFactory
@Override @Override
public void recycle(Deflater deflater) public void recycle(Deflater deflater)
{ {
deflater.reset();
if (_deflater.get()==null) if (_deflater.get()==null)
_deflater.set(deflater); _deflater.set(deflater);

View File

@ -35,9 +35,12 @@ import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IteratingNestedCallback; import org.eclipse.jetty.util.IteratingNestedCallback;
import org.eclipse.jetty.util.StringUtil; 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 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 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 }; 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 GzipFactory _factory;
private ByteBuffer _buffer; private ByteBuffer _buffer;
public GzipHttpOutput(HttpChannel<?> channel) public GzipHttpOutput(HttpChannel<?> channel)
{ {
super(channel); super(channel);
@ -58,6 +60,7 @@ public class GzipHttpOutput extends HttpOutput
@Override @Override
public void reset() public void reset()
{ {
_state.set(GZState.NOT_COMPRESSING);
super.reset(); super.reset();
} }
@ -152,6 +155,7 @@ public class GzipHttpOutput extends HttpOutput
int sc = response.getStatus(); int sc = response.getStatus();
if (sc>0 && (sc<200 || sc==204 || sc==205 || sc>=300)) if (sc>0 && (sc<200 || sc==204 || sc==205 || sc>=300))
{ {
LOG.debug("{} exclude by status {}",this,sc);
noCompression(); noCompression();
super.write(content,complete,callback); super.write(content,complete,callback);
return; return;
@ -164,6 +168,7 @@ public class GzipHttpOutput extends HttpOutput
ct=MimeTypes.getContentTypeWithoutCharset(ct); ct=MimeTypes.getContentTypeWithoutCharset(ct);
if (_factory.isExcludedMimeType(StringUtil.asciiToLowerCase(ct))) if (_factory.isExcludedMimeType(StringUtil.asciiToLowerCase(ct)))
{ {
LOG.debug("{} exclude by mimeType {}",this,ct);
noCompression(); noCompression();
super.write(content,complete,callback); super.write(content,complete,callback);
return; return;
@ -181,10 +186,11 @@ public class GzipHttpOutput extends HttpOutput
if (content_length<0 && complete) if (content_length<0 && complete)
content_length=content.remaining(); content_length=content.remaining();
_deflater = content.isDirect()?null:_factory.getDeflater(getHttpChannel().getRequest(),content_length); _deflater = _factory.getDeflater(getHttpChannel().getRequest(),content_length);
if (_deflater==null) if (_deflater==null)
{ {
LOG.debug("{} exclude no deflater",this);
_state.set(GZState.NOT_COMPRESSING); _state.set(GZState.NOT_COMPRESSING);
super.write(content,complete,callback); super.write(content,complete,callback);
return; return;
@ -201,6 +207,7 @@ public class GzipHttpOutput extends HttpOutput
if (etag!=null) if (etag!=null)
fields.put(HttpHeader.ETAG,etag.substring(0,etag.length()-1)+"--gzip\""); fields.put(HttpHeader.ETAG,etag.substring(0,etag.length()-1)+"--gzip\"");
LOG.debug("{} compressing {}",this,_deflater);
_state.set(GZState.COMPRESSING); _state.set(GZState.COMPRESSING);
gzip(content,complete,callback); 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) public void mightCompress(GzipFactory factory)
{ {
while (true) while (true)
@ -236,12 +265,15 @@ public class GzipHttpOutput extends HttpOutput
case NOT_COMPRESSING: case NOT_COMPRESSING:
_factory=factory; _factory=factory;
if (_state.compareAndSet(GZState.NOT_COMPRESSING,GZState.MIGHT_COMPRESS)) if (_state.compareAndSet(GZState.NOT_COMPRESSING,GZState.MIGHT_COMPRESS))
{
LOG.debug("{} might compress",this);
return; return;
}
_factory=null; _factory=null;
break; break;
default: default:
throw new IllegalStateException(); throw new IllegalStateException(_state.get().toString());
} }
} }
} }
@ -275,24 +307,25 @@ public class GzipHttpOutput extends HttpOutput
_deflater=null; _deflater=null;
getHttpChannel().getByteBufferPool().release(_buffer); getHttpChannel().getByteBufferPool().release(_buffer);
_buffer=null; _buffer=null;
return State.SUCCEEDED;
} }
return State.SUCCEEDED;
if (!_complete)
return State.SUCCEEDED;
} }
BufferUtil.compact(_buffer);
int off=_buffer.arrayOffset()+_buffer.limit(); int off=_buffer.arrayOffset()+_buffer.limit();
int len=_buffer.capacity()-_buffer.limit()- (_complete?8:0); int len=_buffer.capacity()-_buffer.limit()- (_complete?8:0);
int produced=_deflater.deflate(_buffer.array(),off,len,Deflater.NO_FLUSH); int produced=_deflater.deflate(_buffer.array(),off,len,Deflater.NO_FLUSH);
_buffer.limit(_buffer.limit()+produced); _buffer.limit(_buffer.limit()+produced);
boolean complete=_deflater.finished(); boolean complete=_deflater.finished();
if (complete) if (complete)
{
addTrailer(); addTrailer();
_deflater.end(); // TODO recycle
}
superWrite(_buffer,complete,this); superWrite(_buffer,complete,this);
return State.SCHEDULED; return State.SCHEDULED;
} }
} }
private class GzipBufferCB extends IteratingNestedCallback private class GzipBufferCB extends IteratingNestedCallback
@ -321,30 +354,40 @@ public class GzipHttpOutput extends HttpOutput
_deflater=null; _deflater=null;
getHttpChannel().getByteBufferPool().release(_buffer); getHttpChannel().getByteBufferPool().release(_buffer);
_buffer=null; _buffer=null;
return State.SUCCEEDED;
} }
return State.SUCCEEDED;
if (!_complete)
return State.SUCCEEDED;
} }
else
BufferUtil.clearToFill(_input); {
BufferUtil.put(_content,_input); BufferUtil.clearToFill(_input);
BufferUtil.flipToFlush(_input,0); BufferUtil.put(_content,_input);
BufferUtil.flipToFlush(_input,0);
byte[] array=_input.array(); byte[] array=_input.array();
int off=_input.arrayOffset()+_input.position(); int off=_input.arrayOffset()+_input.position();
int len=_input.remaining(); int len=_input.remaining();
_crc.update(array,off,len);
_deflater.setInput(array,off,len); _crc.update(array,off,len);
if (_complete && BufferUtil.isEmpty(_content)) _deflater.setInput(array,off,len);
_deflater.finish(); if (_complete && BufferUtil.isEmpty(_content))
_deflater.finish();
}
} }
BufferUtil.compact(_buffer);
int off=_buffer.arrayOffset()+_buffer.limit(); int off=_buffer.arrayOffset()+_buffer.limit();
int len=_buffer.capacity()-_buffer.limit() - (_complete?8:0); int len=_buffer.capacity()-_buffer.limit() - (_complete?8:0);
int produced=_deflater.deflate(_buffer.array(),off,len,Deflater.NO_FLUSH); int produced=_deflater.deflate(_buffer.array(),off,len,Deflater.NO_FLUSH);
_buffer.limit(_buffer.limit()+produced); _buffer.limit(_buffer.limit()+produced);
boolean complete=_deflater.finished(); boolean complete=_deflater.finished();
if (complete) if (complete)
addTrailer(); addTrailer();
superWrite(_buffer,complete,this); superWrite(_buffer,complete,this);
return State.SCHEDULED; return State.SCHEDULED;
} }

View File

@ -285,6 +285,8 @@ public class BufferUtil
*/ */
public static boolean compact(ByteBuffer buffer) public static boolean compact(ByteBuffer buffer)
{ {
if (buffer.position()==0)
return false;
boolean full = buffer.limit() == buffer.capacity(); boolean full = buffer.limit() == buffer.capacity();
buffer.compact().flip(); buffer.compact().flip();
return full && buffer.limit() < buffer.capacity(); return full && buffer.limit() < buffer.capacity();

View File

@ -115,7 +115,8 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
"org.eclipse.jetty.jndi.", // webapp cannot change naming classes "org.eclipse.jetty.jndi.", // webapp cannot change naming classes
"org.eclipse.jetty.jaas.", // webapp cannot change jaas classes "org.eclipse.jetty.jaas.", // webapp cannot change jaas classes
"org.eclipse.jetty.websocket.", // webapp cannot change / replace websocket 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 // 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.jaas.", // don't hide jaas classes
"-org.eclipse.jetty.servlets.", // don't hide jetty servlets "-org.eclipse.jetty.servlets.", // don't hide jetty servlets
"-org.eclipse.jetty.servlet.DefaultServlet", // don't hide default servlet "-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.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.websocket.", // don't hide websocket classes from webapps (allow webapp to use ones from system classloader)
"org.eclipse.jetty." // hide other jetty classes "org.eclipse.jetty." // hide other jetty classes

View File

@ -55,7 +55,7 @@
<filter> <filter>
<filter-name>GzipFilter</filter-name> <filter-name>GzipFilter</filter-name>
<filter-class>org.eclipse.jetty.servlets.IncludableGzipFilter</filter-class> <filter-class>org.eclipse.jetty.servlets.AsyncGzipFilter</filter-class>
<async-supported>true</async-supported> <async-supported>true</async-supported>
<init-param> <init-param>
<param-name>bufferSize</param-name> <param-name>bufferSize</param-name>
@ -63,7 +63,7 @@
</init-param> </init-param>
<init-param> <init-param>
<param-name>mimeTypes</param-name> <param-name>mimeTypes</param-name>
<param-value>text/plain,application/xml</param-value> <param-value>text/plain,application/xml,text/html</param-value>
</init-param> </init-param>
<init-param> <init-param>
<param-name>minGzipSize</param-name> <param-name>minGzipSize</param-name>
@ -92,7 +92,6 @@
<url-pattern>*.txt</url-pattern> <url-pattern>*.txt</url-pattern>
</filter-mapping> </filter-mapping>
<servlet> <servlet>
<servlet-name>Login</servlet-name> <servlet-name>Login</servlet-name>
<servlet-class>com.acme.LoginServlet</servlet-class> <servlet-class>com.acme.LoginServlet</servlet-class>