474453 - Tiny buffers (under 7 bytes) fail to compress in permessage-deflate

+ Ensure compress() is sanely using Deflater.deflate()
+ Ensure output buffer in .deflate() is always a minimum
  of 256 bytes
This commit is contained in:
Joakim Erdfelt 2015-08-06 15:09:42 -07:00
parent ffcedde60a
commit c424b58153
2 changed files with 51 additions and 60 deletions

View File

@ -41,7 +41,7 @@ import org.eclipse.jetty.websocket.common.frames.DataFrame;
public abstract class CompressExtension extends AbstractExtension
{
protected static final byte[] TAIL_BYTES = new byte[] { 0x00, 0x00, (byte)0xFF, (byte)0xFF};
protected static final byte[] TAIL_BYTES = new byte[] { 0x00, 0x00, (byte)0xFF, (byte)0xFF };
protected static final ByteBuffer TAIL_BYTES_BUF = ByteBuffer.wrap(TAIL_BYTES);
private static final Logger LOG = Log.getLogger(CompressExtension.class);
@ -63,8 +63,10 @@ public abstract class CompressExtension extends AbstractExtension
/** Inflater / Decompressed Buffer Size */
protected static final int INFLATE_BUFFER_SIZE = 8 * 1024;
/** Deflater / Inflater: Maximum Input Buffer Size */
protected static final int INPUT_MAX_BUFFER_SIZE = 8 * 1024;
private final static boolean NOWRAP = true;
private final Queue<FrameEntry> entries = new ConcurrentArrayQueue<>();
@ -142,8 +144,7 @@ public abstract class CompressExtension extends AbstractExtension
return new ByteAccumulator(maxSize);
}
protected void decompress(ByteAccumulator accumulator, ByteBuffer buf)
throws DataFormatException
protected void decompress(ByteAccumulator accumulator, ByteBuffer buf) throws DataFormatException
{
if ((buf == null) || (!buf.hasRemaining()))
{
@ -151,7 +152,7 @@ public abstract class CompressExtension extends AbstractExtension
}
byte[] output = new byte[1024]; // TODO: make configurable size
if (inflater.needsInput() && !supplyInput(inflater, buf))
if (inflater.needsInput() && !supplyInput(inflater,buf))
{
LOG.debug("Needed input, but no buffer could supply input");
return;
@ -257,7 +258,7 @@ public abstract class CompressExtension extends AbstractExtension
else
{
// Only create an return byte buffer that is reasonable in size
len = Math.min(INPUT_MAX_BUFFER_SIZE, buf.remaining());
len = Math.min(INPUT_MAX_BUFFER_SIZE,buf.remaining());
input = new byte[len];
inputOffset = 0;
buf.get(input,0,len);
@ -297,7 +298,7 @@ public abstract class CompressExtension extends AbstractExtension
else
{
// Only create an return byte buffer that is reasonable in size
len = Math.min(INPUT_MAX_BUFFER_SIZE, buf.remaining());
len = Math.min(INPUT_MAX_BUFFER_SIZE,buf.remaining());
input = new byte[len];
inputOffset = 0;
buf.get(input,0,len);
@ -319,8 +320,8 @@ public abstract class CompressExtension extends AbstractExtension
private static String toDetail(Deflater deflater)
{
return String.format("Deflater[finished=%b,read=%d,written=%d,in=%d,out=%d]",deflater.finished(),deflater.getBytesRead(),
deflater.getBytesWritten(),deflater.getTotalIn(),deflater.getTotalOut());
return String.format("Deflater[finished=%b,read=%d,written=%d,in=%d,out=%d]",deflater.finished(),deflater.getBytesRead(),deflater.getBytesWritten(),
deflater.getTotalIn(),deflater.getTotalOut());
}
public static boolean endsWithTail(ByteBuffer buf)
@ -405,16 +406,14 @@ public abstract class CompressExtension extends AbstractExtension
private void compress(FrameEntry entry, boolean first)
{
final int flush = Deflater.SYNC_FLUSH;
// Get a chunk of the payload to avoid to blow
// the heap if the payload is a huge mapped file.
Frame frame = entry.frame;
ByteBuffer data = frame.getPayload();
int remaining = data.remaining();
int chunkSize = Math.min(remaining,INPUT_BUFSIZE);
int outputLength = Math.max(256,data.remaining());
if (LOG.isDebugEnabled())
LOG.debug("Compressing {}: {} bytes in {} bytes chunk",entry,remaining,chunkSize);
LOG.debug("Compressing {}: {} bytes in {} bytes chunk",entry,remaining,outputLength);
boolean needsCompress = true;
@ -426,42 +425,32 @@ public abstract class CompressExtension extends AbstractExtension
ByteArrayOutputStream out = new ByteArrayOutputStream();
byte[] output = new byte[chunkSize];
byte[] output = new byte[outputLength];
boolean fin = frame.isFin();
// Compress the data
while (needsCompress)
{
int read = deflater.deflate(output,0,chunkSize,flush);
if (read == 0)
int compressed = deflater.deflate(output,0,outputLength,Deflater.SYNC_FLUSH);
// Append the output for the eventual frame.
if (LOG.isDebugEnabled())
LOG.debug("Wrote {} bytes to output buffer",compressed);
out.write(output,0,compressed);
if (compressed < outputLength)
{
if (deflater.finished())
{
// done
break;
}
else if (deflater.needsInput())
{
if (!supplyInput(deflater,data))
{
// done
needsCompress = false;
}
}
}
else
{
// Append the output for the eventual frame.
out.write(output,0,read);
}
}
boolean fin = frame.isFin();
ByteBuffer payload = ByteBuffer.wrap(out.toByteArray());
if (payload.remaining()>0)
if (payload.remaining() > 0)
{
// Handle tail bytes generated by SYNC_FLUSH.
if (LOG.isDebugEnabled())
LOG.debug("compressed bytes[] = {}",BufferUtil.toDetailString(payload));
if (tailDrop == TAIL_DROP_ALWAYS)
@ -470,6 +459,7 @@ public abstract class CompressExtension extends AbstractExtension
{
payload.limit(payload.limit() - TAIL_BYTES.length);
}
if (LOG.isDebugEnabled())
LOG.debug("payload (TAIL_DROP_ALWAYS) = {}",BufferUtil.toDetailString(payload));
}
else if (tailDrop == TAIL_DROP_FIN_ONLY)
@ -478,6 +468,7 @@ public abstract class CompressExtension extends AbstractExtension
{
payload.limit(payload.limit() - TAIL_BYTES.length);
}
if (LOG.isDebugEnabled())
LOG.debug("payload (TAIL_DROP_FIN_ONLY) = {}",BufferUtil.toDetailString(payload));
}
}
@ -489,7 +480,7 @@ public abstract class CompressExtension extends AbstractExtension
if (LOG.isDebugEnabled())
{
LOG.debug("Compressed {}: input:{} -> payload:{}",entry,chunkSize,payload.remaining());
LOG.debug("Compressed {}: input:{} -> payload:{}",entry,outputLength,payload.remaining());
}
boolean continuation = frame.getType().isContinuation() || !first;

View File

@ -57,7 +57,7 @@ public class PerMessageDeflateExtensionTest
@Test
public void testPerMessageDeflateDefault() throws Exception
{
Assume.assumeTrue("Server has x-webkit-deflate-frame registered",
Assume.assumeTrue("Server has permessage-deflate registered",
server.getWebSocketServletFactory().getExtensionFactory().isAvailable("permessage-deflate"));
BlockheadClient client = new BlockheadClient(server.getServerUri());