[#1069] Snappy decoding fixes

* Correct reading offset of 1-byte-offset copies
* Keep track of how much we've written so far in order to validate offsets
* Uncomment and reduce number of tests
This commit is contained in:
Luke Wood 2013-02-25 16:04:40 +00:00
parent e65e17c724
commit e45db60b5e
2 changed files with 50 additions and 212 deletions

View File

@ -29,6 +29,7 @@ public class Snappy {
// used as a return value to indicate that we haven't yet read our full preamble // 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 PREAMBLE_NOT_FULL = -1;
private static final int NOT_ENOUGH_INPUT = -1;
// constants for the tag types // constants for the tag types
private static final int LITERAL = 0; private static final int LITERAL = 0;
@ -38,6 +39,7 @@ public class Snappy {
private State state = State.READY; private State state = State.READY;
private byte tag; private byte tag;
private int written;
private static enum State { private static enum State {
READY, READY,
@ -50,6 +52,7 @@ public class Snappy {
public void reset() { public void reset() {
state = State.READY; state = State.READY;
tag = 0; tag = 0;
written = 0;
} }
public void encode(ByteBuf in, ByteBuf out, int length) { public void encode(ByteBuf in, ByteBuf out, int length) {
@ -307,34 +310,43 @@ public class Snappy {
} }
break; break;
case READING_LITERAL: case READING_LITERAL:
if (decodeLiteral(tag, in, out)) { int literalWritten = decodeLiteral(tag, in, out);
if (literalWritten != NOT_ENOUGH_INPUT) {
state = State.READING_TAG; state = State.READING_TAG;
written += literalWritten;
} else { } else {
// Need to wait for more data // Need to wait for more data
return; return;
} }
break; break;
case READING_COPY: case READING_COPY:
int decodeWritten;
switch (tag & 0x03) { switch (tag & 0x03) {
case COPY_1_BYTE_OFFSET: 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; state = State.READING_TAG;
written += decodeWritten;
} else { } else {
// Need to wait for more data // Need to wait for more data
return; return;
} }
break; break;
case COPY_2_BYTE_OFFSET: 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; state = State.READING_TAG;
written += decodeWritten;
} else { } else {
// Need to wait for more data // Need to wait for more data
return; return;
} }
break; break;
case COPY_4_BYTE_OFFSET: 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; state = State.READING_TAG;
written += decodeWritten;
} else { } else {
// Need to wait for more data // Need to wait for more data
return; return;
@ -381,27 +393,29 @@ public class Snappy {
* used to encode part of the length of the data * used to encode part of the length of the data
* @param in The input buffer to read the literal from * @param in The input buffer to read the literal from
* @param out The output buffer to write the literal to * @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(); in.markReaderIndex();
int length; int length;
switch(tag >> 2 & 0x3F) { switch(tag >> 2 & 0x3F) {
case 60: case 60:
if (!in.isReadable()) { if (!in.isReadable()) {
return false; return NOT_ENOUGH_INPUT;
} }
length = in.readUnsignedByte(); length = in.readUnsignedByte();
break; break;
case 61: case 61:
if (in.readableBytes() < 2) { if (in.readableBytes() < 2) {
return false; return NOT_ENOUGH_INPUT;
} }
length = in.readUnsignedByte() length = in.readUnsignedByte()
| in.readUnsignedByte() << 8; | in.readUnsignedByte() << 8;
break; break;
case 62: case 62:
if (in.readableBytes() < 3) { if (in.readableBytes() < 3) {
return false; return NOT_ENOUGH_INPUT;
} }
length = in.readUnsignedByte() length = in.readUnsignedByte()
| in.readUnsignedByte() << 8 | in.readUnsignedByte() << 8
@ -409,7 +423,7 @@ public class Snappy {
break; break;
case 64: case 64:
if (in.readableBytes() < 4) { if (in.readableBytes() < 4) {
return false; return NOT_ENOUGH_INPUT;
} }
length = in.readUnsignedByte() length = in.readUnsignedByte()
| in.readUnsignedByte() << 8 | in.readUnsignedByte() << 8
@ -423,11 +437,11 @@ public class Snappy {
if (in.readableBytes() < length) { if (in.readableBytes() < length) {
in.resetReaderIndex(); in.resetReaderIndex();
return false; return NOT_ENOUGH_INPUT;
} }
out.writeBytes(in, length); out.writeBytes(in, length);
return true; return length;
} }
/** /**
@ -439,18 +453,20 @@ public class Snappy {
* the length and part of the offset * the length and part of the offset
* @param in The input buffer to read from * @param in The input buffer to read from
* @param out The output buffer to write to * @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 * @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()) { if (!in.isReadable()) {
return false; return NOT_ENOUGH_INPUT;
} }
int initialIndex = out.readableBytes(); int initialIndex = out.writerIndex();
int length = 4 + ((tag & 0x01c) >> 2); 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(); out.markReaderIndex();
if (offset < length) { if (offset < length) {
@ -469,7 +485,7 @@ public class Snappy {
} }
out.resetReaderIndex(); out.resetReaderIndex();
return true; return length;
} }
/** /**
@ -482,17 +498,19 @@ public class Snappy {
* @param in The input buffer to read from * @param in The input buffer to read from
* @param out The output buffer to write to * @param out The output buffer to write to
* @throws CompressionException If the read offset is invalid * @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) { 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 length = 1 + (tag >> 2 & 0x03f);
int offset = in.readUnsignedByte() | in.readUnsignedByte() << 8; int offset = in.readUnsignedByte() | in.readUnsignedByte() << 8;
validateOffset(offset, initialIndex); validateOffset(offset, writtenSoFar);
out.markReaderIndex(); out.markReaderIndex();
if (offset < length) { if (offset < length) {
@ -511,7 +529,7 @@ public class Snappy {
} }
out.resetReaderIndex(); out.resetReaderIndex();
return true; return length;
} }
/** /**
@ -523,21 +541,23 @@ public class Snappy {
* the length and part of the offset * the length and part of the offset
* @param in The input buffer to read from * @param in The input buffer to read from
* @param out The output buffer to write to * @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 * @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) { 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 length = 1 + (tag >> 2 & 0x03F);
int offset = in.readUnsignedByte() | int offset = in.readUnsignedByte() |
in.readUnsignedByte() << 8 | in.readUnsignedByte() << 8 |
in.readUnsignedByte() << 16 | in.readUnsignedByte() << 16 |
in.readUnsignedByte() << 24; in.readUnsignedByte() << 24;
validateOffset(offset, initialIndex); validateOffset(offset, writtenSoFar);
out.markReaderIndex(); out.markReaderIndex();
if (offset < length) { if (offset < length) {
@ -556,7 +576,7 @@ public class Snappy {
} }
out.resetReaderIndex(); out.resetReaderIndex();
return true; return length;
} }
/** /**

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.embedded.EmbeddedByteChannel; import io.netty.channel.embedded.EmbeddedByteChannel;
import io.netty.util.CharsetUtil; import io.netty.util.CharsetUtil;
import org.junit.Ignore;
import org.junit.Test; import org.junit.Test;
import java.util.Random; import java.util.Random;
@ -73,201 +72,20 @@ public class SnappyIntegrationTest {
// These tests were found using testRandom() with large RANDOM_RUNS. // 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 @Test
@Ignore
public void test5323211032315942961(){ public void test5323211032315942961(){
testWithSeed(5323211032315942961L); testWithSeed(5323211032315942961L);
} }
@Test // Tests that when generating the hash lookup table for finding copies, we
@Ignore // do not exceed the length of the input when there are no copies
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);
}
@Test @Test
public void test7088170877360183401() { public void test7088170877360183401() {
testWithSeed(7088170877360183401L); 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 @Test
public void testRandom() throws Throwable { public void testRandom() throws Throwable {
Random rnd = new Random(); Random rnd = new Random();