From a6f807dd683243aa388aad0bfc9fcb20f3b1edce Mon Sep 17 00:00:00 2001 From: Christian Lang Date: Thu, 18 Oct 2018 14:55:30 +0300 Subject: [PATCH] Fix context and window sizes sides. (#8395) Motivation: As mentioned in RFC 7692 : The "server_no_context_takeover" Extension Parameter should be used on server side for compression and on client side for decompression. The "client_no_context_takeover" Extension Parameter should be used on client side for compression and on server side for decompression. Right now, in PerMessageDeflateClientExtensionHandshaker, the decoder uses clientNoContext instead of serverNoContext and the encoder uses serverNoContext instead of clientNoContext. The same inversion is present in PerMessageDeflateServerExtensionHandshaker: the decoder uses serverNoContext instead of clientNoContext, while the encoder uses serverNoContext instead of clientNoContext. Besides the context inversion, the sliding window sizes seem to be inversed as well. Modification: Inverse clientNoContext with serverNoContext and clientWindowSize with serverWindowSize for both the Decoder and Encoder in PerMessageDeflateServerExtensionHandshaker and PerMessageDeflateClientExtensionHandshaker. Result: This fixes the decompression fail in the case that one of the contexts is set and the other one is not. --- ...ssageDeflateClientExtensionHandshaker.java | 4 +- ...ssageDeflateServerExtensionHandshaker.java | 4 +- .../PerFrameDeflateDecoderTest.java | 16 +++-- ...eDeflateClientExtensionHandshakerTest.java | 67 ++++++++++++++++++- 4 files changed, 77 insertions(+), 14 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshaker.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshaker.java index ef3dfe7245..ddebaaa92f 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshaker.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshaker.java @@ -186,12 +186,12 @@ public final class PerMessageDeflateClientExtensionHandshaker implements WebSock @Override public WebSocketExtensionEncoder newExtensionEncoder() { - return new PerMessageDeflateEncoder(compressionLevel, serverWindowSize, serverNoContext); + return new PerMessageDeflateEncoder(compressionLevel, clientWindowSize, clientNoContext); } @Override public WebSocketExtensionDecoder newExtensionDecoder() { - return new PerMessageDeflateDecoder(clientNoContext); + return new PerMessageDeflateDecoder(serverNoContext); } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateServerExtensionHandshaker.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateServerExtensionHandshaker.java index 738b6f927d..0bf0162e8a 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateServerExtensionHandshaker.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateServerExtensionHandshaker.java @@ -167,12 +167,12 @@ public final class PerMessageDeflateServerExtensionHandshaker implements WebSock @Override public WebSocketExtensionEncoder newExtensionEncoder() { - return new PerMessageDeflateEncoder(compressionLevel, clientWindowSize, clientNoContext); + return new PerMessageDeflateEncoder(compressionLevel, serverWindowSize, serverNoContext); } @Override public WebSocketExtensionDecoder newExtensionDecoder() { - return new PerMessageDeflateDecoder(serverNoContext); + return new PerMessageDeflateDecoder(clientNoContext); } @Override diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerFrameDeflateDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerFrameDeflateDecoderTest.java index e59d380287..67baa5151f 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerFrameDeflateDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerFrameDeflateDecoderTest.java @@ -15,6 +15,8 @@ */ package io.netty.handler.codec.http.websocketx.extensions.compression; +import static io.netty.handler.codec.http.websocketx.extensions.WebSocketExtension.RSV1; +import static io.netty.handler.codec.http.websocketx.extensions.WebSocketExtension.RSV3; import static org.junit.Assert.*; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; @@ -22,6 +24,7 @@ import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.compression.ZlibCodecFactory; import io.netty.handler.codec.compression.ZlibWrapper; import io.netty.handler.codec.http.websocketx.BinaryWebSocketFrame; +import io.netty.handler.codec.http.websocketx.TextWebSocketFrame; import io.netty.handler.codec.http.websocketx.extensions.WebSocketExtension; import java.util.Arrays; @@ -47,7 +50,7 @@ public class PerFrameDeflateDecoderTest { ByteBuf compressedPayload = encoderChannel.readOutbound(); BinaryWebSocketFrame compressedFrame = new BinaryWebSocketFrame(true, - WebSocketExtension.RSV1 | WebSocketExtension.RSV3, + RSV1 | RSV3, compressedPayload.slice(0, compressedPayload.readableBytes() - 4)); // execute @@ -58,7 +61,7 @@ public class PerFrameDeflateDecoderTest { assertNotNull(uncompressedFrame); assertNotNull(uncompressedFrame.content()); assertTrue(uncompressedFrame instanceof BinaryWebSocketFrame); - assertEquals(WebSocketExtension.RSV3, uncompressedFrame.rsv()); + assertEquals(RSV3, uncompressedFrame.rsv()); assertEquals(300, uncompressedFrame.content().readableBytes()); byte[] finalPayload = new byte[300]; @@ -76,7 +79,7 @@ public class PerFrameDeflateDecoderTest { random.nextBytes(payload); BinaryWebSocketFrame frame = new BinaryWebSocketFrame(true, - WebSocketExtension.RSV3, Unpooled.wrappedBuffer(payload)); + RSV3, Unpooled.wrappedBuffer(payload)); // execute decoderChannel.writeInbound(frame); @@ -86,7 +89,7 @@ public class PerFrameDeflateDecoderTest { assertNotNull(newFrame); assertNotNull(newFrame.content()); assertTrue(newFrame instanceof BinaryWebSocketFrame); - assertEquals(WebSocketExtension.RSV3, newFrame.rsv()); + assertEquals(RSV3, newFrame.rsv()); assertEquals(300, newFrame.content().readableBytes()); byte[] finalPayload = new byte[300]; @@ -105,7 +108,7 @@ public class PerFrameDeflateDecoderTest { encoderChannel.writeOutbound(Unpooled.EMPTY_BUFFER); ByteBuf compressedPayload = encoderChannel.readOutbound(); BinaryWebSocketFrame compressedFrame = - new BinaryWebSocketFrame(true, WebSocketExtension.RSV1 | WebSocketExtension.RSV3, compressedPayload); + new BinaryWebSocketFrame(true, RSV1 | RSV3, compressedPayload); // execute decoderChannel.writeInbound(compressedFrame); @@ -115,9 +118,8 @@ public class PerFrameDeflateDecoderTest { assertNotNull(uncompressedFrame); assertNotNull(uncompressedFrame.content()); assertTrue(uncompressedFrame instanceof BinaryWebSocketFrame); - assertEquals(WebSocketExtension.RSV3, uncompressedFrame.rsv()); + assertEquals(RSV3, uncompressedFrame.rsv()); assertEquals(0, uncompressedFrame.content().readableBytes()); uncompressedFrame.release(); } - } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshakerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshakerTest.java index 7b0fa4a3d3..8ed3a7c25f 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshakerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/extensions/compression/PerMessageDeflateClientExtensionHandshakerTest.java @@ -15,11 +15,15 @@ */ package io.netty.handler.codec.http.websocketx.extensions.compression; +import static io.netty.handler.codec.http.websocketx.extensions.WebSocketExtension.RSV1; import static io.netty.handler.codec.http.websocketx.extensions.compression. PerMessageDeflateServerExtensionHandshaker.*; import static org.junit.Assert.*; +import io.netty.buffer.Unpooled; +import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.compression.ZlibCodecFactory; +import io.netty.handler.codec.http.websocketx.TextWebSocketFrame; import io.netty.handler.codec.http.websocketx.extensions.WebSocketClientExtension; import io.netty.handler.codec.http.websocketx.extensions.WebSocketExtensionData; @@ -66,7 +70,7 @@ public class PerMessageDeflateClientExtensionHandshakerTest { new WebSocketExtensionData(PERMESSAGE_DEFLATE_EXTENSION, Collections.emptyMap())); assertNotNull(extension); - assertEquals(WebSocketClientExtension.RSV1, extension.rsv()); + assertEquals(RSV1, extension.rsv()); assertTrue(extension.newExtensionDecoder() instanceof PerMessageDeflateDecoder); assertTrue(extension.newExtensionEncoder() instanceof PerMessageDeflateEncoder); } @@ -92,7 +96,7 @@ public class PerMessageDeflateClientExtensionHandshakerTest { // test assertNotNull(extension); - assertEquals(WebSocketClientExtension.RSV1, extension.rsv()); + assertEquals(RSV1, extension.rsv()); assertTrue(extension.newExtensionDecoder() instanceof PerMessageDeflateDecoder); assertTrue(extension.newExtensionEncoder() instanceof PerMessageDeflateEncoder); @@ -107,7 +111,7 @@ public class PerMessageDeflateClientExtensionHandshakerTest { // test assertNotNull(extension); - assertEquals(WebSocketClientExtension.RSV1, extension.rsv()); + assertEquals(RSV1, extension.rsv()); assertTrue(extension.newExtensionDecoder() instanceof PerMessageDeflateDecoder); assertTrue(extension.newExtensionEncoder() instanceof PerMessageDeflateEncoder); @@ -121,4 +125,61 @@ public class PerMessageDeflateClientExtensionHandshakerTest { // test assertNull(extension); } + + @Test + public void testDecoderNoClientContext() { + PerMessageDeflateClientExtensionHandshaker handshaker = + new PerMessageDeflateClientExtensionHandshaker(6, true, MAX_WINDOW_SIZE, true, false); + + byte[] firstPayload = new byte[] { + 76, -50, -53, 10, -62, 48, 20, 4, -48, 95, 41, 89, -37, 36, 77, 90, 31, -39, 41, -72, 112, 33, -120, 20, + 20, 119, -79, 70, 123, -95, 121, -48, 92, -116, 80, -6, -17, -58, -99, -37, -31, 12, 51, 19, 1, -9, -12, + 68, -111, -117, 25, 58, 111, 77, -127, -66, -64, -34, 20, 59, -64, -29, -2, 90, -100, -115, 30, 16, 114, + -68, 61, 29, 40, 89, -112, -73, 25, 35, 120, -105, -67, -32, -43, -70, -84, 120, -55, 69, 43, -124, 106, + -92, 18, -110, 114, -50, 111, 25, -3, 10, 17, -75, 13, 127, -84, 106, 90, -66, 84, -75, 84, 53, -89, + -75, 92, -3, -40, -61, 119, 49, -117, 30, 49, 68, -59, 88, 74, -119, -34, 1, -83, -7, -48, 124, -124, + -23, 16, 88, -118, 121, 54, -53, 1, 44, 32, 81, 19, 25, -115, -43, -32, -64, -67, -120, -110, -101, 121, + -2, 2 + }; + + byte[] secondPayload = new byte[] { + -86, 86, 42, 46, 77, 78, 78, 45, 6, 26, 83, 82, 84, -102, -86, 3, -28, 38, 21, 39, 23, 101, 38, -91, 2, + -51, -51, 47, 74, 73, 45, 114, -54, -49, -49, -10, 49, -78, -118, 112, 10, 9, 13, 118, 1, -102, 84, + -108, 90, 88, 10, 116, 27, -56, -84, 124, -112, -13, 16, 26, 116, -108, 18, -117, -46, -127, 6, 69, 99, + -45, 24, 91, 91, 11, 0 + }; + + Map parameters = Collections.singletonMap(CLIENT_NO_CONTEXT, null); + + WebSocketClientExtension extension = handshaker.handshakeExtension( + new WebSocketExtensionData(PERMESSAGE_DEFLATE_EXTENSION, parameters)); + assertNotNull(extension); + + EmbeddedChannel decoderChannel = new EmbeddedChannel(extension.newExtensionDecoder()); + assertTrue( + decoderChannel.writeInbound(new TextWebSocketFrame(true, RSV1, Unpooled.copiedBuffer(firstPayload)))); + TextWebSocketFrame firstFrameDecompressed = decoderChannel.readInbound(); + assertTrue( + decoderChannel.writeInbound(new TextWebSocketFrame(true, RSV1, Unpooled.copiedBuffer(secondPayload)))); + TextWebSocketFrame secondFrameDecompressed = decoderChannel.readInbound(); + + assertNotNull(firstFrameDecompressed); + assertNotNull(firstFrameDecompressed.content()); + assertTrue(firstFrameDecompressed instanceof TextWebSocketFrame); + assertEquals(firstFrameDecompressed.text(), + "{\"info\":\"Welcome to the BitMEX Realtime API.\",\"version\"" + + ":\"2018-10-02T22:53:23.000Z\",\"timestamp\":\"2018-10-15T06:43:40.437Z\"," + + "\"docs\":\"https://www.bitmex.com/app/wsAPI\",\"limit\":{\"remaining\":39}}"); + assertTrue(firstFrameDecompressed.release()); + + assertNotNull(secondFrameDecompressed); + assertNotNull(secondFrameDecompressed.content()); + assertTrue(secondFrameDecompressed instanceof TextWebSocketFrame); + assertEquals(secondFrameDecompressed.text(), + "{\"success\":true,\"subscribe\":\"orderBookL2:XBTUSD\"," + + "\"request\":{\"op\":\"subscribe\",\"args\":[\"orderBookL2:XBTUSD\"]}}"); + assertTrue(secondFrameDecompressed.release()); + + assertFalse(decoderChannel.finish()); + } }