Merge pull request #4079 from eclipse/jetty-9.4.x-2815-hpackOpaqueBytes

Issue #2815 - HPack Opaque Bytes

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java

@@ -89,7 +89,7 @@ public class ServletPathSpec extends PathSpec
         {
             this.group = PathSpecGroup.EXACT;
             this.prefix = servletPathSpec;
-            if (servletPathSpec.endsWith("*") )
+            if (servletPathSpec.endsWith("*"))
             {
                 LOG.warn("Suspicious URL pattern: '{}'; see sections 12.1 and 12.2 of the Servlet specification",
                         servletPathSpec);

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java

@@ -461,6 +461,8 @@ public class HpackContext
             if (value != null && value.length() > 0)
             {
                 int huffmanLen = Huffman.octetsNeeded(value);
+                if (huffmanLen < 0)
+                    throw new IllegalStateException("bad value");
                 int lenLen = NBitInteger.octectsNeeded(7, huffmanLen);
                 _huffmanValue = new byte[1 + lenLen + huffmanLen];
                 ByteBuffer buffer = ByteBuffer.wrap(_huffmanValue);

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java

@@ -177,7 +177,7 @@ public class HpackDecoder
                     else
                         name = toASCIIString(buffer, length);
                     check:
-                    for (int i = name.length(); i-- > 0;)
+                    for (int i = name.length(); i-- > 0; )
                     {
                         char c = name.charAt(i);
                         if (c > 0xff)

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.http2.hpack;
 
 import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
 import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.Set;
@@ -392,19 +393,38 @@ public class HpackEncoder
         {
             // huffman literal value
             buffer.put((byte)0x80);
-            NBitInteger.encode(buffer, 7, Huffman.octetsNeeded(value));
-            Huffman.encode(buffer, value);
+
+            int needed = Huffman.octetsNeeded(value);
+            if (needed >= 0)
+            {
+                NBitInteger.encode(buffer, 7, needed);
+                Huffman.encode(buffer, value);
+            }
+            else
+            {
+                // Not iso_8859_1
+                byte[] bytes = value.getBytes(StandardCharsets.UTF_8);
+                NBitInteger.encode(buffer, 7, Huffman.octetsNeeded(bytes));
+                Huffman.encode(buffer, bytes);
+            }
         }
         else
         {
             // add literal assuming iso_8859_1
-            buffer.put((byte)0x00);
+            buffer.put((byte)0x00).mark();
             NBitInteger.encode(buffer, 7, value.length());
             for (int i = 0; i < value.length(); i++)
             {
                 char c = value.charAt(i);
                 if (c < ' ' || c > 127)
-                    throw new IllegalArgumentException();
+                {
+                    // Not iso_8859_1, so re-encode as UTF-8
+                    buffer.reset();
+                    byte[] bytes = value.getBytes(StandardCharsets.UTF_8);
+                    NBitInteger.encode(buffer, 7, bytes.length);
+                    buffer.put(bytes, 0, bytes.length);
+                    return;
+                }
                 buffer.put((byte)c);
             }
         }

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java

@@ -20,6 +20,8 @@ package org.eclipse.jetty.http2.hpack;
 
 import java.nio.ByteBuffer;
 
+import org.eclipse.jetty.util.Utf8StringBuilder;
+
 public class Huffman
 {
 
@@ -358,7 +360,7 @@ public class Huffman
 
     public static String decode(ByteBuffer buffer, int length) throws HpackException.CompressionException
     {
-        StringBuilder out = new StringBuilder(length * 2);
+        Utf8StringBuilder utf8 = new Utf8StringBuilder(length * 2);
         int node = 0;
         int current = 0;
         int bits = 0;
@@ -378,7 +380,7 @@ public class Huffman
                         throw new HpackException.CompressionException("EOS in content");
 
                     // terminal node
-                    out.append(rowsym[node]);
+                    utf8.append((byte)(0xFF & rowsym[node]));
                     bits -= rowbits[node];
                     node = 0;
                 }
@@ -411,7 +413,7 @@ public class Huffman
                 break;
             }
 
-            out.append(rowsym[node]);
+            utf8.append((byte)(0xFF & rowsym[node]));
             bits -= rowbits[node];
             node = 0;
         }
@@ -419,7 +421,7 @@ public class Huffman
         if (node != 0)
             throw new HpackException.CompressionException("Bad termination");
 
-        return out.toString();
+        return utf8.toString();
     }
 
     public static int octetsNeeded(String s)
@@ -427,11 +429,21 @@ public class Huffman
         return octetsNeeded(CODES, s);
     }
 
+    public static int octetsNeeded(byte[] b)
+    {
+        return octetsNeeded(CODES, b);
+    }
+
     public static void encode(ByteBuffer buffer, String s)
     {
         encode(CODES, buffer, s);
     }
 
+    public static void encode(ByteBuffer buffer, byte[] b)
+    {
+        encode(CODES, buffer, b);
+    }
+
     public static int octetsNeededLC(String s)
     {
         return octetsNeeded(LCCODES, s);
@@ -450,13 +462,30 @@ public class Huffman
         {
             char c = s.charAt(i);
             if (c >= 128 || c < ' ')
-                throw new IllegalArgumentException();
+                return -1;
             needed += table[c][1];
         }
 
         return (needed + 7) / 8;
     }
 
+    private static int octetsNeeded(final int[][] table, byte[] b)
+    {
+        int needed = 0;
+        int len = b.length;
+        for (int i = 0; i < len; i++)
+        {
+            int c = 0xFF & b[i];
+            needed += table[c][1];
+        }
+        return (needed + 7) / 8;
+    }
+
+    /**
+     * @param table The table to encode by
+     * @param buffer The buffer to encode to
+     * @param s The string to encode
+     */
     private static void encode(final int[][] table, ByteBuffer buffer, String s)
     {
         long current = 0;
@@ -488,4 +517,35 @@ public class Huffman
             buffer.put((byte)(current));
         }
     }
+
+    private static void encode(final int[][] table, ByteBuffer buffer, byte[] b)
+    {
+        long current = 0;
+        int n = 0;
+
+        int len = b.length;
+        for (int i = 0; i < len; i++)
+        {
+            int c = 0xFF & b[i];
+            int code = table[c][0];
+            int bits = table[c][1];
+
+            current <<= bits;
+            current |= code;
+            n += bits;
+
+            while (n >= 8)
+            {
+                n -= 8;
+                buffer.put((byte)(current >> n));
+            }
+        }
+
+        if (n > 0)
+        {
+            current <<= (8 - n);
+            current |= (0xFF >>> n);
+            buffer.put((byte)(current));
+        }
+    }
 }

jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java

@@ -136,6 +136,26 @@ public class HpackTest
         }
     }
 
+    @Test
+    public void encodeDecodeNonAscii() throws Exception
+    {
+        HpackEncoder encoder = new HpackEncoder();
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
+        ByteBuffer buffer = BufferUtil.allocate(16 * 1024);
+
+        HttpFields fields0 = new HttpFields();
+        fields0.add("Cookie", "[\uD842\uDF9F]");
+        fields0.add("custom-key", "[\uD842\uDF9F]");
+        Response original0 = new MetaData.Response(HttpVersion.HTTP_2, 200, fields0);
+
+        BufferUtil.clearToFill(buffer);
+        encoder.encode(buffer, original0);
+        BufferUtil.flipToFlush(buffer, 0);
+        Response decoded0 = (Response)decoder.decode(buffer);
+
+        assertMetadataSame(original0, decoded0);
+    }
+
     @Test
     public void evictReferencedFieldTest() throws Exception
     {

jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java

@@ -25,11 +25,13 @@ import java.util.stream.Stream;
 
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.TypeUtil;
+import org.hamcrest.Matchers;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -77,8 +79,7 @@ public class HuffmanTest
     {
         String s = "bad '" + bad + "'";
 
-        assertThrows(IllegalArgumentException.class,
-            () -> Huffman.octetsNeeded(s));
+        assertThat(Huffman.octetsNeeded(s), Matchers.is(-1));
 
         assertThrows(BufferOverflowException.class,
             () -> Huffman.encode(BufferUtil.allocate(32), s));

jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java

@@ -81,24 +81,20 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem
     public SessionData load(String id) throws Exception
     {
         if (!isStarted())
-            throw new IllegalStateException ("Not started");
+            throw new IllegalStateException("Not started");
 
         final AtomicReference<SessionData> reference = new AtomicReference<SessionData>();
         final AtomicReference<Exception> exception = new AtomicReference<Exception>();
 
-        Runnable r = new Runnable()
+        Runnable r = () ->
         {
-            @Override
-            public void run()
+            try
             {
-                try
-                {
-                    reference.set(doLoad(id));
-                }
-                catch (Exception e)
-                {
-                    exception.set(e);
-                }
+                reference.set(doLoad(id));
+            }
+            catch (Exception e)
+            {
+                exception.set(e);
             }
         };
 
@@ -165,7 +161,7 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem
     public Set<String> getExpired(Set<String> candidates)
     {
         if (!isStarted())
-            throw new IllegalStateException ("Not started");
+            throw new IllegalStateException("Not started");
         
         try
         {