From a7b0b727dd842eacfd927d881f0774b7cc83975a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 18 Apr 2023 15:24:59 +1000 Subject: [PATCH] Issue #9554 - add javadoc for huffman / n-bit integer classes and remove static decode methods Signed-off-by: Lachlan Roberts --- .../jetty/http/compression/Huffman.java | 3 ++ .../http/compression/HuffmanDecoder.java | 26 ++++++------ .../http/compression/HuffmanEncoder.java | 30 +++++++++++++ .../http/compression/NBitIntegerEncoder.java | 18 ++++++++ .../http/compression/NBitIntegerParser.java | 42 ++++++++++++------- .../http/compression/NBitStringParser.java | 24 +++++++++++ .../org/eclipse/jetty/http/HuffmanTest.java | 17 +++++++- .../jetty/http2/hpack/HpackDecoder.java | 15 ++++++- .../jetty/http2/hpack/HpackContextTest.java | 31 +++++++++++++- 9 files changed, 174 insertions(+), 32 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/Huffman.java b/jetty-http/src/main/java/org/eclipse/jetty/http/compression/Huffman.java index 6a1ad3d6789..bf530959175 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/Huffman.java +++ b/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() diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanDecoder.java index cd1ed97fc37..da656ba638d 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanDecoder.java +++ b/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; +/** + *

Used to decoded Huffman encoded strings.

+ * + *

Characters which are illegal field-vchar values are replaced with + * either ' ' or '?' as described in RFC9110

+ */ 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++) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanEncoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanEncoder.java index 13cb81b0492..0475578079b 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/HuffmanEncoder.java +++ b/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; +/** + *

Used to encode strings Huffman encoding.

+ * + *

Characters are encoded with ISO-8859-1, if any multi-byte characters or + * control characters are present the encoder will throw {@link EncodingException}.

+ */ 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); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerEncoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerEncoder.java index 944c824615b..0750d6584f8 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerEncoder.java +++ b/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) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerParser.java index 377cef19bc4..aa8a8f085dd 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitIntegerParser.java +++ b/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; diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitStringParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitStringParser.java index b3091317f10..fadc45d0988 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/compression/NBitStringParser.java +++ b/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; +/** + *

Used to decode string literals as described in RFC7541.

+ * + *

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.

+ * + *

Characters which are illegal field-vchar values are replaced with + * either ' ' or '?' as described in RFC9110

+ */ 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) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java index 5007877094e..42dd39981e9 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java +++ b/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 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()); } } diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java index 77e4d05c61b..09c23c5e477 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java +++ b/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 diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java index 25cd4656687..0b46b8eb723 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackContextTest.java +++ b/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); }