Issue #4188 Spin in close of GzipHandler (#4198)

* Issue #4188 Spin in close of GzipHandler

Cleanup and simplify code

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4188 Spin in close of GzipHandler

Fix slice code. Added unit test for it.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4188 Spin in close of GzipHandler

Fixed last slice.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* cleanup from review

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-10-16 14:12:52 +11:00 committed by GitHub
parent 92c8bb8dd5
commit 73924d2774
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 176 additions and 76 deletions

View File

@ -128,20 +128,8 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
private void addTrailer()
{
int i = _buffer.limit();
_buffer.limit(i + 8);
int v = (int)_crc.getValue();
_buffer.put(i++, (byte)(v & 0xFF));
_buffer.put(i++, (byte)((v >>> 8) & 0xFF));
_buffer.put(i++, (byte)((v >>> 16) & 0xFF));
_buffer.put(i++, (byte)((v >>> 24) & 0xFF));
v = _deflater.getTotalIn();
_buffer.put(i++, (byte)(v & 0xFF));
_buffer.put(i++, (byte)((v >>> 8) & 0xFF));
_buffer.put(i++, (byte)((v >>> 16) & 0xFF));
_buffer.put(i++, (byte)((v >>> 24) & 0xFF));
BufferUtil.putIntLittleEndian(_buffer, (int)_crc.getValue());
BufferUtil.putIntLittleEndian(_buffer, _deflater.getTotalIn());
}
private void gzip(ByteBuffer content, boolean complete, final Callback callback)
@ -231,8 +219,6 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
fields.put(GZIP._contentEncoding);
_crc.reset();
_buffer = _channel.getByteBufferPool().acquire(_bufferSize, false);
BufferUtil.fill(_buffer, GZIP_HEADER, 0, GZIP_HEADER.length);
// Adjust headers
response.setContentLength(-1);
@ -325,19 +311,17 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
@Override
protected Action process() throws Exception
{
// If we have no deflator
if (_deflater == null)
return Action.SUCCEEDED;
if (_deflater.needsInput())
{
if (BufferUtil.isEmpty(_content))
// then the trailer has been generated and written below.
// we have finished compressing the entire content, so
// cleanup and succeed.
if (_buffer != null)
{
if (_deflater.finished())
{
_factory.recycle(_deflater);
_deflater = null;
_channel.getByteBufferPool().release(_buffer);
_buffer = null;
}
if (_copy != null)
{
_channel.getByteBufferPool().release(_copy);
@ -346,61 +330,96 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
return Action.SUCCEEDED;
}
if (!_last)
// If we have no buffer
if (_buffer == null)
{
return Action.SUCCEEDED;
// allocate a buffer and add the gzip header
_buffer = _channel.getByteBufferPool().acquire(_bufferSize, false);
BufferUtil.fill(_buffer, GZIP_HEADER, 0, GZIP_HEADER.length);
}
else
{
// otherwise clear the buffer as previous writes will always fully consume.
BufferUtil.clear(_buffer);
}
_deflater.finish();
}
else if (_content.hasArray())
// If the deflator is not finished, then compress more data
if (!_deflater.finished())
{
if (_deflater.needsInput())
{
// if there is no more content available to compress
// then we are either finished all content or just the current write.
if (BufferUtil.isEmpty(_content))
{
byte[] array = _content.array();
int off = _content.arrayOffset() + _content.position();
int len = _content.remaining();
BufferUtil.clear(_content);
_crc.update(array, off, len);
_deflater.setInput(array, off, len);
if (_last)
_deflater.finish();
else
return Action.SUCCEEDED;
}
else
{
// If there is more content available to compress, we have to make sure
// it is available in an array for the current deflator API, maybe slicing
// of content.
ByteBuffer slice;
if (_content.hasArray())
slice = _content;
else
{
if (_copy == null)
_copy = _channel.getByteBufferPool().acquire(_bufferSize, false);
BufferUtil.clearToFill(_copy);
int took = BufferUtil.put(_content, _copy);
BufferUtil.flipToFlush(_copy, 0);
if (took == 0)
throw new IllegalStateException();
byte[] array = _copy.array();
int off = _copy.arrayOffset() + _copy.position();
int len = _copy.remaining();
else
BufferUtil.clear(_copy);
slice = _copy;
BufferUtil.append(_copy, _content);
}
// transfer the data from the slice to the the deflator
byte[] array = slice.array();
int off = slice.arrayOffset() + slice.position();
int len = slice.remaining();
_crc.update(array, off, len);
_deflater.setInput(array, off, len);
_deflater.setInput(array, off, len); // TODO use ByteBuffer API in Jetty-10
BufferUtil.clear(slice);
if (_last && BufferUtil.isEmpty(_content))
_deflater.finish();
}
}
BufferUtil.compact(_buffer);
// deflate the content into the available space in the buffer
int off = _buffer.arrayOffset() + _buffer.limit();
int len = _buffer.capacity() - _buffer.limit() - (_last ? 8 : 0);
if (len > 0)
{
int len = BufferUtil.space(_buffer);
int produced = _deflater.deflate(_buffer.array(), off, len, _syncFlush ? Deflater.SYNC_FLUSH : Deflater.NO_FLUSH);
_buffer.limit(_buffer.limit() + produced);
}
boolean finished = _deflater.finished();
if (finished)
// If we have finished deflation and there is room for the trailer.
if (_deflater.finished() && BufferUtil.space(_buffer) >= 8)
{
// add the trailer and recycle the deflator to flag that we will have had completeSuccess when
// the write below completes.
addTrailer();
_factory.recycle(_deflater);
_deflater = null;
}
_interceptor.write(_buffer, finished, this);
// write the compressed buffer.
_interceptor.write(_buffer, _deflater == null, this);
return Action.SCHEDULED;
}
@Override
public String toString()
{
return String.format("%s[content=%s last=%b copy=%s buffer=%s deflate=%s",
super.toString(),
BufferUtil.toDetailString(_content),
_last,
BufferUtil.toDetailString(_copy),
BufferUtil.toDetailString(_buffer),
_deflater,
_deflater != null && _deflater.finished() ? "(finished)" : "");
}
}
}

View File

@ -45,9 +45,11 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.server.HttpOutput;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
@ -116,6 +118,7 @@ public class GzipHandlerTest
servlets.addServletWithMapping(EchoServlet.class, "/echo/*");
servlets.addServletWithMapping(DumpServlet.class, "/dump/*");
servlets.addServletWithMapping(AsyncServlet.class, "/async/*");
servlets.addServletWithMapping(BufferServlet.class, "/buffer/*");
servlets.addFilterWithMapping(CheckFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
_server.start();
@ -221,6 +224,19 @@ public class GzipHandlerTest
}
}
public static class BufferServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
HttpOutput out = (HttpOutput)response.getOutputStream();
ByteBuffer buffer = BufferUtil.toBuffer(__bytes).asReadOnlyBuffer();
response.setContentLength(buffer.remaining());
response.setContentType("text/plain");
out.write(buffer);
}
}
public static class EchoServlet extends HttpServlet
{
@Override
@ -357,6 +373,32 @@ public class GzipHandlerTest
assertEquals(__content, testOut.toString("UTF8"));
}
@Test
public void testBufferResponse() throws Exception
{
// generated and parsed test
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;
request.setMethod("GET");
request.setURI("/ctx/buffer/info");
request.setVersion("HTTP/1.0");
request.setHeader("Host", "tester");
request.setHeader("accept-encoding", "gzip");
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding"));
InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn, testOut);
assertEquals(__content, testOut.toString("UTF8"));
}
@Test
public void testAsyncLargeResponse() throws Exception
{
@ -387,6 +429,31 @@ public class GzipHandlerTest
assertEquals(__content, new String(Arrays.copyOfRange(bytes,i * __bytes.length, (i + 1) * __bytes.length), StandardCharsets.UTF_8), "chunk " + i);
}
@Test
public void testAsyncEmptyResponse() throws Exception
{
int writes = 0;
_server.getChildHandlerByClass(GzipHandler.class).setMinGzipSize(0);
// generated and parsed test
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;
request.setMethod("GET");
request.setURI("/ctx/async/info?writes=" + writes);
request.setVersion("HTTP/1.0");
request.setHeader("Host", "tester");
request.setHeader("accept-encoding", "gzip");
response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
assertThat(response.getStatus(), is(200));
assertThat(response.get("Content-Encoding"), Matchers.equalToIgnoringCase("gzip"));
assertThat(response.getCSV("Vary", false), Matchers.contains("Accept-Encoding"));
}
@Test
public void testGzipHandlerWithMultipleAcceptEncodingHeaders() throws Exception
{

View File

@ -219,6 +219,20 @@ public class BufferUtil
buffer.position(position);
}
/** Put an integer little endian
* @param buffer The buffer to put to
* @param value The value to put.
*/
public static void putIntLittleEndian(ByteBuffer buffer, int value)
{
int p = flipToFill(buffer);
buffer.put((byte)(value & 0xFF));
buffer.put((byte)((value >>> 8) & 0xFF));
buffer.put((byte)((value >>> 16) & 0xFF));
buffer.put((byte)((value >>> 24) & 0xFF));
flipToFlush(buffer, p);
}
/**
* Convert a ByteBuffer to a byte array.
*