diff --git a/codec/src/main/java/io/netty/handler/codec/compression/Snappy.java b/codec/src/main/java/io/netty/handler/codec/compression/Snappy.java index 80bbdcf571..c6454cd49c 100644 --- a/codec/src/main/java/io/netty/handler/codec/compression/Snappy.java +++ b/codec/src/main/java/io/netty/handler/codec/compression/Snappy.java @@ -29,6 +29,7 @@ public class Snappy { // used as a return value to indicate that we haven't yet read our full preamble private static final int PREAMBLE_NOT_FULL = -1; + private static final int NOT_ENOUGH_INPUT = -1; // constants for the tag types private static final int LITERAL = 0; @@ -38,6 +39,7 @@ public class Snappy { private State state = State.READY; private byte tag; + private int written; private static enum State { READY, @@ -50,6 +52,7 @@ public class Snappy { public void reset() { state = State.READY; tag = 0; + written = 0; } public void encode(ByteBuf in, ByteBuf out, int length) { @@ -307,34 +310,43 @@ public class Snappy { } break; case READING_LITERAL: - if (decodeLiteral(tag, in, out)) { + int literalWritten = decodeLiteral(tag, in, out); + if (literalWritten != NOT_ENOUGH_INPUT) { state = State.READING_TAG; + written += literalWritten; } else { // Need to wait for more data return; } break; case READING_COPY: + int decodeWritten; switch (tag & 0x03) { case COPY_1_BYTE_OFFSET: - if (decodeCopyWith1ByteOffset(tag, in, out)) { + decodeWritten = decodeCopyWith1ByteOffset(tag, in, out, written); + if (decodeWritten != NOT_ENOUGH_INPUT) { state = State.READING_TAG; + written += decodeWritten; } else { // Need to wait for more data return; } break; case COPY_2_BYTE_OFFSET: - if (decodeCopyWith2ByteOffset(tag, in, out)) { + decodeWritten = decodeCopyWith2ByteOffset(tag, in, out, written); + if (decodeWritten != NOT_ENOUGH_INPUT) { state = State.READING_TAG; + written += decodeWritten; } else { // Need to wait for more data return; } break; case COPY_4_BYTE_OFFSET: - if (decodeCopyWith4ByteOffset(tag, in, out)) { + decodeWritten = decodeCopyWith4ByteOffset(tag, in, out, written); + if (decodeWritten != NOT_ENOUGH_INPUT) { state = State.READING_TAG; + written += decodeWritten; } else { // Need to wait for more data return; @@ -381,27 +393,29 @@ public class Snappy { * used to encode part of the length of the data * @param in The input buffer to read the literal from * @param out The output buffer to write the literal to + * @return The number of bytes appended to the output buffer, or -1 to indicate + * "try again later" */ - private static boolean decodeLiteral(byte tag, ByteBuf in, ByteBuf out) { + private static int decodeLiteral(byte tag, ByteBuf in, ByteBuf out) { in.markReaderIndex(); int length; switch(tag >> 2 & 0x3F) { case 60: if (!in.isReadable()) { - return false; + return NOT_ENOUGH_INPUT; } length = in.readUnsignedByte(); break; case 61: if (in.readableBytes() < 2) { - return false; + return NOT_ENOUGH_INPUT; } length = in.readUnsignedByte() | in.readUnsignedByte() << 8; break; case 62: if (in.readableBytes() < 3) { - return false; + return NOT_ENOUGH_INPUT; } length = in.readUnsignedByte() | in.readUnsignedByte() << 8 @@ -409,7 +423,7 @@ public class Snappy { break; case 64: if (in.readableBytes() < 4) { - return false; + return NOT_ENOUGH_INPUT; } length = in.readUnsignedByte() | in.readUnsignedByte() << 8 @@ -423,11 +437,11 @@ public class Snappy { if (in.readableBytes() < length) { in.resetReaderIndex(); - return false; + return NOT_ENOUGH_INPUT; } out.writeBytes(in, length); - return true; + return length; } /** @@ -439,18 +453,20 @@ public class Snappy { * the length and part of the offset * @param in The input buffer to read from * @param out The output buffer to write to + * @return The number of bytes appended to the output buffer, or -1 to indicate + * "try again later" * @throws CompressionException If the read offset is invalid */ - private static boolean decodeCopyWith1ByteOffset(byte tag, ByteBuf in, ByteBuf out) { + private static int decodeCopyWith1ByteOffset(byte tag, ByteBuf in, ByteBuf out, int writtenSoFar) { if (!in.isReadable()) { - return false; + return NOT_ENOUGH_INPUT; } - int initialIndex = out.readableBytes(); + int initialIndex = out.writerIndex(); int length = 4 + ((tag & 0x01c) >> 2); - int offset = (tag & 0x0e0) << 8 | in.readUnsignedByte(); + int offset = (tag & 0x0e0) << 8 >> 5 | in.readUnsignedByte(); - validateOffset(offset, initialIndex); + validateOffset(offset, writtenSoFar); out.markReaderIndex(); if (offset < length) { @@ -469,7 +485,7 @@ public class Snappy { } out.resetReaderIndex(); - return true; + return length; } /** @@ -482,17 +498,19 @@ public class Snappy { * @param in The input buffer to read from * @param out The output buffer to write to * @throws CompressionException If the read offset is invalid + * @return The number of bytes appended to the output buffer, or -1 to indicate + * "try again later" */ - private static boolean decodeCopyWith2ByteOffset(byte tag, ByteBuf in, ByteBuf out) { + private static int decodeCopyWith2ByteOffset(byte tag, ByteBuf in, ByteBuf out, int writtenSoFar) { if (in.readableBytes() < 2) { - return false; + return NOT_ENOUGH_INPUT; } - int initialIndex = out.readableBytes(); + int initialIndex = out.writerIndex(); int length = 1 + (tag >> 2 & 0x03f); int offset = in.readUnsignedByte() | in.readUnsignedByte() << 8; - validateOffset(offset, initialIndex); + validateOffset(offset, writtenSoFar); out.markReaderIndex(); if (offset < length) { @@ -511,7 +529,7 @@ public class Snappy { } out.resetReaderIndex(); - return true; + return length; } /** @@ -523,21 +541,23 @@ public class Snappy { * the length and part of the offset * @param in The input buffer to read from * @param out The output buffer to write to + * @return The number of bytes appended to the output buffer, or -1 to indicate + * "try again later" * @throws CompressionException If the read offset is invalid */ - private static boolean decodeCopyWith4ByteOffset(byte tag, ByteBuf in, ByteBuf out) { + private static int decodeCopyWith4ByteOffset(byte tag, ByteBuf in, ByteBuf out, int writtenSoFar) { if (in.readableBytes() < 4) { - return false; + return NOT_ENOUGH_INPUT; } - int initialIndex = out.readableBytes(); + int initialIndex = out.writerIndex(); int length = 1 + (tag >> 2 & 0x03F); int offset = in.readUnsignedByte() | in.readUnsignedByte() << 8 | in.readUnsignedByte() << 16 | in.readUnsignedByte() << 24; - validateOffset(offset, initialIndex); + validateOffset(offset, writtenSoFar); out.markReaderIndex(); if (offset < length) { @@ -556,7 +576,7 @@ public class Snappy { } out.resetReaderIndex(); - return true; + return length; } /** diff --git a/codec/src/test/java/io/netty/handler/codec/compression/SnappyIntegrationTest.java b/codec/src/test/java/io/netty/handler/codec/compression/SnappyIntegrationTest.java index ef9fa3cde4..ea3ffa0418 100644 --- a/codec/src/test/java/io/netty/handler/codec/compression/SnappyIntegrationTest.java +++ b/codec/src/test/java/io/netty/handler/codec/compression/SnappyIntegrationTest.java @@ -18,7 +18,6 @@ package io.netty.handler.codec.compression; import io.netty.buffer.ByteBuf; import io.netty.channel.embedded.EmbeddedByteChannel; import io.netty.util.CharsetUtil; -import org.junit.Ignore; import org.junit.Test; import java.util.Random; @@ -73,201 +72,20 @@ public class SnappyIntegrationTest { // These tests were found using testRandom() with large RANDOM_RUNS. - // FIXME: Fix and unignore these failing test. - // Fixing one test might fix other tests, too. In such a case, please remove the redundant tests - // to shorten the test duration. + // Tests that copies do not attempt to overrun into a previous frame chunk @Test - @Ignore public void test5323211032315942961(){ testWithSeed(5323211032315942961L); } - @Test - @Ignore - public void test5956997334949727845(){ - testWithSeed(5956997334949727845L); - } - - @Test - @Ignore - public void test987469642329137223(){ - testWithSeed(987469642329137223L); - } - - @Test - @Ignore - public void test5798580555644912844(){ - testWithSeed(5798580555644912844L); - } - - @Test - @Ignore - public void test6700065949554745127(){ - testWithSeed(6700065949554745127L); - } - - @Test - @Ignore - public void test6262781682093393396(){ - testWithSeed(6262781682093393396L); - } - - @Test - @Ignore - public void test358737913816100034(){ - testWithSeed(358737913816100034L); - } - - @Test - @Ignore - public void test7510836247514432004(){ - testWithSeed(7510836247514432004L); - } - - @Test - @Ignore - public void test2135280061177091902(){ - testWithSeed(2135280061177091902L); - } - - @Test - @Ignore - public void test6057787589708412305(){ - testWithSeed(6057787589708412305L); - } - - @Test - @Ignore - public void test8411340917985079134(){ - testWithSeed(8411340917985079134L); - } - + // Tests that when generating the hash lookup table for finding copies, we + // do not exceed the length of the input when there are no copies @Test public void test7088170877360183401() { testWithSeed(7088170877360183401L); } - @Test - public void test7354134887958970957() { - testWithSeed(7354134887958970957L); - } - - @Test - public void test265123194979327191() { - testWithSeed(265123194979327191L); - } - - @Test - public void test4730809278569396315() { - testWithSeed(4730809278569396315L); - } - - @Test - public void test2048930638087468368() { - testWithSeed(2048930638087468368L); - } - - @Test - public void test7896596325568044047() { - testWithSeed(7896596325568044047L); - } - - @Test - public void test3397027453071844468() { - testWithSeed(3397027453071844468L); - } - - @Test - public void test4157969824584948251() { - testWithSeed(4157969824584948251L); - } - - @Test - public void test4753934068873093038(){ - testWithSeed(4753934068873093038L); - } - - @Test - public void test5925569922155870475(){ - testWithSeed(5925569922155870475L); - } - - @Test - public void test7269843964854027868(){ - testWithSeed(7269843964854027868L); - } - - @Test - public void test3588069159611484749(){ - testWithSeed(3588069159611484749L); - } - - @Test - public void test6779187833722801305(){ - testWithSeed(6779187833722801305L); - } - - @Test - public void test4686313400062453552(){ - testWithSeed(4686313400062453552L); - } - - @Test - public void test2991001407882611338(){ - testWithSeed(2991001407882611338L); - } - - @Test - public void test4943660132286394340(){ - testWithSeed(4943660132286394340L); - } - - @Test - public void test1922387899411087229(){ - testWithSeed(1922387899411087229L); - } - - @Test - public void test1224584698616862175(){ - testWithSeed(1224584698616862175L); - } - - @Test - public void test985619956074250243(){ - testWithSeed(985619956074250243L); - } - - @Test - public void test930789503984237252(){ - testWithSeed(930789503984237252L); - } - - @Test - public void test1480332326718517164(){ - testWithSeed(1480332326718517164L); - } - - @Test - public void test8997827733405782755(){ - testWithSeed(8997827733405782755L); - } - - @Test - public void test7059191520894204311(){ - testWithSeed(7059191520894204311L); - } - - @Test - public void test4484339162540496103(){ - testWithSeed(4484339162540496103L); - } - - @Test - public void test1939623429893866631(){ - testWithSeed(1939623429893866631L); - } - @Test public void testRandom() throws Throwable { Random rnd = new Random();