Optimise conversion of HTTP/2 header field names to lower case

Author: markt-asf

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

@@ -19,6 +19,7 @@
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.Locale;
 import java.util.Set;
 
 import org.apache.tomcat.util.res.StringManager;
@@ -437,8 +438,25 @@ public static void decode(ByteBuffer data, int length, StringBuilder target) thr
      * @param forceLowercase If the string should be encoded in lower case
      *
      * @return true if encoding succeeded
+     *
+     * @deprecated Unused. This method will be removed in Tomcat 12 onwards.
      */
+    @Deprecated
     public static boolean encode(ByteBuffer buffer, String toEncode, boolean forceLowercase) {
+        return encode(buffer, toEncode.toLowerCase(Locale.ENGLISH));
+    }
+
+
+    /**
+     * Encodes the given string into the buffer. If there is not enough space in the buffer, or the encoded version is
+     * bigger than the original it will return false and not modify the buffers position.
+     *
+     * @param buffer         The buffer to encode into
+     * @param toEncode       The string to encode
+     *
+     * @return true if encoding succeeded
+     */
+    public static boolean encode(ByteBuffer buffer, String toEncode) {
         if (buffer.remaining() <= toEncode.length()) {
             return false;
         }
@@ -453,9 +471,6 @@ public static boolean encode(ByteBuffer buffer, String toEncode, boolean forceLo
                 throw new IllegalArgumentException(
                         sm.getString("hpack.invalidCharacter", Character.toString(c), Integer.valueOf(c)));
             }
-            if (forceLowercase) {
-                c = Hpack.toLower(c);
-            }
             HuffmanCode code = HUFFMAN_CODES[c];
             length += code.length;
         }
@@ -469,9 +484,6 @@ public static boolean encode(ByteBuffer buffer, String toEncode, boolean forceLo
         byte currentBufferByte = 0;
         for (int i = 0; i < toEncode.length(); ++i) {
             char c = toEncode.charAt(i);
-            if (forceLowercase) {
-                c = Hpack.toLower(c);
-            }
             HuffmanCode code = HUFFMAN_CODES[c];
             if (code.length + bytePos <= 8) {
                 // it fits in the current byte

java/org/apache/coyote/http2/Hpack.java

@@ -217,15 +217,18 @@ static void encodeInteger(ByteBuffer source, int value, int n) {
         }
     }
 
-
+    /*
+     * Unused. Will be removed in Tomcat 12 onwards.
+     */
+    @Deprecated
     static char toLower(char c) {
         if (c >= 'A' && c <= 'Z') {
             return (char) (c + LOWER_DIFF);
         }
         return c;
     }
 
+
     private Hpack() {
     }
-
 }

java/org/apache/coyote/http2/HpackEncoder.java

@@ -133,8 +133,11 @@ State encode(MimeHeaders headers, ByteBuffer target) {
             }
         }
         while (it < currentHeaders.size()) {
-            // FIXME: Review lowercase policy
-            String headerName = headers.getName(it).toString().toLowerCase(Locale.US);
+            /*
+             * Need to ensure header names are lower case from this point onwards as table lookups etc. are
+             * case-sensitive.
+             */
+            String headerName = headers.getName(it).toString().toLowerCase(Locale.ENGLISH);
             boolean skip = false;
             if (firstPass) {
                 if (headerName.charAt(0) != ':') {
@@ -208,23 +211,29 @@ State encode(MimeHeaders headers, ByteBuffer target) {
         return State.COMPLETE;
     }
 
+    /*
+     * headerName must be lower case by the time this method is called.
+     *
+     * The exception to the above rule is test cases which may deliberately use some upper case characters to test how
+     * Tomcat responds to such invalid input.
+     */
     private void writeHuffmanEncodableName(ByteBuffer target, String headerName) {
         if (hpackHeaderFunction.shouldUseHuffman(headerName)) {
-            if (HPackHuffman.encode(target, headerName, true)) {
+            if (HPackHuffman.encode(target, headerName)) {
                 return;
             }
         }
         target.put((byte) 0); // to use encodeInteger we need to place the first byte in the buffer.
         Hpack.encodeInteger(target, headerName.length(), 7);
         for (int j = 0; j < headerName.length(); ++j) {
-            target.put((byte) Hpack.toLower(headerName.charAt(j)));
+            target.put((byte) headerName.charAt(j));
         }
 
     }
 
     private void writeHuffmanEncodableValue(ByteBuffer target, String headerName, String val) {
         if (hpackHeaderFunction.shouldUseHuffman(headerName, val)) {
-            if (!HPackHuffman.encode(target, val, false)) {
+            if (!HPackHuffman.encode(target, val)) {
                 writeValueString(target, val);
             }
         } else {

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

@@ -31,7 +31,7 @@ public class TestHPackHuffman {
     public void testValueLeftBrace() throws Exception {
         ByteBuffer buf = ByteBuffer.allocate(10);
         String data = "x-value{";
-        HPackHuffman.encode(buf, data, false);
+        HPackHuffman.encode(buf, data);
 
         buf.flip();
         // Remove the header byte (in Tomcat this is parsed before the bytes are passed to the HPACK decoder)

webapps/docs/changelog.xml

@@ -282,6 +282,10 @@
         decoding that could result in a valid header triggering an unexpected
         connection close. (markt)
       </fix>
+      <fix>
+        Refactor HTTP/2 HPACK encoding so field names are only converted to
+        lower case once during the encoding process. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">