From ed8c4e70f6efaff0d13b7e8a742d1472a5b6aece Mon Sep 17 00:00:00 2001 From: Alessandro Zoffoli <953519+AL333Z@users.noreply.github.com> Date: Fri, 29 Oct 2021 09:52:51 +0200 Subject: [PATCH] Fix gzip decoding when FLG.FHCRC is set (#11805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit   Motivation: The current implementation is not respecting the spec at https://www.ietf.org/rfc/rfc1952, section 2.3. > If FHCRC is set, a CRC16 for the gzip header is present, > immediately before the compressed data. The CRC16 consists > of the two least significant bytes of the CRC32 for all > bytes of the gzip header up to and not including the CRC16. The implementation is reusing the CRC32 logic without taking the two least significant bytes. Modifications: Use a method that respects the spec.   Result: When the FLG.FHCRC bit is set, the correct verification algorithm will be used. --- .../codec/compression/JdkZlibDecoder.java | 21 ++++++++- .../handler/codec/compression/ZlibTest.java | 44 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibDecoder.java b/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibDecoder.java index 96aa2ad8e62..1cd29e229b4 100644 --- a/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibDecoder.java +++ b/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibDecoder.java @@ -388,7 +388,7 @@ private boolean readGZIPHeader(ByteBuf in) { // fall through case PROCESS_FHCRC: if ((flags & FHCRC) != 0) { - if (!verifyCrc(in)) { + if (!verifyCrc16(in)) { return false; } } @@ -478,6 +478,25 @@ private boolean verifyCrc(ByteBuf in) { return true; } + private boolean verifyCrc16(ByteBuf in) { + if (in.readableBytes() < 2) { + return false; + } + long readCrc32 = crc.getValue(); + long crc16Value = 0; + long readCrc16 = 0; // the two least significant bytes from the CRC32 + for (int i = 0; i < 2; ++i) { + crc16Value |= (long) in.readUnsignedByte() << (i * 8); + readCrc16 |= ((readCrc32 >> (i * 8)) & 0xff) << (i * 8); + } + + if (crc16Value != readCrc16) { + throw new DecompressionException( + "CRC16 value mismatch. Expected: " + crc16Value + ", Got: " + readCrc16); + } + return true; + } + /* * Returns true if the cmf_flg parameter (think: first two bytes of a zlib stream) * indicates that this is a zlib stream. diff --git a/codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java b/codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java index 1f3602d71a1..ee7acdd0bad 100644 --- a/codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java +++ b/codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java @@ -133,6 +133,50 @@ public void testGZIP2() throws Exception { } } + @Test + public void testGZIP3() throws Exception { + byte[] bytes = "Foo".getBytes(CharsetUtil.UTF_8); + ByteBuf data = Unpooled.wrappedBuffer(bytes); + ByteBuf deflatedData = Unpooled.wrappedBuffer( + new byte[]{ + 31, -117, // magic number + 8, // CM + 2, // FLG.FHCRC + 0, 0, 0, 0, // MTIME + 0, // XFL + 7, // OS + -66, -77, // CRC16 + 115, -53, -49, 7, 0, // compressed blocks + -63, 35, 62, -76, // CRC32 + 3, 0, 0, 0 // ISIZE + } + ); + + EmbeddedChannel chDecoderGZip = new EmbeddedChannel(createDecoder(ZlibWrapper.GZIP)); + try { + while (deflatedData.isReadable()) { + chDecoderGZip.writeInbound(deflatedData.readRetainedSlice(1)); + } + deflatedData.release(); + assertTrue(chDecoderGZip.finish()); + ByteBuf buf = Unpooled.buffer(); + for (;;) { + ByteBuf b = chDecoderGZip.readInbound(); + if (b == null) { + break; + } + buf.writeBytes(b); + b.release(); + } + assertEquals(buf, data); + assertNull(chDecoderGZip.readInbound()); + data.release(); + buf.release(); + } finally { + dispose(chDecoderGZip); + } + } + private void testCompress0(ZlibWrapper encoderWrapper, ZlibWrapper decoderWrapper, ByteBuf data) throws Exception { EmbeddedChannel chEncoder = new EmbeddedChannel(createEncoder(encoderWrapper)); EmbeddedChannel chDecoderZlib = new EmbeddedChannel(createDecoder(decoderWrapper));