8160218: HPack decoder fails when processing header in multiple ByteBuffers

Reviewed-by: michaelm
This commit is contained in:
Pavel Rappo 2016-06-29 10:19:48 +01:00
parent 50623e2781
commit 27036a761f
4 changed files with 168 additions and 3 deletions

View File

@ -298,7 +298,7 @@ class Http2Connection implements BufferHandler {
ByteBuffer[] buffers = frame.getHeaderBlock();
for (int i = 0; i < buffers.length; i++) {
hpackIn.decode(buffers[i], endOfHeaders, decoder);
hpackIn.decode(buffers[i], endOfHeaders && (i == buffers.length - 1), decoder);
}
}

View File

@ -78,6 +78,7 @@ final class StringReader {
if (isLast) {
input.limit(input.position() + remainingLength);
}
remainingLength -= Math.min(input.remaining(), remainingLength);
if (huffman) {
huffmanReader.read(input, output, isLast);
} else {
@ -85,6 +86,7 @@ final class StringReader {
}
if (isLast) {
input.limit(oldLimit);
state = DONE;
}
return isLast;
}

View File

@ -27,14 +27,20 @@ import org.testng.annotations.Test;
import java.io.UncheckedIOException;
import java.net.ProtocolException;
import java.nio.ByteBuffer;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static sun.net.httpclient.hpack.TestHelper.*;
//
// Tests whose names start with "testX" are the ones captured from real HPACK
// use cases
//
public final class DecoderTest {
//
@ -138,6 +144,23 @@ public final class DecoderTest {
// @formatter:on
}
@Test
public void example5AllSplits() {
// @formatter:off
testAllSplits(
"8286 8441 0f77 7777 2e65 7861 6d70 6c65\n" +
"2e63 6f6d",
"[ 1] (s = 57) :authority: www.example.com\n" +
" Table size: 57",
":method: GET\n" +
":scheme: http\n" +
":path: /\n" +
":authority: www.example.com");
// @formatter:on
}
//
// http://tools.ietf.org/html/rfc7541#appendix-C.4
//
@ -334,6 +357,45 @@ public final class DecoderTest {
// @formatter:on
}
@Test
public void testX1() {
// Supplier of a decoder with a particular state
Supplier<Decoder> s = () -> {
Decoder d = new Decoder(4096);
// @formatter:off
test(d, "88 76 92 ca 54 a7 d7 f4 fa ec af ed 6d da 61 d7 bb 1e ad ff" +
"df 61 97 c3 61 be 94 13 4a 65 b6 a5 04 00 b8 a0 5a b8 db 77" +
"1b 71 4c 5a 37 ff 0f 0d 84 08 00 00 03",
"[ 1] (s = 65) date: Fri, 24 Jun 2016 14:55:56 GMT\n" +
"[ 2] (s = 59) server: Jetty(9.3.z-SNAPSHOT)\n" +
" Table size: 124",
":status: 200\n" +
"server: Jetty(9.3.z-SNAPSHOT)\n" +
"date: Fri, 24 Jun 2016 14:55:56 GMT\n" +
"content-length: 100000"
);
// @formatter:on
return d;
};
// For all splits of the following data fed to the supplied decoder we
// must get what's expected
// @formatter:off
testAllSplits(s,
"88 bf be 0f 0d 84 08 00 00 03",
"[ 1] (s = 65) date: Fri, 24 Jun 2016 14:55:56 GMT\n" +
"[ 2] (s = 59) server: Jetty(9.3.z-SNAPSHOT)\n" +
" Table size: 124",
":status: 200\n" +
"server: Jetty(9.3.z-SNAPSHOT)\n" +
"date: Fri, 24 Jun 2016 14:55:56 GMT\n" +
"content-length: 100000");
// @formatter:on
}
//
// This test is missing in the spec
//
@ -567,6 +629,38 @@ public final class DecoderTest {
test(new Decoder(4096), hexdump, headerTable, headerList);
}
private static void testAllSplits(String hexdump,
String expectedHeaderTable,
String expectedHeaderList) {
testAllSplits(() -> new Decoder(256), hexdump, expectedHeaderTable, expectedHeaderList);
}
private static void testAllSplits(Supplier<Decoder> supplier, String hexdump,
String expectedHeaderTable, String expectedHeaderList) {
ByteBuffer source = SpecHelper.toBytes(hexdump);
BuffersTestingKit.forEachSplit(source, iterable -> {
List<String> actual = new LinkedList<>();
Iterator<? extends ByteBuffer> i = iterable.iterator();
if (!i.hasNext()) {
return;
}
Decoder d = supplier.get();
do {
ByteBuffer n = i.next();
d.decode(n, !i.hasNext(), (name, value) -> {
if (value == null) {
actual.add(name.toString());
} else {
actual.add(name + ": " + value);
}
});
} while (i.hasNext());
assertEquals(d.getTable().getStateString(), expectedHeaderTable);
assertEquals(actual.stream().collect(Collectors.joining("\n")), expectedHeaderList);
});
}
//
// Sometimes we need to keep the same decoder along several runs,
// as it models the same connection

View File

@ -24,17 +24,23 @@ package sun.net.httpclient.hpack;
import org.testng.annotations.Test;
import java.nio.Buffer;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.Function;
import static sun.net.httpclient.hpack.BuffersTestingKit.concat;
import static sun.net.httpclient.hpack.BuffersTestingKit.forEachSplit;
import static sun.net.httpclient.hpack.SpecHelper.toHexdump;
import static sun.net.httpclient.hpack.TestHelper.assertVoidThrows;
import static java.util.Arrays.asList;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
import static sun.net.httpclient.hpack.SpecHelper.toHexdump;
import static sun.net.httpclient.hpack.TestHelper.assertVoidThrows;
// TODO: map textual representation of commands from the spec to actual
// calls to encoder (actually, this is a good idea for decoder as well)
@ -198,6 +204,61 @@ public final class EncoderTest {
// @formatter:on
}
@Test
public void example5AllSplits() {
List<Consumer<Encoder>> actions = new LinkedList<>();
actions.add(e -> e.indexed(2));
actions.add(e -> e.indexed(6));
actions.add(e -> e.indexed(4));
actions.add(e -> e.literalWithIndexing(1, "www.example.com", false));
encodeAllSplits(
actions,
"8286 8441 0f77 7777 2e65 7861 6d70 6c65\n" +
"2e63 6f6d",
"[ 1] (s = 57) :authority: www.example.com\n" +
" Table size: 57");
}
private static void encodeAllSplits(Iterable<Consumer<Encoder>> consumers,
String expectedHexdump,
String expectedTableState) {
ByteBuffer buffer = SpecHelper.toBytes(expectedHexdump);
erase(buffer); // Zeroed buffer of size needed to hold the encoding
forEachSplit(buffer, iterable -> {
List<ByteBuffer> copy = new LinkedList<>();
iterable.forEach(b -> copy.add(ByteBuffer.allocate(b.remaining())));
Iterator<ByteBuffer> output = copy.iterator();
if (!output.hasNext()) {
throw new IllegalStateException("No buffers to encode to");
}
Encoder e = newCustomEncoder(256); // FIXME: pull up (as a parameter)
drainInitialUpdate(e);
boolean encoded;
ByteBuffer b = output.next();
for (Consumer<Encoder> c : consumers) {
c.accept(e);
do {
encoded = e.encode(b);
if (!encoded) {
if (output.hasNext()) {
b = output.next();
} else {
throw new IllegalStateException("No room for encoding");
}
}
}
while (!encoded);
}
copy.forEach(Buffer::flip);
ByteBuffer data = concat(copy);
test(e, data, expectedHexdump, expectedTableState);
});
}
//
// http://tools.ietf.org/html/rfc7541#appendix-C.4
//
@ -620,4 +681,12 @@ public final class EncoderTest {
b.flip();
} while (!done);
}
private static void erase(ByteBuffer buffer) {
buffer.clear();
while (buffer.hasRemaining()) {
buffer.put((byte) 0);
}
buffer.clear();
}
}