Use ISO-8859-1 for encoding/decoding in huffman/hpack/qpack

Signed-off-by: Lachlan Roberts <[email protected]>

jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java

@@ -15,6 +15,7 @@
 
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.Objects;
 import java.util.function.Supplier;
 
 public class MetaData implements Iterable<HttpField>
@@ -100,6 +101,28 @@ public Iterator<HttpField> iterator()
         return _fields.iterator();
     }
 
+    @Override
+    public int hashCode()
+    {
+        return Objects.hash(_httpVersion, _contentLength, _fields, _trailerSupplier);
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+        if (!(obj instanceof MetaData))
+            return false;
+
+        MetaData other = (MetaData)obj;
+        if (!Objects.equals(_httpVersion, other._httpVersion))
+            return false;
+        if (!Objects.equals(_contentLength, other._contentLength))
+            return false;
+        if (!Objects.equals(_fields, other._fields))
+            return false;
+        return _trailerSupplier == null && other._trailerSupplier == null;
+    }
+
     @Override
     public String toString()
     {

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

@@ -346,4 +346,47 @@ private Huffman()
             }
         }
     }
+
+    public static boolean isIllegalCharacter(char c)
+    {
+        return (c >= 256 || c < ' ');
+    }
+
+    public static char getReplacementCharacter(char c)
+    {
+        switch (c)
+        {
+            // A recipient of CR, LF, or NUL within a field value MUST either reject the message
+            // or replace each of those characters with SP before further processing
+            case '\r':
+            case '\n':
+            case 0x00:
+                return ' ';
+
+            default:
+                if (c >= 256 || c < ' ')
+                    return '?';
+        }
+
+        return c;
+    }
+
+    public static int getReplacementCharacter(int i)
+    {
+        switch (i)
+        {
+            // A recipient of CR, LF, or NUL within a field value MUST either reject the message
+            // or replace each of those characters with SP before further processing
+            case '\r':
+            case '\n':
+            case 0x00:
+                return ' ';
+
+            default:
+                if (i >= 256 || i < ' ')
+                    return '?';
+        }
+
+        return i;
+    }
 }

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

@@ -15,7 +15,7 @@
 
 import java.nio.ByteBuffer;
 
-import org.eclipse.jetty.util.Utf8StringBuilder;
+import org.eclipse.jetty.util.CharsetStringBuilder;
 
 import static org.eclipse.jetty.http.compression.Huffman.rowbits;
 import static org.eclipse.jetty.http.compression.Huffman.rowsym;
@@ -34,7 +34,7 @@ public static String decode(ByteBuffer buffer, int length) throws EncodingExcept
         return decoded;
     }
 
-    private final Utf8StringBuilder _utf8 = new Utf8StringBuilder();
+    private final CharsetStringBuilder.Iso8859StringBuilder _builder = new CharsetStringBuilder.Iso8859StringBuilder();
     private int _length = 0;
     private int _count = 0;
     private int _node = 0;
@@ -71,7 +71,9 @@ public String decode(ByteBuffer buffer) throws EncodingException
                     }
 
                     // terminal node
-                    _utf8.append((byte)(0xFF & rowsym[_node]));
+                    int i = 0xFF & rowsym[_node];
+                    i = Huffman.getReplacementCharacter(i);
+                    _builder.append((byte)i);
                     _bits -= rowbits[_node];
                     _node = 0;
                 }
@@ -104,7 +106,9 @@ public String decode(ByteBuffer buffer) throws EncodingException
                 break;
             }
 
-            _utf8.append((byte)(0xFF & rowsym[_node]));
+            int i = 0xFF & rowsym[_node];
+            i = Huffman.getReplacementCharacter(i);
+            _builder.append((byte)i);
             _bits -= rowbits[_node];
             _node = 0;
         }
@@ -115,14 +119,14 @@ public String decode(ByteBuffer buffer) throws EncodingException
             throw new EncodingException("bad_termination");
         }
 
-        String value = _utf8.toString();
+        String value = _builder.build();
         reset();
         return value;
     }
 
     public void reset()
     {
-        _utf8.reset();
+        _builder.reset();
         _count = 0;
         _current = 0;
         _node = 0;

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

@@ -50,12 +50,12 @@ public static void encode(ByteBuffer buffer, byte[] b)
         encode(CODES, buffer, b);
     }
 
-    public static int octetsNeededLC(String s)
+    public static int octetsNeededLowercase(String s)
     {
         return octetsNeeded(LCCODES, s);
     }
 
-    public static void encodeLC(ByteBuffer buffer, String s)
+    public static void encodeLowercase(ByteBuffer buffer, String s)
     {
         encode(LCCODES, buffer, s);
     }
@@ -67,7 +67,7 @@ private static int octetsNeeded(final int[][] table, String s)
         for (int i = 0; i < len; i++)
         {
             char c = s.charAt(i);
-            if (c >= 128 || c < ' ')
+            if (Huffman.isIllegalCharacter(c))
                 return -1;
             needed += table[c][1];
         }
@@ -88,8 +88,8 @@ private static void encode(final int[][] table, ByteBuffer buffer, String s)
         for (int i = 0; i < len; i++)
         {
             char c = s.charAt(i);
-            if (c >= 128 || c < ' ')
-                throw new IllegalArgumentException();
+            if (Huffman.isIllegalCharacter(c))
+                 throw new IllegalArgumentException();
             int code = table[c][0];
             int bits = table[c][1];
 
@@ -119,9 +119,9 @@ private static void encode(final int[][] table, ByteBuffer buffer, byte[] b)
 
         for (byte value : b)
         {
-            int c = 0xFF & value;
-            int code = table[c][0];
-            int bits = table[c][1];
+            int i = 0xFF & value;
+            int code = table[i][0];
+            int bits = table[i][1];
 
             current <<= bits;
             current |= code;

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

@@ -15,11 +15,13 @@
 
 import java.nio.ByteBuffer;
 
+import org.eclipse.jetty.util.CharsetStringBuilder;
+
 public class NBitStringParser
 {
     private final NBitIntegerParser _integerParser;
     private final HuffmanDecoder _huffmanBuilder;
-    private final StringBuilder _stringBuilder;
+    private final CharsetStringBuilder.Iso8859StringBuilder _builder;
     private boolean _huffman;
     private int _count;
     private int _length;
@@ -38,7 +40,7 @@ public NBitStringParser()
     {
         _integerParser = new NBitIntegerParser();
         _huffmanBuilder = new HuffmanDecoder();
-        _stringBuilder = new StringBuilder();
+        _builder = new CharsetStringBuilder.Iso8859StringBuilder();
     }
 
     public void setPrefix(int prefix)
@@ -70,7 +72,7 @@ public String decode(ByteBuffer buffer) throws EncodingException
                     continue;
 
                 case VALUE:
-                    String value = _huffman ? _huffmanBuilder.decode(buffer) : asciiStringDecode(buffer);
+                    String value = _huffman ? _huffmanBuilder.decode(buffer) : stringDecode(buffer);
                     if (value != null)
                         reset();
                     return value;
@@ -81,23 +83,24 @@ public String decode(ByteBuffer buffer) throws EncodingException
         }
     }
 
-    private String asciiStringDecode(ByteBuffer buffer)
+    private String stringDecode(ByteBuffer buffer)
     {
         for (; _count < _length; _count++)
         {
             if (!buffer.hasRemaining())
                 return null;
-            _stringBuilder.append((char)(0x7F & buffer.get()));
+            _builder.append(buffer.get());
         }
-        return _stringBuilder.toString();
+
+        return _builder.build();
     }
 
     public void reset()
     {
         _state = State.PARSING;
         _integerParser.reset();
         _huffmanBuilder.reset();
-        _stringBuilder.setLength(0);
+        _builder.reset();
         _prefix = 0;
         _count = 0;
         _length = 0;

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/HuffmanTest.java → jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java

@@ -11,24 +11,24 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.http3.qpack;
+package org.eclipse.jetty.http;
 
-import java.nio.BufferOverflowException;
 import java.nio.ByteBuffer;
 import java.util.Locale;
 import java.util.stream.Stream;
 
 import org.eclipse.jetty.http.compression.HuffmanDecoder;
 import org.eclipse.jetty.http.compression.HuffmanEncoder;
 import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.TypeUtil;
-import org.hamcrest.Matchers;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
-import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -72,15 +72,80 @@ public void testEncode(String specSection, String hex, String expected)
         assertEquals(hex.length() / 2, HuffmanEncoder.octetsNeeded(expected));
     }
 
+    public static Stream<Arguments> testDecode8859OnlyArguments()
+    {
+        return Stream.of(
+            // These are valid characters for ISO-8859-1.
+            Arguments.of("FfFe6f", (char)128),
+            Arguments.of("FfFfFbBf", (char)255),
+
+            // RFC9110 specifies these to be replaced as ' ' during decoding.
+            Arguments.of("FfC7", ' '), // (char)0
+            Arguments.of("FfFfFfF7", ' '), // '\r'
+            Arguments.of("FfFfFfF3", ' '), // '\n'
+
+            // We replace control chars with the default replacement character of '?'.
+            Arguments.of("FfFfFfBf", '?') // (char)(' ' - 1)
+        );
+    }
+
+    @ParameterizedTest(name = "[{index}]") // don't include unprintable character in test display-name
+    @MethodSource("testDecode8859OnlyArguments")
+    public void testDecode8859Only(String hexString, char expected) throws Exception
+    {
+        ByteBuffer buffer = ByteBuffer.wrap(StringUtil.fromHexString(hexString));
+        String decoded = HuffmanDecoder.decode(buffer, buffer.remaining());
+        assertThat(decoded, equalTo("" + expected));
+    }
+
+    public static Stream<Arguments> testEncode8859OnlyArguments()
+    {
+        return Stream.of(
+            Arguments.of((char)128, (char)128),
+            Arguments.of((char)255, (char)255),
+            Arguments.of((char)0, null),
+            Arguments.of('\r', null),
+            Arguments.of('\n', null),
+            Arguments.of((char)456, null),
+            Arguments.of((char)256, null),
+            Arguments.of((char)-1, null),
+            Arguments.of((char)(' ' - 1), null)
+        );
+    }
+
     @ParameterizedTest(name = "[{index}]") // don't include unprintable character in test display-name
-    @ValueSource(chars = {(char)128, (char)0, (char)-1, ' ' - 1})
-    public void testEncode8859Only(char bad)
+    @MethodSource("testEncode8859OnlyArguments")
+    public void testEncode8859Only(char value, Character expectedValue) throws Exception
     {
-        String s = "bad '" + bad + "'";
+        String s = "value = '" + value + "'";
+
+        // If expected is null we should not be able to encode.
+        if (expectedValue == null)
+        {
+            assertThat(HuffmanEncoder.octetsNeeded(s), equalTo(-1));
+            assertThrows(Throwable.class, () -> encode(s));
+            return;
+        }
 
-        assertThat(HuffmanEncoder.octetsNeeded(s), Matchers.is(-1));
+        String expected = "value = '" + expectedValue + "'";
+        assertThat(HuffmanEncoder.octetsNeeded(s), greaterThan(0));
+        ByteBuffer buffer = encode(s);
+        String decode = decode(buffer);
+        System.err.println("decoded: " + decode);
+        assertThat(decode, equalTo(expected));
+    }
 
-        assertThrows(BufferOverflowException.class,
-            () -> HuffmanEncoder.encode(BufferUtil.allocate(32), s));
+    private ByteBuffer encode(String s)
+    {
+        ByteBuffer buffer = BufferUtil.allocate(32);
+        BufferUtil.clearToFill(buffer);
+        HuffmanEncoder.encode(buffer, s);
+        BufferUtil.flipToFlush(buffer, 0);
+        return buffer;
+    }
+
+    private String decode(ByteBuffer buffer) throws Exception
+    {
+        return HuffmanDecoder.decode(buffer, buffer.remaining());
     }
 }

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerParserTest.java → jetty-http/src/test/java/org/eclipse/jetty/http/NBitIntegerParserTest.java

@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.http3.qpack;
+package org.eclipse.jetty.http;
 
 import java.nio.ByteBuffer;
 

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerTest.java → jetty-http/src/test/java/org/eclipse/jetty/http/NBitIntegerTest.java

@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.http3.qpack;
+package org.eclipse.jetty.http;
 
 import java.nio.ByteBuffer;