Issue #9554 - add javadoc for huffman / n-bit integer classes and remove static decode methods

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http/src/main/java/org/eclipse/jetty/http/compression/Huffman.java

@@ -13,6 +13,9 @@
 
 package org.eclipse.jetty.http.compression;
 
+/**
+ * This class contains the Huffman Codes defined in RFC7541.
+ */
 public class Huffman
 {
     private Huffman()

jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanDecoder.java

@@ -21,20 +21,14 @@ import org.eclipse.jetty.util.CharsetStringBuilder;
 import static org.eclipse.jetty.http.compression.Huffman.rowbits;
 import static org.eclipse.jetty.http.compression.Huffman.rowsym;
 
+/**
+ * <p>Used to decoded Huffman encoded strings.</p>
+ *
+ * <p>Characters which are illegal field-vchar values are replaced with
+ * either ' ' or '?' as described in RFC9110</p>
+ */
 public class HuffmanDecoder
 {
-    public static String decode(ByteBuffer buffer, int length) throws EncodingException
-    {
-        HuffmanDecoder huffmanDecoder = new HuffmanDecoder();
-        huffmanDecoder.setLength(length);
-        String decoded = huffmanDecoder.decode(buffer);
-        if (decoded == null)
-            throw new EncodingException("invalid string encoding");
-
-        huffmanDecoder.reset();
-        return decoded;
-    }
-
     private final CharsetStringBuilder.Iso8859StringBuilder _builder = new CharsetStringBuilder.Iso8859StringBuilder();
     private int _length = 0;
     private int _count = 0;
@@ -42,6 +36,9 @@ public class HuffmanDecoder
     private int _current = 0;
     private int _bits = 0;
 
+    /**
+     * @param length in bytes of the huffman data.
+     */
     public void setLength(int length)
     {
         if (_count != 0)
@@ -49,6 +46,11 @@ public class HuffmanDecoder
         _length = length;
     }
 
+    /**
+     * @param buffer the buffer containing the Huffman encoded bytes.
+     * @return the decoded String.
+     * @throws EncodingException if the huffman encoding is invalid.
+     */
     public String decode(ByteBuffer buffer) throws EncodingException
     {
         for (; _count < _length; _count++)

jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanEncoder.java

@@ -20,17 +20,31 @@ import org.eclipse.jetty.http.HttpTokens;
 import static org.eclipse.jetty.http.compression.Huffman.CODES;
 import static org.eclipse.jetty.http.compression.Huffman.LCCODES;
 
+/**
+ * <p>Used to encode strings Huffman encoding.</p>
+ *
+ * <p>Characters are encoded with ISO-8859-1, if any multi-byte characters or
+ * control characters are present the encoder will throw {@link EncodingException}.</p>
+ */
 public class HuffmanEncoder
 {
     private HuffmanEncoder()
     {
     }
 
+    /**
+     * @param s the string to encode.
+     * @return the number of octets needed to encode the string, or -1 if it cannot be encoded.
+     */
     public static int octetsNeeded(String s)
     {
         return octetsNeeded(CODES, s);
     }
 
+    /**
+     * @param b the byte array to encode.
+     * @return the number of octets needed to encode the bytes, or -1 if it cannot be encoded.
+     */
     public static int octetsNeeded(byte[] b)
     {
         int needed = 0;
@@ -42,21 +56,37 @@ public class HuffmanEncoder
         return (needed + 7) / 8;
     }
 
+    /**
+     * @param buffer the buffer to encode into.
+     * @param s the string to encode.
+     */
     public static void encode(ByteBuffer buffer, String s)
     {
         encode(CODES, buffer, s);
     }
 
+    /**
+     * @param buffer the buffer to encode into.
+     * @param b the byte array to encode.
+     */
     public static void encode(ByteBuffer buffer, byte[] b)
     {
         encode(CODES, buffer, b);
     }
 
+    /**
+     * @param s the string to encode in lowercase.
+     * @return the number of octets needed to encode the string, or -1 if it cannot be encoded.
+     */
     public static int octetsNeededLowercase(String s)
     {
         return octetsNeeded(LCCODES, s);
     }
 
+    /**
+     * @param buffer the buffer to encode into in lowercase.
+     * @param s the string to encode.
+     */
     public static void encodeLowercase(ByteBuffer buffer, String s)
     {
         encode(LCCODES, buffer, s);

jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerEncoder.java

@@ -15,8 +15,20 @@ package org.eclipse.jetty.http.compression;
 
 import java.nio.ByteBuffer;
 
+/**
+ * Used to encode integers as described in RFC7541.
+ */
 public class NBitIntegerEncoder
 {
+    private NBitIntegerEncoder()
+    {
+    }
+
+    /**
+     * @param n the prefix used to encode this long.
+     * @param i the integer to encode.
+     * @return the number of octets it would take to encode the long.
+     */
     public static int octetsNeeded(int n, long i)
     {
         if (n == 8)
@@ -43,6 +55,12 @@ public class NBitIntegerEncoder
         return (log + 6) / 7;
     }
 
+    /**
+     *
+     * @param buf the buffer to encode into.
+     * @param n the prefix used to encode this long.
+     * @param i the long to encode into the buffer.
+     */
     public static void encode(ByteBuffer buf, int n, long i)
     {
         if (n == 8)

jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerParser.java

@@ -15,28 +15,21 @@ package org.eclipse.jetty.http.compression;
 
 import java.nio.ByteBuffer;
 
+/**
+ * Used to decode integers as described in RFC7541.
+ */
 public class NBitIntegerParser
 {
-    public static int decode(ByteBuffer buffer, int prefix) throws EncodingException
-    {
-        // TODO: This is a fix for HPACK as it already takes the first byte of the encoded integer.
-        if (prefix != 8)
-            buffer.position(buffer.position() - 1);
-
-        NBitIntegerParser parser = new NBitIntegerParser();
-        parser.setPrefix(prefix);
-        int decodedInt = parser.decodeInt(buffer);
-        if (decodedInt < 0)
-            throw new EncodingException("invalid integer encoding");
-        parser.reset();
-        return decodedInt;
-    }
-
     private int _prefix;
     private long _total;
     private long _multiplier;
     private boolean _started;
 
+    /**
+     * Set the prefix length in of the integer representation in bits.
+     * A prefix of 6 means the integer representation starts after the first 2 bits.
+     * @param prefix the number of bits in the integer prefix.
+     */
     public void setPrefix(int prefix)
     {
         if (_started)
@@ -44,11 +37,27 @@ public class NBitIntegerParser
         _prefix = prefix;
     }
 
+    /**
+     * Decode an integer from the buffer. If the buffer does not contain the complete integer representation
+     * a value of -1 is returned to indicate that more data is needed to complete parsing.
+     * This should be only after the prefix has been set with {@link #setPrefix(int)}.
+     * @param buffer the buffer containing the encoded integer.
+     * @return the decoded integer or -1 to indicate that more data is needed.
+     * @throws ArithmeticException if the value overflows a int.
+     */
     public int decodeInt(ByteBuffer buffer)
     {
         return Math.toIntExact(decodeLong(buffer));
     }
 
+    /**
+     * Decode a long from the buffer. If the buffer does not contain the complete integer representation
+     * a value of -1 is returned to indicate that more data is needed to complete parsing.
+     * This should be only after the prefix has been set with {@link #setPrefix(int)}.
+     * @param buffer the buffer containing the encoded integer.
+     * @return the decoded long or -1 to indicate that more data is needed.
+     * @throws ArithmeticException if the value overflows a long.
+     */
     public long decodeLong(ByteBuffer buffer)
     {
         if (!_started)
@@ -86,6 +95,9 @@ public class NBitIntegerParser
         }
     }
 
+    /**
+     * Reset the internal state of the parser.
+     */
     public void reset()
     {
         _prefix = 0;

jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitStringParser.java

@@ -17,6 +17,16 @@ import java.nio.ByteBuffer;
 
 import org.eclipse.jetty.util.CharsetStringBuilder;
 
+/**
+ * <p>Used to decode string literals as described in RFC7541.</p>
+ *
+ * <p>The string literal representation consists of a single bit to indicate whether huffman encoding is used,
+ * followed by the string byte length encoded with the n-bit integer representation also from RFC7541, and
+ * the bytes of the string are directly after this.</p>
+ *
+ * <p>Characters which are illegal field-vchar values are replaced with
+ * either ' ' or '?' as described in RFC9110</p>
+ */
 public class NBitStringParser
 {
     private final NBitIntegerParser _integerParser;
@@ -43,6 +53,11 @@ public class NBitStringParser
         _builder = new CharsetStringBuilder.Iso8859StringBuilder();
     }
 
+    /**
+     * Set the prefix length in of the string representation in bits.
+     * A prefix of 6 means the string representation starts after the first 2 bits.
+     * @param prefix the number of bits in the string prefix.
+     */
     public void setPrefix(int prefix)
     {
         if (_state != State.PARSING)
@@ -50,6 +65,15 @@ public class NBitStringParser
         _prefix = prefix;
     }
 
+    /**
+     * Decode a string from the buffer. If the buffer does not contain the complete string representation
+     * then a value of null is returned to indicate that more data is needed to complete parsing.
+     * This should be only after the prefix has been set with {@link #setPrefix(int)}.
+     * @param buffer the buffer containing the encoded string.
+     * @return the decoded string or null to indicate that more data is needed.
+     * @throws ArithmeticException if the string length value overflows a int.
+     * @throws EncodingException if the string encoding is invalid.
+     */
     public String decode(ByteBuffer buffer) throws EncodingException
     {
         while (true)

jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java

@@ -17,6 +17,7 @@ import java.nio.ByteBuffer;
 import java.util.Locale;
 import java.util.stream.Stream;
 
+import org.eclipse.jetty.http.compression.EncodingException;
 import org.eclipse.jetty.http.compression.HuffmanDecoder;
 import org.eclipse.jetty.http.compression.HuffmanEncoder;
 import org.eclipse.jetty.util.BufferUtil;
@@ -34,6 +35,18 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class HuffmanTest
 {
+    public static String decode(ByteBuffer buffer, int length) throws EncodingException
+    {
+        HuffmanDecoder huffmanDecoder = new HuffmanDecoder();
+        huffmanDecoder.setLength(length);
+        String decoded = huffmanDecoder.decode(buffer);
+        if (decoded == null)
+            throw new EncodingException("invalid string encoding");
+
+        huffmanDecoder.reset();
+        return decoded;
+    }
+
     public static Stream<Arguments> data()
     {
         return Stream.of(
@@ -94,7 +107,7 @@ public class HuffmanTest
     public void testDecode8859Only(String hexString, char expected) throws Exception
     {
         ByteBuffer buffer = ByteBuffer.wrap(StringUtil.fromHexString(hexString));
-        String decoded = HuffmanDecoder.decode(buffer, buffer.remaining());
+        String decoded = decode(buffer, buffer.remaining());
         assertThat(decoded, equalTo("" + expected));
     }
 
@@ -146,6 +159,6 @@ public class HuffmanTest
 
     private String decode(ByteBuffer buffer) throws Exception
     {
-        return HuffmanDecoder.decode(buffer, buffer.remaining());
+        return decode(buffer, buffer.remaining());
     }
 }

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

@@ -41,6 +41,7 @@ public class HpackDecoder
     private final HpackContext _context;
     private final MetaDataBuilder _builder;
     private final HuffmanDecoder _huffmanDecoder;
+    private final NBitIntegerParser _integerParser;
     private int _localMaxDynamicTableSize;
 
     /**
@@ -53,6 +54,7 @@ public class HpackDecoder
         _localMaxDynamicTableSize = localMaxDynamicTableSize;
         _builder = new MetaDataBuilder(maxHeaderSize);
         _huffmanDecoder = new HuffmanDecoder();
+        _integerParser = new NBitIntegerParser();
     }
 
     public HpackContext getHpackContext()
@@ -277,7 +279,14 @@ public class HpackDecoder
     {
         try
         {
-            return NBitIntegerParser.decode(buffer, prefix);
+            if (prefix != 8)
+                buffer.position(buffer.position() - 1);
+
+            _integerParser.setPrefix(prefix);
+            int decodedInt = _integerParser.decodeInt(buffer);
+            if (decodedInt < 0)
+                throw new EncodingException("invalid integer encoding");
+            return decodedInt;
         }
         catch (EncodingException e)
         {
@@ -285,6 +294,10 @@ public class HpackDecoder
             compressionException.initCause(e);
             throw compressionException;
         }
+        finally
+        {
+            _integerParser.reset();
+        }
     }
 
     private String huffmanDecode(ByteBuffer buffer, int length) throws HpackException.CompressionException

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

@@ -16,6 +16,7 @@ package org.eclipse.jetty.http2.hpack;
 import java.nio.ByteBuffer;
 
 import org.eclipse.jetty.http.HttpField;
+import org.eclipse.jetty.http.compression.EncodingException;
 import org.eclipse.jetty.http.compression.HuffmanDecoder;
 import org.eclipse.jetty.http.compression.NBitIntegerParser;
 import org.eclipse.jetty.http2.hpack.HpackContext.Entry;
@@ -34,6 +35,32 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class HpackContextTest
 {
+    public static String decode(ByteBuffer buffer, int length) throws EncodingException
+    {
+        HuffmanDecoder huffmanDecoder = new HuffmanDecoder();
+        huffmanDecoder.setLength(length);
+        String decoded = huffmanDecoder.decode(buffer);
+        if (decoded == null)
+            throw new EncodingException("invalid string encoding");
+
+        huffmanDecoder.reset();
+        return decoded;
+    }
+
+    public static int decodeInt(ByteBuffer buffer, int prefix) throws EncodingException
+    {
+        // This is a fix for HPACK as it already takes the first byte of the encoded integer.
+        if (prefix != 8)
+            buffer.position(buffer.position() - 1);
+
+        NBitIntegerParser parser = new NBitIntegerParser();
+        parser.setPrefix(prefix);
+        int decodedInt = parser.decodeInt(buffer);
+        if (decodedInt < 0)
+            throw new EncodingException("invalid integer encoding");
+        parser.reset();
+        return decodedInt;
+    }
 
     @Test
     public void testStaticName()
@@ -425,10 +452,10 @@ public class HpackContextTest
             int huff = 0xff & buffer.get();
             assertTrue((0x80 & huff) == 0x80);
 
-            int len = NBitIntegerParser.decode(buffer, 7);
+            int len = decodeInt(buffer, 7);
 
             assertEquals(len, buffer.remaining());
-            String value = HuffmanDecoder.decode(buffer, buffer.remaining());
+            String value = decode(buffer, buffer.remaining());
 
             assertEquals(entry.getHttpField().getValue(), value);
         }