Correctly parse empty header values when Indeterminate-Length Message…

…s are used. (#57)

Motivation:

We did not correctly parse Indeterminate-Length Field Sections and so produced an assertion failure when a header with an empty value was received.

Modifications:

- Fix parsing code
- Add unti tests

Result:

Correctly parse headers with empty values

codec-bhttp/src/main/java/io/netty/incubator/codec/bhttp/BinaryHttpParser.java

@@ -492,6 +492,8 @@ private static int getIndeterminateLength(ByteBuf in) {
         if (!in.isReadable()) {
             return -1;
         }
+        // Start with the name which can never be length of 0.
+        boolean name = true;
         for (int sumBytes = 0; sumBytes < in.readableBytes();) {
             int idx = in.readerIndex() + sumBytes;
             int possibleTerminatorBytes = numBytesForVariableLengthIntegerFromByte(in.getByte(idx));
@@ -504,9 +506,14 @@ private static int getIndeterminateLength(ByteBuf in) {
             if (in.readableBytes() < sumBytes) {
                 return -1;
             }
-            if (possibleTerminator == 0) {
+            if (name && possibleTerminator == 0) {
+                // If we are currently parsing the name length and found 0 we know that it must be the terminator
+                // as a name cane never be length of 0.
                 return sumBytes - possibleTerminatorBytes;
             }
+            // We flip between name and not name parsing, as after a name must always follow a value even if its
+            // length of 0.
+            name = !name;
         }
         return -1;
     }

codec-bhttp/src/test/java/io/netty/incubator/codec/bhttp/BinaryHttpHeadersTest.java

@@ -49,4 +49,10 @@ public void customPseudoHeaderNameInTrailer() {
         HttpHeaders headers = BinaryHttpHeaders.newTrailers(true);
         assertThrows(IllegalArgumentException.class, () -> headers.set(":custom", "x"));
     }
+
+    @Test
+    public void emptyHeaderValue() {
+        HttpHeaders headers = BinaryHttpHeaders.newHeaders(true);
+        headers.set("name", "");
+    }
 }

codec-bhttp/src/test/java/io/netty/incubator/codec/bhttp/BinaryHttpMessageEndToEndTest.java

@@ -565,6 +565,38 @@ void testFullMessageWithContentAndWithoutTrailers(boolean fragmented) throws IOE
         assertFalse(reader.finishAndReleaseAll());
     }
 
+    @ParameterizedTest
+    @ValueSource(booleans = { true, false })
+    void testMessageWithEmptyHeader(boolean fragmented) {
+        EmbeddedChannel writer = newWriter();
+        EmbeddedChannel reader = newReader();
+
+        M message = newHttpMessage();
+        message.headers().set("x-test-header", "");
+        message.headers().set("x-test-header2", "test-value2");
+        assertTrue(writer.writeOutbound(message));
+
+        LastHttpContent content = new DefaultLastHttpContent();
+        assertTrue(writer.writeOutbound(content));
+
+        transfer(writer, reader, fragmented);
+
+        M readMessage = reader.readInbound();
+        assertHttpMessage(readMessage);
+
+        HttpHeaders readHeaders = readMessage.headers();
+        assertEquals(2, readHeaders.size());
+        assertEquals("", readHeaders.get("x-test-header"));
+        assertEquals("test-value2", readHeaders.get("x-test-header2"));
+
+        LastHttpContent readLastContent = reader.readInbound();
+        assertEquals(0, readLastContent.content().readableBytes());
+        HttpHeaders readLastHeaders = readLastContent.trailingHeaders();
+        assertEquals(0, readLastHeaders.size());
+        readLastContent.release();
+        assertFalse(reader.finishAndReleaseAll());
+    }
+
     private static void assertContentWithoutTrailers(EmbeddedChannel reader, byte[] expectedContent)
             throws IOException {
         try (ByteArrayOutputStream contentWriter = new ByteArrayOutputStream()) {