[#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.
This commit is contained in:
Norman Maurer 2014-06-16 10:17:02 +02:00
parent 101b9ded33
commit a4bd566cef
2 changed files with 84 additions and 8 deletions

View File

@ -18,7 +18,6 @@ package io.netty.handler.codec.compression;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import java.nio.ByteOrder;
import java.util.List; import java.util.List;
import java.util.zip.CRC32; import java.util.zip.CRC32;
import java.util.zip.DataFormatException; import java.util.zip.DataFormatException;
@ -173,8 +172,9 @@ public class JdkZlibDecoder extends ZlibDecoder {
boolean readFooter = false; boolean readFooter = false;
byte[] outArray = decompressed.array(); byte[] outArray = decompressed.array();
while (!inflater.needsInput()) { while (!inflater.needsInput()) {
int outIndex = decompressed.arrayOffset() + decompressed.writerIndex(); int writerIndex = decompressed.writerIndex();
int length = outArray.length - outIndex; int outIndex = decompressed.arrayOffset() + writerIndex;
int length = decompressed.writableBytes();
if (length == 0) { if (length == 0) {
// completely filled the buffer allocate a new one and start to fill it // completely filled the buffer allocate a new one and start to fill it
@ -186,7 +186,7 @@ public class JdkZlibDecoder extends ZlibDecoder {
int outputLength = inflater.inflate(outArray, outIndex, length); int outputLength = inflater.inflate(outArray, outIndex, length);
if (outputLength > 0) { if (outputLength > 0) {
decompressed.writerIndex(decompressed.writerIndex() + outputLength); decompressed.writerIndex(writerIndex + outputLength);
if (crc != null) { if (crc != null) {
crc.update(outArray, outIndex, outputLength); crc.update(outArray, outIndex, outputLength);
} }

View File

@ -25,6 +25,8 @@ import org.junit.Test;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream;
import java.util.zip.DeflaterOutputStream;
import java.util.zip.GZIPOutputStream; import java.util.zip.GZIPOutputStream;
import static org.junit.Assert.*; 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_SMALL = new byte[128];
private static final byte[] BYTES_LARGE = new byte[1024 * 1024]; private static final byte[] BYTES_LARGE = new byte[1024 * 1024];
private static final byte[] BYTES_LARGE2 = ("<!--?xml version=\"1.0\" encoding=\"ISO-8859-1\"?-->\n" +
"<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" " +
"\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n" +
"<html xmlns=\"http://www.w3.org/1999/xhtml\" xml:lang=\"en\" lang=\"en\"><head>\n" +
" <title>Apache Tomcat</title>\n" +
"</head>\n" +
'\n' +
"<body>\n" +
"<h1>It works !</h1>\n" +
'\n' +
"<p>If you're seeing this page via a web browser, it means you've setup Tomcat successfully." +
" Congratulations!</p>\n" +
" \n" +
"<p>This is the default Tomcat home page." +
" It can be found on the local filesystem at: <code>/var/lib/tomcat7/webapps/ROOT/index.html</code></p>\n" +
'\n' +
"<p>Tomcat7 veterans might be pleased to learn that this system instance of Tomcat is installed with" +
" <code>CATALINA_HOME</code> in <code>/usr/share/tomcat7</code> and <code>CATALINA_BASE</code> in" +
" <code>/var/lib/tomcat7</code>, following the rules from" +
" <code>/usr/share/doc/tomcat7-common/RUNNING.txt.gz</code>.</p>\n" +
'\n' +
"<p>You might consider installing the following packages, if you haven't already done so:</p>\n" +
'\n' +
"<p><b>tomcat7-docs</b>: This package installs a web application that allows to browse the Tomcat 7" +
" documentation locally. Once installed, you can access it by clicking <a href=\"docs/\">here</a>.</p>\n" +
'\n' +
"<p><b>tomcat7-examples</b>: 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" +
" <a href=\"examples/\">here</a>.</p>\n" +
'\n' +
"<p><b>tomcat7-admin</b>: This package installs two web applications that can help managing this Tomcat" +
" instance. Once installed, you can access the <a href=\"manager/html\">manager webapp</a> and" +
" the <a href=\"host-manager/html\">host-manager webapp</a>.</p><p>\n" +
'\n' +
"</p><p>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 <code>/etc/tomcat7/tomcat-users.xml</code>.</p>\n" +
'\n' +
'\n' +
'\n' +
"</body></html>").getBytes(CharsetUtil.UTF_8);
static { static {
ThreadLocalRandom rand = ThreadLocalRandom.current(); ThreadLocalRandom rand = ThreadLocalRandom.current();
rand.nextBytes(BYTES_SMALL); rand.nextBytes(BYTES_SMALL);
@ -44,8 +89,9 @@ public abstract class ZlibTest {
@Test @Test
public void testGZIP2() throws Exception { public void testGZIP2() throws Exception {
ByteBuf data = Unpooled.wrappedBuffer("message".getBytes(CharsetUtil.UTF_8)); byte[] bytes = "message".getBytes(CharsetUtil.UTF_8);
ByteBuf deflatedData = Unpooled.wrappedBuffer(gzip("message")); ByteBuf data = Unpooled.wrappedBuffer(bytes);
ByteBuf deflatedData = Unpooled.wrappedBuffer(gzip(bytes));
EmbeddedChannel chDecoderGZip = new EmbeddedChannel(createDecoder(ZlibWrapper.GZIP)); EmbeddedChannel chDecoderGZip = new EmbeddedChannel(createDecoder(ZlibWrapper.GZIP));
try { try {
@ -168,6 +214,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 = (ByteBuf) 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 { private void testCompressSmall(ZlibWrapper encoderWrapper, ZlibWrapper decoderWrapper) throws Exception {
testCompress0(encoderWrapper, decoderWrapper, Unpooled.wrappedBuffer(BYTES_SMALL)); testCompress0(encoderWrapper, decoderWrapper, Unpooled.wrappedBuffer(BYTES_SMALL));
testCompress0(encoderWrapper, decoderWrapper, testCompress0(encoderWrapper, decoderWrapper,
@ -185,6 +252,7 @@ public abstract class ZlibTest {
testCompressNone(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB); testCompressNone(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB);
testCompressSmall(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB); testCompressSmall(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB);
testCompressLarge(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB); testCompressLarge(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB);
testCompressLarge2(ZlibWrapper.ZLIB, deflate(BYTES_LARGE2), BYTES_LARGE2);
} }
@Test @Test
@ -199,6 +267,7 @@ public abstract class ZlibTest {
testCompressNone(ZlibWrapper.GZIP, ZlibWrapper.GZIP); testCompressNone(ZlibWrapper.GZIP, ZlibWrapper.GZIP);
testCompressSmall(ZlibWrapper.GZIP, ZlibWrapper.GZIP); testCompressSmall(ZlibWrapper.GZIP, ZlibWrapper.GZIP);
testCompressLarge(ZlibWrapper.GZIP, ZlibWrapper.GZIP); testCompressLarge(ZlibWrapper.GZIP, ZlibWrapper.GZIP);
testCompressLarge2(ZlibWrapper.GZIP, gzip(BYTES_LARGE2), BYTES_LARGE2);
} }
@Test @Test
@ -222,12 +291,19 @@ public abstract class ZlibTest {
testCompressLarge(ZlibWrapper.GZIP, ZlibWrapper.ZLIB_OR_NONE); 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(); ByteArrayOutputStream out = new ByteArrayOutputStream();
GZIPOutputStream stream = new GZIPOutputStream(out); GZIPOutputStream stream = new GZIPOutputStream(out);
stream.write(message.getBytes(CharsetUtil.UTF_8)); stream.write(bytes);
stream.close(); stream.close();
return out.toByteArray(); 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();
}
} }