Merge pull request #2742 from lachlan-roberts/jetty-9.4.x-2679-h2spec_compliance

Issue #2679 - h2spec compliance Huffman encoding

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

@@ -287,6 +287,7 @@ public class Huffman
     };
 
     static final int[][] LCCODES = new int[CODES.length][];
+    static final char EOS = 256;
     
     // Huffman decode tree stored in a flattened char array for good 
     // locality of reference.
@@ -344,24 +345,25 @@ public class Huffman
         }
     }
 
-    public static String decode(ByteBuffer buffer)
+    public static String decode(ByteBuffer buffer) throws HpackException.CompressionException
     {  
         return decode(buffer,buffer.remaining());
     }
 
-    public static String decode(ByteBuffer buffer,int length)
+    public static String decode(ByteBuffer buffer,int length) throws HpackException.CompressionException
     {        
         StringBuilder out = new StringBuilder(length*2);
         int node = 0;
         int current = 0;
         int bits = 0;
-        
+
         byte[] array = buffer.array();
         int position=buffer.position();
         int start=buffer.arrayOffset()+position;
         int end=start+length;
         buffer.position(position+length);
-        
+
+
         for (int i=start; i<end; i++)
         {
             int b = array[i]&0xFF;
@@ -373,6 +375,9 @@ public class Huffman
                 node = tree[node*256+c];
                 if (rowbits[node]!=0) 
                 {
+                    if(rowsym[node] == EOS)
+                        throw new HpackException.CompressionException("EOS in content");
+
                     // terminal node
                     out.append(rowsym[node]);
                     bits -= rowbits[node];
@@ -389,18 +394,30 @@ public class Huffman
         while (bits > 0) 
         {
             int c = (current << (8 - bits)) & 0xFF;
+            int lastNode = node;
             node = tree[node*256+c];
-            if (rowbits[node]==0 || rowbits[node] > bits) 
+
+            if (rowbits[node]==0 || rowbits[node] > bits)
+            {
+                int requiredPadding = 0;
+                for(int i=0; i<bits; i++)
+                    requiredPadding = (requiredPadding << 1) | 1;
+
+                if((c>>(8-bits)) != requiredPadding)
+                    throw new HpackException.CompressionException("Incorrect padding");
+
+                node = lastNode;
                 break;
-            
+            }
-            if (rowbits[node]==0)
+
-                throw new IllegalStateException();
-            
             out.append(rowsym[node]);
             bits -= rowbits[node];
             node = 0;
         }
 
+        if(node != 0)
+            throw new HpackException.CompressionException("Bad termination");
+
         return out.toString();
     }
 

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

@@ -411,7 +411,7 @@ public class HpackContextTest
     }
     
     @Test
-    public void testStaticHuffmanValues()
+    public void testStaticHuffmanValues() throws Exception
     {
         HpackContext ctx = new HpackContext(4096);
         for (int i=2;i<=14;i++)

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

@@ -462,25 +462,124 @@ public class HpackDecoderTest
         }        
     }
 
-    
-    /*
-        8.1.2.6. Malformed Requests and Responses
-          × 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length
-          × 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length
 
-      5. Primitive Type Representations
+    @Test()
-        5.2. String Literal Representation
+    public void testHuffmanEncodedStandard() throws Exception
-          × 1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits
+    {
-            -> The endpoint MUST treat this as a decoding error.
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
-               Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR)
+
-                         Connection closed
+        String encoded = "82868441" + "83" + "49509F";
-                 Actual: DATA Frame (length:687, flags:0x01, stream_id:1)
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
-          × 2: Sends a Huffman-encoded string literal representation padded by zero
+
-            -> The endpoint MUST treat this as a decoding error.
+        MetaData.Request request = (MetaData.Request)decoder.decode(buffer);
-               Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR)
+
-                         Connection closed
+        assertEquals("GET", request.getMethod());
-                 Actual: DATA Frame (length:687, flags:0x01, stream_id:1)
+        assertEquals(HttpScheme.HTTP.asString(), request.getURI().getScheme());
-          ✔ 3: Sends a Huffman-encoded string literal representation containing the EOS symbol
+        assertEquals("/", request.getURI().getPath());
-    */
+        assertEquals("test", request.getURI().getHost());
-    
+        assertFalse(request.iterator().hasNext());
+    }
+
+
+    /* 5.2.1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits */
+    @Test()
+    public void testHuffmanEncodedExtraPadding() throws Exception
+    {
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
+
+        String encoded = "82868441" + "84" + "49509FFF";
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
+
+        try
+        {
+            decoder.decode(buffer);
+            Assert.fail();
+        }
+        catch (CompressionException ex)
+        {
+            Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination"));
+        }
+    }
+
+
+    /* 5.2.2: Sends a Huffman-encoded string literal representation padded by zero */
+    @Test()
+    public void testHuffmanEncodedZeroPadding() throws Exception
+    {
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
+
+        String encoded = "82868441" + "83" + "495090";
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
+
+        try
+        {
+            decoder.decode(buffer);
+            Assert.fail();
+        }
+        catch (CompressionException ex)
+        {
+            Assert.assertThat(ex.getMessage(), Matchers.containsString("Incorrect padding"));
+        }
+    }
+
+
+    /* 5.2.3: Sends a Huffman-encoded string literal representation containing the EOS symbol */
+    @Test()
+    public void testHuffmanEncodedWithEOS() throws Exception
+    {
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
+
+        String encoded = "82868441" + "87" + "497FFFFFFF427F";
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
+
+        try
+        {
+            decoder.decode(buffer);
+            Assert.fail();
+        }
+        catch (CompressionException ex)
+        {
+            Assert.assertThat(ex.getMessage(), Matchers.containsString("EOS in content"));
+        }
+    }
+
+
+    @Test()
+    public void testHuffmanEncodedOneIncompleteOctet() throws Exception
+    {
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
+
+        String encoded = "82868441" + "81" + "FE";
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
+
+        try
+        {
+            decoder.decode(buffer);
+            Assert.fail();
+        }
+        catch (CompressionException ex)
+        {
+            Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination"));
+        }
+    }
+
+
+    @Test()
+    public void testHuffmanEncodedTwoIncompleteOctet() throws Exception
+    {
+        HpackDecoder decoder = new HpackDecoder(4096, 8192);
+
+        String encoded = "82868441" + "82" + "FFFE";
+        ByteBuffer buffer = ByteBuffer.wrap(TypeUtil.fromHexString(encoded));
+
+        try
+        {
+            decoder.decode(buffer);
+            Assert.fail();
+        }
+        catch (CompressionException ex)
+        {
+            Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination"));
+        }
+    }
 }

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

@@ -50,17 +50,6 @@ public class HuffmanTest
         }
     }
 
-    @Test
-    public void testDecodeTrailingFF() throws Exception
-    {
-        for (String[] test:tests)
-        {
-            byte[] encoded=TypeUtil.fromHexString(test[1]+"FF");
-            String decoded=Huffman.decode(ByteBuffer.wrap(encoded));
-            Assert.assertEquals(test[0],test[2],decoded);
-        }
-    }
-
     @Test
     public void testEncode() throws Exception
     {