From b5e607068f435116bfe5b7ad6e703938471bec20 Mon Sep 17 00:00:00 2001 From: lachan-roberts Date: Sat, 21 Jul 2018 22:10:12 +1000 Subject: [PATCH] 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 --- .../eclipse/jetty/http2/hpack/Huffman.java | 22 +++---- .../jetty/http2/hpack/HpackDecoderTest.java | 60 +++++++++++++++---- .../jetty/http2/hpack/HuffmanTest.java | 11 ---- 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java index 8a1dbb3c5ed..ddb2c4a97df 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/Huffman.java +++ b/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 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(); } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java index 03982a01241..60bca31b5a2 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackDecoderTest.java +++ b/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")); + } + } } diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java index 61a6f42e184..3b697ac0fe5 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java +++ b/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 {