Issue #2679 - HTTP/2 Spec Compliance

now handling padding over 7 bits with Exception
throwing CompressionExceptions instead of StreamExceptions
fixed issue with non terminal encoded strings

Signed-off-by: lachan-roberts <lachlan@webtide.com>

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

@@ -345,24 +345,25 @@ public class Huffman
         }
     }
 
-    public static String decode(ByteBuffer buffer) throws HpackException.StreamException
+    public static String decode(ByteBuffer buffer) throws HpackException.CompressionException
     {  
         return decode(buffer,buffer.remaining());
     }
 
-    public static String decode(ByteBuffer buffer,int length) throws HpackException.StreamException
+    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;
@@ -375,7 +376,7 @@ public class Huffman
                 if (rowbits[node]!=0) 
                 {
                     if(rowsym[node] == EOS)
-                        throw new HpackException.StreamException("EOS in content");
+                        throw new HpackException.CompressionException("EOS in content");
 
                     // terminal node
                     out.append(rowsym[node]);
@@ -393,6 +394,7 @@ 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)
@@ -402,20 +404,20 @@ public class Huffman
                     requiredPadding = (requiredPadding << 1) | 1;
 
                 if((c>>(8-bits)) != requiredPadding)
-                    throw new HpackException.StreamException("Incorrect padding");
+                    throw new HpackException.CompressionException("Incorrect padding");
 
+                node = lastNode;
                 break;
             }
 
-            // TODO why is this even here
-            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/HpackDecoderTest.java

@@ -19,12 +19,6 @@
 
 package org.eclipse.jetty.http2.hpack;
 
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-
 import java.nio.ByteBuffer;
 import java.util.Iterator;
 
@@ -39,6 +33,12 @@ import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
 public class HpackDecoderTest
 {
     @Test
@@ -495,9 +495,9 @@ public class HpackDecoderTest
             decoder.decode(buffer);
             Assert.fail();
         }
-        catch (StreamException ex)
+        catch (CompressionException ex)
         {
-            Assert.assertThat(ex.getMessage(), Matchers.containsString("Padding length exceeded"));
+            Assert.assertThat(ex.getMessage(), Matchers.containsString("Bad termination"));
         }
     }
 
@@ -516,7 +516,7 @@ public class HpackDecoderTest
             decoder.decode(buffer);
             Assert.fail();
         }
-        catch (StreamException ex)
+        catch (CompressionException ex)
         {
             Assert.assertThat(ex.getMessage(), Matchers.containsString("Incorrect padding"));
         }
@@ -537,9 +537,49 @@ public class HpackDecoderTest
             decoder.decode(buffer);
             Assert.fail();
         }
-        catch (StreamException ex)
+        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
     {