8160218: HPack decoder fails when processing header in multiple ByteBuffers
Reviewed-by: michaelm
--- a/jdk/src/java.httpclient/share/classes/java/net/http/Http2Connection.java Wed Jun 29 10:07:15 2016 +0200
+++ b/jdk/src/java.httpclient/share/classes/java/net/http/Http2Connection.java Wed Jun 29 10:19:48 2016 +0100
@@ -298,7 +298,7 @@
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);
}
}
--- a/jdk/src/java.httpclient/share/classes/sun/net/httpclient/hpack/StringReader.java Wed Jun 29 10:07:15 2016 +0200
+++ b/jdk/src/java.httpclient/share/classes/sun/net/httpclient/hpack/StringReader.java Wed Jun 29 10:19:48 2016 +0100
@@ -78,6 +78,7 @@
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 @@
}
if (isLast) {
input.limit(oldLimit);
+ state = DONE;
}
return isLast;
}
--- a/jdk/test/java/net/httpclient/http2/java.httpclient/sun/net/httpclient/hpack/DecoderTest.java Wed Jun 29 10:07:15 2016 +0200
+++ b/jdk/test/java/net/httpclient/http2/java.httpclient/sun/net/httpclient/hpack/DecoderTest.java Wed Jun 29 10:19:48 2016 +0100
@@ -27,14 +27,20 @@
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 @@
// @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 @@
// @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 @@
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
--- a/jdk/test/java/net/httpclient/http2/java.httpclient/sun/net/httpclient/hpack/EncoderTest.java Wed Jun 29 10:07:15 2016 +0200
+++ b/jdk/test/java/net/httpclient/http2/java.httpclient/sun/net/httpclient/hpack/EncoderTest.java Wed Jun 29 10:19:48 2016 +0100
@@ -24,17 +24,23 @@
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 @@
// @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 @@
b.flip();
} while (!done);
}
+
+ private static void erase(ByteBuffer buffer) {
+ buffer.clear();
+ while (buffer.hasRemaining()) {
+ buffer.put((byte) 0);
+ }
+ buffer.clear();
+ }
}