Skip to content

Commit cf63bc1

Browse files
amizurovnormanmaurer
authored andcommitted
Fix netty#9770, last frame may contain extra data that doesn't affect decompression (netty#9832)
Motivation: Client can split data into different numbers of fragments and sometimes the last frame may contain trash data that doesn't affect decompression process. Modification: Added check if last frame is `ContinuationWebSocketFrame` and decompression data is empty then don't throw an exception. Result: Fixes netty#9770
1 parent 9fa8b02 commit cf63bc1

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/DeflateDecoder.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,12 @@ private ByteBuf decompressContent(ChannelHandlerContext ctx, WebSocketFrame msg)
136136
// Correctly handle empty frames
137137
// See https://github.com/netty/netty/issues/4348
138138
if (!emptyDeflateBlock && readable && compositeDecompressedContent.numComponents() <= 0) {
139-
compositeDecompressedContent.release();
140-
throw new CodecException("cannot read uncompressed buffer");
139+
// Sometimes after fragmentation the last frame
140+
// May contain left-over data that doesn't affect decompression
141+
if (!(msg instanceof ContinuationWebSocketFrame)) {
142+
compositeDecompressedContent.release();
143+
throw new CodecException("cannot read uncompressed buffer");
144+
}
141145
}
142146

143147
if (msg.isFinalFragment() && noContext) {

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateDecoderTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,4 +328,62 @@ public void testEmptyFrameDecompression() {
328328
assertFalse(decoderChannel.finish());
329329
}
330330

331+
@Test
332+
public void testFragmentedFrameWithLeftOverInLastFragment() {
333+
String hexDump = "677170647a777a737574656b707a787a6f6a7561756578756f6b7868616371716c657a6d64697479766d726f6" +
334+
"269746c6376777464776f6f72767a726f64667278676764687775786f6762766d776d706b76697773777a7072" +
335+
"6a6a737279707a7078697a6c69616d7461656d646278626d786f66666e686e776a7a7461746d7a776668776b6" +
336+
"f6f736e73746575637a6d727a7175707a6e74627578687871767771697a71766c64626d78726d6d7675756877" +
337+
"62667963626b687a726d676e646263776e67797264706d6c6863626577616967706a78636a72697464756e627" +
338+
"977616f79736475676f76736f7178746a7a7479626c64636b6b6778637768746c62";
339+
EmbeddedChannel encoderChannel = new EmbeddedChannel(
340+
ZlibCodecFactory.newZlibEncoder(ZlibWrapper.NONE, 9, 15, 8));
341+
EmbeddedChannel decoderChannel = new EmbeddedChannel(new PerMessageDeflateDecoder(false));
342+
343+
ByteBuf originPayload = Unpooled.wrappedBuffer(ByteBufUtil.decodeHexDump(hexDump));
344+
assertTrue(encoderChannel.writeOutbound(originPayload.duplicate().retain()));
345+
346+
ByteBuf compressedPayload = encoderChannel.readOutbound();
347+
compressedPayload = compressedPayload.slice(0, compressedPayload.readableBytes() - 4);
348+
349+
int oneThird = compressedPayload.readableBytes() / 3;
350+
351+
TextWebSocketFrame compressedFrame1 = new TextWebSocketFrame(
352+
false, WebSocketExtension.RSV1, compressedPayload.slice(0, oneThird));
353+
ContinuationWebSocketFrame compressedFrame2 = new ContinuationWebSocketFrame(
354+
false, WebSocketExtension.RSV3, compressedPayload.slice(oneThird, oneThird));
355+
ContinuationWebSocketFrame compressedFrame3 = new ContinuationWebSocketFrame(
356+
false, WebSocketExtension.RSV3, compressedPayload.slice(oneThird * 2, oneThird));
357+
int offset = oneThird * 3;
358+
ContinuationWebSocketFrame compressedFrameWithExtraData = new ContinuationWebSocketFrame(
359+
true, WebSocketExtension.RSV3, compressedPayload.slice(offset,
360+
compressedPayload.readableBytes() - offset));
361+
362+
// check that last fragment contains only one extra byte
363+
assertEquals(1, compressedFrameWithExtraData.content().readableBytes());
364+
assertEquals(1, compressedFrameWithExtraData.content().getByte(0));
365+
366+
// write compressed frames
367+
assertTrue(decoderChannel.writeInbound(compressedFrame1.retain()));
368+
assertTrue(decoderChannel.writeInbound(compressedFrame2.retain()));
369+
assertTrue(decoderChannel.writeInbound(compressedFrame3.retain()));
370+
assertTrue(decoderChannel.writeInbound(compressedFrameWithExtraData));
371+
372+
// read uncompressed frames
373+
TextWebSocketFrame uncompressedFrame1 = decoderChannel.readInbound();
374+
ContinuationWebSocketFrame uncompressedFrame2 = decoderChannel.readInbound();
375+
ContinuationWebSocketFrame uncompressedFrame3 = decoderChannel.readInbound();
376+
ContinuationWebSocketFrame uncompressedExtraData = decoderChannel.readInbound();
377+
assertFalse(uncompressedExtraData.content().isReadable());
378+
379+
ByteBuf uncompressedPayload = Unpooled.wrappedBuffer(uncompressedFrame1.content(), uncompressedFrame2.content(),
380+
uncompressedFrame3.content(), uncompressedExtraData.content());
381+
assertEquals(originPayload, uncompressedPayload);
382+
383+
assertTrue(originPayload.release());
384+
assertTrue(uncompressedPayload.release());
385+
386+
assertTrue(encoderChannel.finishAndReleaseAll());
387+
assertFalse(decoderChannel.finish());
388+
}
331389
}

0 commit comments

Comments
 (0)