Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60451

Correctly handle HTTP/2 header values that contain characters with unicode code points in the range 128 to 255. Reject with a clear error message HTTP/2 header values that contain characters with unicode code points above 255.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1773306 13f79535-47bb-0310-9956-ffa450edef68

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

@@ -434,7 +434,11 @@ public static boolean encode(ByteBuffer buffer, String toEncode, boolean forceLo
         //so we end up iterating twice
         int length = 0;
         for (int i = 0; i < toEncode.length(); ++i) {
-            byte c = (byte) toEncode.charAt(i);
+            char c = toEncode.charAt(i);
+            if (c > 255) {
+                throw new IllegalArgumentException(sm.getString("hpack.invalidCharacter",
+                        Character.toString(c), Integer.valueOf(c)));
+            }
             if(forceLowercase) {
                 c = Hpack.toLower(c);
             }
@@ -450,7 +454,7 @@ public static boolean encode(ByteBuffer buffer, String toEncode, boolean forceLo
         int bytePos = 0;
         byte currentBufferByte = 0;
         for (int i = 0; i < toEncode.length(); ++i) {
-            byte c = (byte) toEncode.charAt(i);
+            char c = toEncode.charAt(i);
             if(forceLowercase) {
                 c = Hpack.toLower(c);
             }

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

@@ -204,11 +204,11 @@ static void encodeInteger(ByteBuffer source, int value, int n) {
     }
 
 
-    static byte toLower(byte b) {
-        if (b >= 'A' && b <= 'Z') {
-            return (byte) (b + LOWER_DIFF);
+    static char toLower(char c) {
+        if (c >= 'A' && c <= 'Z') {
+            return (char) (c + LOWER_DIFF);
         }
-        return b;
+        return c;
     }
 
     private Hpack() {}

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

@@ -214,7 +214,7 @@ private void writeHuffmanEncodableName(ByteBuffer target, String headerName) {
         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(Hpack.toLower((byte) headerName.charAt(j)));
+            target.put((byte) Hpack.toLower(headerName.charAt(j)));
         }
 
     }

java/org/apache/coyote/http2/LocalStrings.properties

@@ -32,6 +32,7 @@ frameType.checkPayloadSize=Payload size of [{0}] is not valid for frame type [{1
 frameType.checkStream=Invalid frame type [{0}]
 
 hpack.integerEncodedOverTooManyOctets=HPACK variable length integer encoded over too many octets, max is {0}
+hpack.invalidCharacter=The Unicode character [{0}] at code point [{1}] cannot be encoded as it is outside the permitted range of 0 to 255.
 
 hpackdecoder.zeroNotValidHeaderTableIndex=Zero is not a valid header table index
 

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

@@ -85,6 +85,39 @@ public void validateHeaders() throws StreamException {
         }
     }
 
-    // TODO: Write more complete tests
+    @Test
+    public void testHeaderValueBug60451() throws HpackException {
+        doTestHeaderValueBug60451("fooébar");
+    }
+
+    @Test
+    public void testHeaderValueFullRange() {
+        for (int i = 0; i < 256; i++) {
+            // Skip the control characters except VTAB
+            if (i == 9 || i > 31 && i < 127 || i > 127) {
+                try {
+                    doTestHeaderValueBug60451("foo" + Character.toString((char) i)  + "bar");
+                } catch (Exception e) {
+                    e.printStackTrace();
+                    Assert.fail(e.getMessage() + "[" + i + "]");
+                }
+            }
+        }
+    }
 
+    private void doTestHeaderValueBug60451(String filename) throws HpackException {
+        String headerName = "Content-Disposition";
+        String headerValue = "attachment;filename=\"" + filename + "\"";
+        MimeHeaders headers = new MimeHeaders();
+        headers.setValue(headerName).setString(headerValue);
+        ByteBuffer output = ByteBuffer.allocate(512);
+        HpackEncoder encoder = new HpackEncoder(1024);
+        encoder.encode(headers, output);
+        output.flip();
+        MimeHeaders headers2 = new MimeHeaders();
+        HpackDecoder decoder = new HpackDecoder();
+        decoder.setHeaderEmitter(new HeadersListener(headers2));
+        decoder.decode(output);
+        Assert.assertEquals(headerValue, headers2.getHeader(headerName));
+    }
 }

webapps/docs/changelog.xml

@@ -56,6 +56,12 @@
         Extract the common Acceptor code from each Endpoint into a new Acceptor
         class that is used by all Endpoints. (markt)
       </scode>
+      <fix>
+        <bug>60451</bug>: Correctly handle HTTP/2 header values that contain
+        characters with unicode code points in the range 128 to 255. Reject
+        with a clear error message HTTP/2 header values that contain characters
+        with unicode code points above 255. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>