From 984b0aa961b21845ab92cd2cddab3f2ee354f4f1 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 16 Jun 2014 10:17:02 +0200 Subject: [PATCH] [#2572] Correctly calculate length of output buffer before inflate to fix IndexOutOfBoundException Motivation: JdkZlibDecoder fails to decode because the length of the output buffer is not calculated correctly. This can cause an IndexOutOfBoundsException or data-corruption when the PooledByteBuffAllocator is used. Modifications: Correctly calculate the length Result: No more IndexOutOfBoundsException or data-corruption. --- .../codec/compression/JdkZlibDecoder.java | 7 +- .../handler/codec/compression/ZlibTest.java | 84 ++++++++++++++++++- 2 files changed, 84 insertions(+), 7 deletions(-) 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 3e7c198fe1..a35f48fda1 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 @@ -172,8 +172,9 @@ public class JdkZlibDecoder extends ZlibDecoder { boolean readFooter = false; byte[] outArray = decompressed.array(); while (!inflater.needsInput()) { - int outIndex = decompressed.arrayOffset() + decompressed.writerIndex(); - int length = outArray.length - outIndex; + int writerIndex = decompressed.writerIndex(); + int outIndex = decompressed.arrayOffset() + writerIndex; + int length = decompressed.writableBytes(); if (length == 0) { // completely filled the buffer allocate a new one and start to fill it @@ -185,7 +186,7 @@ public class JdkZlibDecoder extends ZlibDecoder { int outputLength = inflater.inflate(outArray, outIndex, length); if (outputLength > 0) { - decompressed.writerIndex(decompressed.writerIndex() + outputLength); + decompressed.writerIndex(writerIndex + outputLength); if (crc != null) { crc.update(outArray, outIndex, outputLength); } 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 ed3622c374..8f32a7189a 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 @@ -25,6 +25,8 @@ import org.junit.Test; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; +import java.util.zip.DeflaterOutputStream; import java.util.zip.GZIPOutputStream; import static org.junit.Assert.*; @@ -33,6 +35,49 @@ public abstract class ZlibTest { private static final byte[] BYTES_SMALL = new byte[128]; private static final byte[] BYTES_LARGE = new byte[1024 * 1024]; + private static final byte[] BYTES_LARGE2 = ("\n" + + "\n" + + "\n" + + " Apache Tomcat\n" + + "\n" + + '\n' + + "\n" + + "

It works !

\n" + + '\n' + + "

If you're seeing this page via a web browser, it means you've setup Tomcat successfully." + + " Congratulations!

\n" + + " \n" + + "

This is the default Tomcat home page." + + " It can be found on the local filesystem at: /var/lib/tomcat7/webapps/ROOT/index.html

\n" + + '\n' + + "

Tomcat7 veterans might be pleased to learn that this system instance of Tomcat is installed with" + + " CATALINA_HOME in /usr/share/tomcat7 and CATALINA_BASE in" + + " /var/lib/tomcat7, following the rules from" + + " /usr/share/doc/tomcat7-common/RUNNING.txt.gz.

\n" + + '\n' + + "

You might consider installing the following packages, if you haven't already done so:

\n" + + '\n' + + "

tomcat7-docs: This package installs a web application that allows to browse the Tomcat 7" + + " documentation locally. Once installed, you can access it by clicking here.

\n" + + '\n' + + "

tomcat7-examples: This package installs a web application that allows to access the Tomcat" + + " 7 Servlet and JSP examples. Once installed, you can access it by clicking" + + " here.

\n" + + '\n' + + "

tomcat7-admin: This package installs two web applications that can help managing this Tomcat" + + " instance. Once installed, you can access the manager webapp and" + + " the host-manager webapp.

\n" + + '\n' + + "

NOTE: For security reasons, using the manager webapp is restricted" + + " to users with role \"manager\"." + + " The host-manager webapp is restricted to users with role \"admin\". Users are " + + "defined in /etc/tomcat7/tomcat-users.xml.

\n" + + '\n' + + '\n' + + '\n' + + "").getBytes(CharsetUtil.UTF_8); + static { ThreadLocalRandom rand = ThreadLocalRandom.current(); rand.nextBytes(BYTES_SMALL); @@ -44,8 +89,9 @@ public abstract class ZlibTest { @Test public void testGZIP2() throws Exception { - ByteBuf data = Unpooled.wrappedBuffer("message".getBytes(CharsetUtil.UTF_8)); - ByteBuf deflatedData = Unpooled.wrappedBuffer(gzip("message")); + byte[] bytes = "message".getBytes(CharsetUtil.UTF_8); + ByteBuf data = Unpooled.wrappedBuffer(bytes); + ByteBuf deflatedData = Unpooled.wrappedBuffer(gzip(bytes)); EmbeddedChannel chDecoderGZip = new EmbeddedChannel(createDecoder(ZlibWrapper.GZIP)); try { @@ -169,6 +215,27 @@ public abstract class ZlibTest { } } } + + // Test for https://github.com/netty/netty/issues/2572 + private void testCompressLarge2(ZlibWrapper decoderWrapper, byte[] compressed, byte[] data) throws Exception { + EmbeddedChannel chDecoder = new EmbeddedChannel(createDecoder(decoderWrapper)); + chDecoder.writeInbound(Unpooled.wrappedBuffer(compressed)); + assertTrue(chDecoder.finish()); + + ByteBuf decoded = Unpooled.buffer(data.length); + + for (;;) { + ByteBuf buf = chDecoder.readInbound(); + if (buf == null) { + break; + } + decoded.writeBytes(buf); + buf.release(); + } + assertEquals(Unpooled.wrappedBuffer(data), decoded); + decoded.release(); + } + private void testCompressSmall(ZlibWrapper encoderWrapper, ZlibWrapper decoderWrapper) throws Exception { testCompress0(encoderWrapper, decoderWrapper, Unpooled.wrappedBuffer(BYTES_SMALL)); testCompress0(encoderWrapper, decoderWrapper, @@ -186,6 +253,7 @@ public abstract class ZlibTest { testCompressNone(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB); testCompressSmall(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB); testCompressLarge(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB); + testCompressLarge2(ZlibWrapper.ZLIB, deflate(BYTES_LARGE2), BYTES_LARGE2); } @Test @@ -200,6 +268,7 @@ public abstract class ZlibTest { testCompressNone(ZlibWrapper.GZIP, ZlibWrapper.GZIP); testCompressSmall(ZlibWrapper.GZIP, ZlibWrapper.GZIP); testCompressLarge(ZlibWrapper.GZIP, ZlibWrapper.GZIP); + testCompressLarge2(ZlibWrapper.GZIP, gzip(BYTES_LARGE2), BYTES_LARGE2); } @Test @@ -223,12 +292,19 @@ public abstract class ZlibTest { testCompressLarge(ZlibWrapper.GZIP, ZlibWrapper.ZLIB_OR_NONE); } - private static byte[] gzip(String message) throws IOException { + private static byte[] gzip(byte[] bytes) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); GZIPOutputStream stream = new GZIPOutputStream(out); - stream.write(message.getBytes(CharsetUtil.UTF_8)); + stream.write(bytes); stream.close(); return out.toByteArray(); } + private static byte[] deflate(byte[] bytes) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + OutputStream stream = new DeflaterOutputStream(out); + stream.write(bytes); + stream.close(); + return out.toByteArray(); + } }