From 491b1f428b00bba45a2c51d171cf819704b0fbe6 Mon Sep 17 00:00:00 2001 From: Andrey Mizurov Date: Wed, 28 Aug 2019 09:21:38 +0300 Subject: [PATCH] Fix sending an empty String like "" causes an error #9429 (#9512) Motivation: Handle https://tools.ietf.org/html/rfc7692#section-7.2.3.6 Result: The empty buffer is correctly handled in deflate encoder/decoder. Fixes #9429 . --- .../compression/DeflateDecoder.java | 92 +++++++++++-------- .../compression/DeflateEncoder.java | 54 +++++++---- .../PerMessageDeflateDecoderTest.java | 18 ++++ .../PerMessageDeflateEncoderTest.java | 35 ++++++- 4 files changed, 139 insertions(+), 60 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateDecoder.java index 746032dbbc..223e2e6520 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateDecoder.java @@ -44,6 +44,10 @@ abstract class DeflateDecoder extends WebSocketExtensionDecoder { Unpooled.wrappedBuffer(new byte[] {0x00, 0x00, (byte) 0xff, (byte) 0xff})) .asReadOnly(); + static final ByteBuf EMPTY_DEFLATE_BLOCK = Unpooled.unreleasableBuffer( + Unpooled.wrappedBuffer(new byte[] { 0x00 })) + .asReadOnly(); + private final boolean noContext; private final WebSocketExtensionFilter extensionDecoderFilter; @@ -73,53 +77,19 @@ abstract class DeflateDecoder extends WebSocketExtensionDecoder { @Override protected void decode(ChannelHandlerContext ctx, WebSocketFrame msg, List out) throws Exception { - if (decoder == null) { - if (!(msg instanceof TextWebSocketFrame) && !(msg instanceof BinaryWebSocketFrame)) { - throw new CodecException("unexpected initial frame type: " + msg.getClass().getName()); - } - decoder = new EmbeddedChannel(ZlibCodecFactory.newZlibDecoder(ZlibWrapper.NONE)); - } + final ByteBuf decompressedContent = decompressContent(ctx, msg); - boolean readable = msg.content().isReadable(); - decoder.writeInbound(msg.content().retain()); - if (appendFrameTail(msg)) { - decoder.writeInbound(FRAME_TAIL.duplicate()); - } - - CompositeByteBuf compositeUncompressedContent = ctx.alloc().compositeBuffer(); - for (;;) { - ByteBuf partUncompressedContent = decoder.readInbound(); - if (partUncompressedContent == null) { - break; - } - if (!partUncompressedContent.isReadable()) { - partUncompressedContent.release(); - continue; - } - compositeUncompressedContent.addComponent(true, partUncompressedContent); - } - // Correctly handle empty frames - // See https://github.com/netty/netty/issues/4348 - if (readable && compositeUncompressedContent.numComponents() <= 0) { - compositeUncompressedContent.release(); - throw new CodecException("cannot read uncompressed buffer"); - } - - if (msg.isFinalFragment() && noContext) { - cleanup(); - } - - WebSocketFrame outMsg; + final WebSocketFrame outMsg; if (msg instanceof TextWebSocketFrame) { - outMsg = new TextWebSocketFrame(msg.isFinalFragment(), newRsv(msg), compositeUncompressedContent); + outMsg = new TextWebSocketFrame(msg.isFinalFragment(), newRsv(msg), decompressedContent); } else if (msg instanceof BinaryWebSocketFrame) { - outMsg = new BinaryWebSocketFrame(msg.isFinalFragment(), newRsv(msg), compositeUncompressedContent); + outMsg = new BinaryWebSocketFrame(msg.isFinalFragment(), newRsv(msg), decompressedContent); } else if (msg instanceof ContinuationWebSocketFrame) { - outMsg = new ContinuationWebSocketFrame(msg.isFinalFragment(), newRsv(msg), - compositeUncompressedContent); + outMsg = new ContinuationWebSocketFrame(msg.isFinalFragment(), newRsv(msg), decompressedContent); } else { throw new CodecException("unexpected frame type: " + msg.getClass().getName()); } + out.add(outMsg); } @@ -135,6 +105,48 @@ abstract class DeflateDecoder extends WebSocketExtensionDecoder { super.channelInactive(ctx); } + private ByteBuf decompressContent(ChannelHandlerContext ctx, WebSocketFrame msg) { + if (decoder == null) { + if (!(msg instanceof TextWebSocketFrame) && !(msg instanceof BinaryWebSocketFrame)) { + throw new CodecException("unexpected initial frame type: " + msg.getClass().getName()); + } + decoder = new EmbeddedChannel(ZlibCodecFactory.newZlibDecoder(ZlibWrapper.NONE)); + } + + boolean readable = msg.content().isReadable(); + boolean emptyDeflateBlock = EMPTY_DEFLATE_BLOCK.equals(msg.content()); + + decoder.writeInbound(msg.content().retain()); + if (appendFrameTail(msg)) { + decoder.writeInbound(FRAME_TAIL.duplicate()); + } + + CompositeByteBuf compositeDecompressedContent = ctx.alloc().compositeBuffer(); + for (;;) { + ByteBuf partUncompressedContent = decoder.readInbound(); + if (partUncompressedContent == null) { + break; + } + if (!partUncompressedContent.isReadable()) { + partUncompressedContent.release(); + continue; + } + compositeDecompressedContent.addComponent(true, partUncompressedContent); + } + // Correctly handle empty frames + // See https://github.com/netty/netty/issues/4348 + if (!emptyDeflateBlock && readable && compositeDecompressedContent.numComponents() <= 0) { + compositeDecompressedContent.release(); + throw new CodecException("cannot read uncompressed buffer"); + } + + if (msg.isFinalFragment() && noContext) { + cleanup(); + } + + return compositeDecompressedContent; + } + private void cleanup() { if (decoder != null) { // Clean-up the previous encoder if not cleaned up correctly. diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateEncoder.java index 07d5ca34bc..1203c49880 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateEncoder.java @@ -82,8 +82,39 @@ abstract class DeflateEncoder extends WebSocketExtensionEncoder { protected abstract boolean removeFrameTail(WebSocketFrame msg); @Override - protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, - List out) throws Exception { + protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List out) throws Exception { + final ByteBuf compressedContent; + if (msg.content().isReadable()) { + compressedContent = compressContent(ctx, msg); + } else if (msg.isFinalFragment()) { + // Set empty DEFLATE block manually for unknown buffer size + // https://tools.ietf.org/html/rfc7692#section-7.2.3.6 + compressedContent = EMPTY_DEFLATE_BLOCK.duplicate(); + } else { + throw new CodecException("cannot compress content buffer"); + } + + final WebSocketFrame outMsg; + if (msg instanceof TextWebSocketFrame) { + outMsg = new TextWebSocketFrame(msg.isFinalFragment(), rsv(msg), compressedContent); + } else if (msg instanceof BinaryWebSocketFrame) { + outMsg = new BinaryWebSocketFrame(msg.isFinalFragment(), rsv(msg), compressedContent); + } else if (msg instanceof ContinuationWebSocketFrame) { + outMsg = new ContinuationWebSocketFrame(msg.isFinalFragment(), rsv(msg), compressedContent); + } else { + throw new CodecException("unexpected frame type: " + msg.getClass().getName()); + } + + out.add(outMsg); + } + + @Override + public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { + cleanup(); + super.handlerRemoved(ctx); + } + + private ByteBuf compressContent(ChannelHandlerContext ctx, WebSocketFrame msg) { if (encoder == null) { encoder = new EmbeddedChannel(ZlibCodecFactory.newZlibEncoder( ZlibWrapper.NONE, compressionLevel, windowSize, 8)); @@ -103,6 +134,7 @@ abstract class DeflateEncoder extends WebSocketExtensionEncoder { } fullCompressedContent.addComponent(true, partCompressedContent); } + if (fullCompressedContent.numComponents() <= 0) { fullCompressedContent.release(); throw new CodecException("cannot read compressed buffer"); @@ -120,23 +152,7 @@ abstract class DeflateEncoder extends WebSocketExtensionEncoder { compressedContent = fullCompressedContent; } - WebSocketFrame outMsg; - if (msg instanceof TextWebSocketFrame) { - outMsg = new TextWebSocketFrame(msg.isFinalFragment(), rsv(msg), compressedContent); - } else if (msg instanceof BinaryWebSocketFrame) { - outMsg = new BinaryWebSocketFrame(msg.isFinalFragment(), rsv(msg), compressedContent); - } else if (msg instanceof ContinuationWebSocketFrame) { - outMsg = new ContinuationWebSocketFrame(msg.isFinalFragment(), rsv(msg), compressedContent); - } else { - throw new CodecException("unexpected frame type: " + msg.getClass().getName()); - } - out.add(outMsg); - } - - @Override - public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { - cleanup(); - super.handlerRemoved(ctx); + return compressedContent; } private void cleanup() { diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateDecoderTest.java index fc872b1ecc..fcd290ab9d 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateDecoderTest.java @@ -33,6 +33,7 @@ import org.junit.Test; import java.util.Random; import static io.netty.handler.codec.http.websocketx.extensions.WebSocketExtensionFilter.*; +import static io.netty.handler.codec.http.websocketx.extensions.compression.DeflateDecoder.*; import static io.netty.util.CharsetUtil.*; import static org.junit.Assert.*; @@ -310,4 +311,21 @@ public class PerMessageDeflateDecoderTest { } } + @Test + public void testEmptyFrameDecompression() { + EmbeddedChannel decoderChannel = new EmbeddedChannel(new PerMessageDeflateDecoder(false)); + + TextWebSocketFrame emptyDeflateBlockFrame = new TextWebSocketFrame(true, WebSocketExtension.RSV1, + EMPTY_DEFLATE_BLOCK); + + assertTrue(decoderChannel.writeInbound(emptyDeflateBlockFrame)); + TextWebSocketFrame emptyBufferFrame = decoderChannel.readInbound(); + + assertFalse(emptyBufferFrame.content().isReadable()); + + // Composite empty buffer + assertTrue(emptyBufferFrame.release()); + assertFalse(decoderChannel.finish()); + } + } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateEncoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateEncoderTest.java index 9e986511c5..1f8b47744a 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateEncoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateEncoderTest.java @@ -34,6 +34,7 @@ import java.util.Arrays; import java.util.Random; import static io.netty.handler.codec.http.websocketx.extensions.WebSocketExtensionFilter.*; +import static io.netty.handler.codec.http.websocketx.extensions.compression.DeflateDecoder.*; import static io.netty.util.CharsetUtil.*; import static org.junit.Assert.*; @@ -102,7 +103,7 @@ public class PerMessageDeflateEncoderTest { } @Test - public void testFramementedFrame() { + public void testFragmentedFrame() { EmbeddedChannel encoderChannel = new EmbeddedChannel(new PerMessageDeflateEncoder(9, 15, false, NEVER_SKIP)); EmbeddedChannel decoderChannel = new EmbeddedChannel( @@ -272,4 +273,36 @@ public class PerMessageDeflateEncoderTest { } } + @Test + public void testEmptyFrameCompression() { + EmbeddedChannel encoderChannel = new EmbeddedChannel(new PerMessageDeflateEncoder(9, 15, false)); + + TextWebSocketFrame emptyFrame = new TextWebSocketFrame(""); + + assertTrue(encoderChannel.writeOutbound(emptyFrame)); + TextWebSocketFrame emptyDeflateFrame = encoderChannel.readOutbound(); + + assertEquals(WebSocketExtension.RSV1, emptyDeflateFrame.rsv()); + assertTrue(ByteBufUtil.equals(EMPTY_DEFLATE_BLOCK, emptyDeflateFrame.content())); + // Unreleasable buffer + assertFalse(emptyDeflateFrame.release()); + + assertFalse(encoderChannel.finish()); + } + + @Test(expected = EncoderException.class) + public void testCodecExceptionForNotFinEmptyFrame() { + EmbeddedChannel encoderChannel = new EmbeddedChannel(new PerMessageDeflateEncoder(9, 15, false)); + + TextWebSocketFrame emptyNotFinFrame = new TextWebSocketFrame(false, 0, ""); + + try { + encoderChannel.writeOutbound(emptyNotFinFrame); + } finally { + // EmptyByteBuf buffer + assertFalse(emptyNotFinFrame.release()); + assertFalse(encoderChannel.finish()); + } + } + }