Fix an edge case HTTP/2 HPACK header decoding bug

Author: markt-asf

java/org/apache/coyote/http2/HPackHuffman.java

@@ -387,11 +387,14 @@ public static void decode(ByteBuffer data, int length, StringBuilder target) thr
                     if ((val & LOW_TERMINAL_BIT) == 0) {
                         treePos = val & LOW_MASK;
                         eosBits = false;
+                        // Found a zero, can't be counting EOS bits
                         eosBitCount = 0;
                     } else {
                         target.append((char) (val & LOW_MASK));
                         treePos = 0;
                         eosBits = true;
+                        // Output a character, reset eosBitCount
+                        eosBitCount = 0;
                     }
                 } else {
                     if (eosBits) {
@@ -409,6 +412,8 @@ public static void decode(ByteBuffer data, int length, StringBuilder target) thr
                         target.append((char) ((val >> 16) & LOW_MASK));
                         treePos = 0;
                         eosBits = true;
+                        // Output a character, reset eosBitCount
+                        eosBitCount = 0;
                     }
                 }
                 bitPos--;

test/org/apache/coyote/http2/TestHPackHuffman.java

@@ -0,0 +1,45 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.coyote.http2;
+
+import java.nio.ByteBuffer;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+
+public class TestHPackHuffman {
+
+    /*
+     * This specific String exposed an edge case that triggered an unexpected HPACK parsing failure.
+     */
+    @Test
+    public void testValueLeftBrace() throws Exception {
+        ByteBuffer buf = ByteBuffer.allocate(10);
+        String data = "x-value{";
+        HPackHuffman.encode(buf, data, false);
+
+        buf.flip();
+        // Remove the header byte (in Tomcat this is parsed before the bytes are passed to the HPACK decoder)
+        buf.get();
+
+        StringBuilder target = new StringBuilder();
+        HPackHuffman.decode(buf, buf.remaining(), target);
+
+        Assert.assertEquals("Value changed after encode/decode roundtrip", data, target.toString());
+    }
+}

webapps/docs/changelog.xml

@@ -241,6 +241,11 @@
       <update>
         Remove support for HTTP 0.9. (markt)
       </update>
+      <fix>
+        Correct an unlikely edge-case parsing bug in the HTTP/2 HPACK header
+        decoding that could result in a valid header triggering an unexpected
+        connection close. (markt)
+      </fix>
       <!-- Entries for backport and removal before 12.0.0-M1 below this line -->
       <fix>
         Avoid various edge cases if <code>Content-Length</code> is set via