diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java index bc7cc82998d..2bd270829e2 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java @@ -15,6 +15,7 @@ package org.eclipse.jetty.http; import java.util.Collections; import java.util.Iterator; +import java.util.Objects; import java.util.function.Supplier; public class MetaData implements Iterable @@ -100,6 +101,28 @@ public class MetaData implements Iterable 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() { 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..354885b9b9e 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 @@ -346,4 +346,47 @@ public class 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; + } } 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 a043357946f..c0f19dc0851 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 @@ -15,7 +15,7 @@ package org.eclipse.jetty.http.compression; 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 class HuffmanDecoder 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 class HuffmanDecoder } // 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 class HuffmanDecoder 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 class HuffmanDecoder 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; 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 15360d88e55..b6f05b5fc6b 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 @@ -50,12 +50,12 @@ public class HuffmanEncoder 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 @@ public class HuffmanEncoder 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 @@ public class HuffmanEncoder 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 @@ public class HuffmanEncoder 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; 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 5c2456d0383..b3091317f10 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 @@ -15,11 +15,13 @@ package org.eclipse.jetty.http.compression; 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 class NBitStringParser { _integerParser = new NBitIntegerParser(); _huffmanBuilder = new HuffmanDecoder(); - _stringBuilder = new StringBuilder(); + _builder = new CharsetStringBuilder.Iso8859StringBuilder(); } public void setPrefix(int prefix) @@ -70,7 +72,7 @@ public class NBitStringParser 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,15 +83,16 @@ public class NBitStringParser } } - 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() @@ -97,7 +100,7 @@ public class NBitStringParser _state = State.PARSING; _integerParser.reset(); _huffmanBuilder.reset(); - _stringBuilder.setLength(0); + _builder.reset(); _prefix = 0; _count = 0; _length = 0; diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/HuffmanTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java similarity index 50% rename from jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/HuffmanTest.java rename to jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java index 48ce235f8b2..5007877094e 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/HuffmanTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HuffmanTest.java @@ -11,9 +11,8 @@ // ======================================================================== // -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; @@ -21,14 +20,15 @@ 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 class HuffmanTest assertEquals(hex.length() / 2, HuffmanEncoder.octetsNeeded(expected)); } - @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) + public static Stream testDecode8859OnlyArguments() { - String s = "bad '" + bad + "'"; + return Stream.of( + // These are valid characters for ISO-8859-1. + Arguments.of("FfFe6f", (char)128), + Arguments.of("FfFfFbBf", (char)255), - assertThat(HuffmanEncoder.octetsNeeded(s), Matchers.is(-1)); + // RFC9110 specifies these to be replaced as ' ' during decoding. + Arguments.of("FfC7", ' '), // (char)0 + Arguments.of("FfFfFfF7", ' '), // '\r' + Arguments.of("FfFfFfF3", ' '), // '\n' - assertThrows(BufferOverflowException.class, - () -> HuffmanEncoder.encode(BufferUtil.allocate(32), s)); + // 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 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 + @MethodSource("testEncode8859OnlyArguments") + public void testEncode8859Only(char value, Character expectedValue) throws Exception + { + 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; + } + + 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)); + } + + 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()); } } diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/NBitIntegerParserTest.java similarity index 97% rename from jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerParserTest.java rename to jetty-http/src/test/java/org/eclipse/jetty/http/NBitIntegerParserTest.java index 2c0647bc6c4..e0673a64705 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerParserTest.java +++ b/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; diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/NBitIntegerTest.java similarity index 99% rename from jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerTest.java rename to jetty-http/src/test/java/org/eclipse/jetty/http/NBitIntegerTest.java index 695e87791e7..8559b65cfe0 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerTest.java +++ b/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; 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 aa5fadeb19b..77e4d05c61b 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 @@ -24,6 +24,7 @@ import org.eclipse.jetty.http.compression.HuffmanDecoder; import org.eclipse.jetty.http.compression.NBitIntegerParser; import org.eclipse.jetty.http2.hpack.HpackContext.Entry; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.CharsetStringBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,6 +40,7 @@ public class HpackDecoder private final HpackContext _context; private final MetaDataBuilder _builder; + private final HuffmanDecoder _huffmanDecoder; private int _localMaxDynamicTableSize; /** @@ -50,6 +52,7 @@ public class HpackDecoder _context = new HpackContext(localMaxDynamicTableSize); _localMaxDynamicTableSize = localMaxDynamicTableSize; _builder = new MetaDataBuilder(maxHeaderSize); + _huffmanDecoder = new HuffmanDecoder(); } public HpackContext getHpackContext() @@ -168,7 +171,7 @@ public class HpackDecoder if (huffmanName) name = huffmanDecode(buffer, length); else - name = toASCIIString(buffer, length); + name = toISO8859String(buffer, length); check: for (int i = name.length(); i-- > 0; ) { @@ -209,7 +212,7 @@ public class HpackDecoder if (huffmanValue) value = huffmanDecode(buffer, length); else - value = toASCIIString(buffer, length); + value = toISO8859String(buffer, length); // Make the new field HttpField field; @@ -288,7 +291,11 @@ public class HpackDecoder { try { - return HuffmanDecoder.decode(buffer, length); + _huffmanDecoder.setLength(length); + String decoded = _huffmanDecoder.decode(buffer); + if (decoded == null) + throw new HpackException.CompressionException("invalid string encoding"); + return decoded; } catch (EncodingException e) { @@ -296,16 +303,20 @@ public class HpackDecoder compressionException.initCause(e); throw compressionException; } + finally + { + _huffmanDecoder.reset(); + } } - public static String toASCIIString(ByteBuffer buffer, int length) + public static String toISO8859String(ByteBuffer buffer, int length) { - StringBuilder builder = new StringBuilder(length); + CharsetStringBuilder.Iso8859StringBuilder builder = new CharsetStringBuilder.Iso8859StringBuilder(); for (int i = 0; i < length; ++i) { builder.append((char)(0x7F & buffer.get())); } - return builder.toString(); + return builder.build(); } @Override diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java index 0117e992136..e665c839f35 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.http2.hpack; import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; import java.util.EnumMap; import java.util.EnumSet; import java.util.HashSet; @@ -29,6 +28,7 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.PreEncodedHttpField; +import org.eclipse.jetty.http.compression.Huffman; import org.eclipse.jetty.http.compression.HuffmanEncoder; import org.eclipse.jetty.http.compression.NBitIntegerEncoder; import org.eclipse.jetty.http2.hpack.HpackContext.Entry; @@ -441,8 +441,8 @@ public class HpackEncoder // leave name index bits as 0 // Encode the name always with lowercase huffman buffer.put((byte)0x80); - NBitIntegerEncoder.encode(buffer, 7, HuffmanEncoder.octetsNeededLC(name)); - HuffmanEncoder.encodeLC(buffer, name); + NBitIntegerEncoder.encode(buffer, 7, HuffmanEncoder.octetsNeededLowercase(name)); + HuffmanEncoder.encodeLowercase(buffer, name); } else { @@ -456,20 +456,9 @@ public class HpackEncoder { // huffman literal value buffer.put((byte)0x80); - int needed = HuffmanEncoder.octetsNeeded(value); - if (needed >= 0) - { - NBitIntegerEncoder.encode(buffer, 7, needed); - HuffmanEncoder.encode(buffer, value); - } - else - { - // Not iso_8859_1 - byte[] bytes = value.getBytes(StandardCharsets.UTF_8); - NBitIntegerEncoder.encode(buffer, 7, HuffmanEncoder.octetsNeeded(bytes)); - HuffmanEncoder.encode(buffer, bytes); - } + NBitIntegerEncoder.encode(buffer, 7, needed); + HuffmanEncoder.encode(buffer, value); } else { @@ -479,15 +468,7 @@ public class HpackEncoder for (int i = 0; i < value.length(); i++) { char c = value.charAt(i); - if (c < ' ' || c > 127) - { - // Not iso_8859_1, so re-encode as UTF-8 - buffer.reset(); - byte[] bytes = value.getBytes(StandardCharsets.UTF_8); - NBitIntegerEncoder.encode(buffer, 7, bytes.length); - buffer.put(bytes, 0, bytes.length); - return; - } + c = Huffman.getReplacementCharacter(c); buffer.put((byte)c); } } diff --git a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackFieldPreEncoder.java b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackFieldPreEncoder.java index 7d60108e105..3b924f0368b 100644 --- a/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackFieldPreEncoder.java +++ b/jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackFieldPreEncoder.java @@ -73,8 +73,8 @@ public class HpackFieldPreEncoder implements HttpFieldPreEncoder else { buffer.put((byte)0x80); - NBitIntegerEncoder.encode(buffer, 7, HuffmanEncoder.octetsNeededLC(name)); - HuffmanEncoder.encodeLC(buffer, name); + NBitIntegerEncoder.encode(buffer, 7, HuffmanEncoder.octetsNeededLowercase(name)); + HuffmanEncoder.encodeLowercase(buffer, name); } HpackEncoder.encodeValue(buffer, huffman, value); diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java index 576d310937d..c1fb03ab666 100644 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java +++ b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackTest.java @@ -134,26 +134,27 @@ public class HpackTest } @Test - public void encodeDecodeNonAscii() throws Exception + public void encodeNonAscii() throws Exception { HpackEncoder encoder = new HpackEncoder(); - HpackDecoder decoder = new HpackDecoder(4096, 8192); ByteBuffer buffer = BufferUtil.allocate(16 * 1024); HttpFields fields0 = HttpFields.build() - // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck + // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck .add("Cookie", "[\uD842\uDF9F]") .add("custom-key", "[\uD842\uDF9F]"); Response original0 = new MetaData.Response(HttpVersion.HTTP_2, 200, fields0); - BufferUtil.clearToFill(buffer); - encoder.encode(buffer, original0); - BufferUtil.flipToFlush(buffer, 0); - Response decoded0 = (Response)decoder.decode(buffer); + HpackException.SessionException throwable = assertThrows(HpackException.SessionException.class, () -> + { + BufferUtil.clearToFill(buffer); + encoder.encode(buffer, original0); + BufferUtil.flipToFlush(buffer, 0); + }); - assertMetaDataSame(original0, decoded0); + assertThat(throwable.getMessage(), containsString("Could not hpack encode")); } - + @Test public void evictReferencedFieldTest() throws Exception { 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 deleted file mode 100644 index a2daea5ba2e..00000000000 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HuffmanTest.java +++ /dev/null @@ -1,84 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.http2.hpack; - -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.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.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; - -public class HuffmanTest -{ - public static Stream data() - { - return Stream.of( - new String[][]{ - {"D.4.1", "f1e3c2e5f23a6ba0ab90f4ff", "www.example.com"}, - {"D.4.2", "a8eb10649cbf", "no-cache"}, - {"D.6.1k", "6402", "302"}, - {"D.6.1v", "aec3771a4b", "private"}, - {"D.6.1d", "d07abe941054d444a8200595040b8166e082a62d1bff", "Mon, 21 Oct 2013 20:13:21 GMT"}, - {"D.6.1l", "9d29ad171863c78f0b97c8e9ae82ae43d3", "https://www.example.com"}, - {"D.6.2te", "640cff", "303"}, - }).map(Arguments::of); - } - - @ParameterizedTest(name = "[{index}] spec={0}") - @MethodSource("data") - public void testDecode(String specSection, String hex, String expected) throws Exception - { - byte[] encoded = StringUtil.fromHexString(hex); - String decoded = HuffmanDecoder.decode(ByteBuffer.wrap(encoded), encoded.length); - assertEquals(expected, decoded, specSection); - } - - @ParameterizedTest(name = "[{index}] spec={0}") - @MethodSource("data") - public void testEncode(String specSection, String hex, String expected) - { - ByteBuffer buf = BufferUtil.allocate(1024); - int pos = BufferUtil.flipToFill(buf); - HuffmanEncoder.encode(buf, expected); - BufferUtil.flipToFlush(buf, pos); - String encoded = StringUtil.toHexString(BufferUtil.toArray(buf)).toLowerCase(Locale.ENGLISH); - assertEquals(hex, encoded, specSection); - assertEquals(hex.length() / 2, HuffmanEncoder.octetsNeeded(expected)); - } - - @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) - { - String s = "bad '" + bad + "'"; - - assertThat(HuffmanEncoder.octetsNeeded(s), Matchers.is(-1)); - - assertThrows(BufferOverflowException.class, - () -> HuffmanEncoder.encode(BufferUtil.allocate(32), s)); - } -} diff --git a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/NBitIntegerTest.java b/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/NBitIntegerTest.java deleted file mode 100644 index 29615a5b514..00000000000 --- a/jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/NBitIntegerTest.java +++ /dev/null @@ -1,201 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.http2.hpack; - -import java.nio.ByteBuffer; - -import org.eclipse.jetty.http.compression.NBitIntegerEncoder; -import org.eclipse.jetty.http.compression.NBitIntegerParser; -import org.eclipse.jetty.util.BufferUtil; -import org.eclipse.jetty.util.StringUtil; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -public class NBitIntegerTest -{ - - @Test - public void testOctetsNeeded() - { - assertEquals(0, NBitIntegerEncoder.octetsNeeded(5, 10)); - assertEquals(2, NBitIntegerEncoder.octetsNeeded(5, 1337)); - assertEquals(1, NBitIntegerEncoder.octetsNeeded(8, 42)); - assertEquals(3, NBitIntegerEncoder.octetsNeeded(8, 1337)); - - assertEquals(0, NBitIntegerEncoder.octetsNeeded(6, 62)); - assertEquals(1, NBitIntegerEncoder.octetsNeeded(6, 63)); - assertEquals(1, NBitIntegerEncoder.octetsNeeded(6, 64)); - assertEquals(2, NBitIntegerEncoder.octetsNeeded(6, 63 + 0x00 + 0x80 * 0x01)); - assertEquals(3, NBitIntegerEncoder.octetsNeeded(6, 63 + 0x00 + 0x80 * 0x80)); - assertEquals(4, NBitIntegerEncoder.octetsNeeded(6, 63 + 0x00 + 0x80 * 0x80 * 0x80)); - } - - @Test - public void testEncode() - { - testEncode(6, 0, "00"); - testEncode(6, 1, "01"); - testEncode(6, 62, "3e"); - testEncode(6, 63, "3f00"); - testEncode(6, 63 + 1, "3f01"); - testEncode(6, 63 + 0x7e, "3f7e"); - testEncode(6, 63 + 0x7f, "3f7f"); - testEncode(6, 63 + 0x00 + 0x80 * 0x01, "3f8001"); - testEncode(6, 63 + 0x01 + 0x80 * 0x01, "3f8101"); - testEncode(6, 63 + 0x7f + 0x80 * 0x01, "3fFf01"); - testEncode(6, 63 + 0x00 + 0x80 * 0x02, "3f8002"); - testEncode(6, 63 + 0x01 + 0x80 * 0x02, "3f8102"); - testEncode(6, 63 + 0x7f + 0x80 * 0x7f, "3fFf7f"); - testEncode(6, 63 + 0x00 + 0x80 * 0x80, "3f808001"); - testEncode(6, 63 + 0x7f + 0x80 * 0x80 * 0x7f, "3fFf807f"); - testEncode(6, 63 + 0x00 + 0x80 * 0x80 * 0x80, "3f80808001"); - - testEncode(8, 0, "00"); - testEncode(8, 1, "01"); - testEncode(8, 128, "80"); - testEncode(8, 254, "Fe"); - testEncode(8, 255, "Ff00"); - testEncode(8, 255 + 1, "Ff01"); - testEncode(8, 255 + 0x7e, "Ff7e"); - testEncode(8, 255 + 0x7f, "Ff7f"); - testEncode(8, 255 + 0x80, "Ff8001"); - testEncode(8, 255 + 0x00 + 0x80 * 0x80, "Ff808001"); - } - - public void testEncode(int n, int i, String expected) - { - ByteBuffer buf = BufferUtil.allocate(16); - int p = BufferUtil.flipToFill(buf); - if (n < 8) - buf.put((byte)0x00); - NBitIntegerEncoder.encode(buf, n, i); - BufferUtil.flipToFlush(buf, p); - String r = StringUtil.toHexString(BufferUtil.toArray(buf)); - assertEquals(expected, r); - - assertEquals(expected.length() / 2, (n < 8 ? 1 : 0) + NBitIntegerEncoder.octetsNeeded(n, i)); - } - - @Test - public void testDecode() throws Exception - { - testDecode(6, 0, "00"); - testDecode(6, 1, "01"); - testDecode(6, 62, "3e"); - testDecode(6, 63, "3f00"); - testDecode(6, 63 + 1, "3f01"); - testDecode(6, 63 + 0x7e, "3f7e"); - testDecode(6, 63 + 0x7f, "3f7f"); - testDecode(6, 63 + 0x80, "3f8001"); - testDecode(6, 63 + 0x81, "3f8101"); - testDecode(6, 63 + 0x7f + 0x80 * 0x01, "3fFf01"); - testDecode(6, 63 + 0x00 + 0x80 * 0x02, "3f8002"); - testDecode(6, 63 + 0x01 + 0x80 * 0x02, "3f8102"); - testDecode(6, 63 + 0x7f + 0x80 * 0x7f, "3fFf7f"); - testDecode(6, 63 + 0x00 + 0x80 * 0x80, "3f808001"); - testDecode(6, 63 + 0x7f + 0x80 * 0x80 * 0x7f, "3fFf807f"); - testDecode(6, 63 + 0x00 + 0x80 * 0x80 * 0x80, "3f80808001"); - - testDecode(8, 0, "00"); - testDecode(8, 1, "01"); - testDecode(8, 128, "80"); - testDecode(8, 254, "Fe"); - testDecode(8, 255, "Ff00"); - testDecode(8, 255 + 1, "Ff01"); - testDecode(8, 255 + 0x7e, "Ff7e"); - testDecode(8, 255 + 0x7f, "Ff7f"); - testDecode(8, 255 + 0x80, "Ff8001"); - testDecode(8, 255 + 0x00 + 0x80 * 0x80, "Ff808001"); - } - - public void testDecode(int n, int expected, String encoded) throws Exception - { - ByteBuffer buf = ByteBuffer.wrap(StringUtil.fromHexString(encoded)); - buf.position(n == 8 ? 0 : 1); - assertEquals(expected, NBitIntegerParser.decode(buf, n)); - } - - @Test - public void testEncodeExampleD11() - { - ByteBuffer buf = BufferUtil.allocate(16); - int p = BufferUtil.flipToFill(buf); - buf.put((byte)0x77); - buf.put((byte)0xFF); - NBitIntegerEncoder.encode(buf, 5, 10); - BufferUtil.flipToFlush(buf, p); - - String r = StringUtil.toHexString(BufferUtil.toArray(buf)); - - assertEquals("77Ea", r); - } - - @Test - public void testDecodeExampleD11() throws Exception - { - ByteBuffer buf = ByteBuffer.wrap(StringUtil.fromHexString("77EaFF")); - buf.position(2); - - assertEquals(10, NBitIntegerParser.decode(buf, 5)); - } - - @Test - public void testEncodeExampleD12() - { - ByteBuffer buf = BufferUtil.allocate(16); - int p = BufferUtil.flipToFill(buf); - buf.put((byte)0x88); - buf.put((byte)0x00); - NBitIntegerEncoder.encode(buf, 5, 1337); - BufferUtil.flipToFlush(buf, p); - - String r = StringUtil.toHexString(BufferUtil.toArray(buf)); - - assertEquals("881f9a0a", r); - } - - @Test - public void testDecodeExampleD12() throws Exception - { - ByteBuffer buf = ByteBuffer.wrap(StringUtil.fromHexString("881f9a0aff")); - buf.position(2); - - assertEquals(1337, NBitIntegerParser.decode(buf, 5)); - } - - @Test - public void testEncodeExampleD13() - { - ByteBuffer buf = BufferUtil.allocate(16); - int p = BufferUtil.flipToFill(buf); - buf.put((byte)0x88); - buf.put((byte)0xFF); - NBitIntegerEncoder.encode(buf, 8, 42); - BufferUtil.flipToFlush(buf, p); - - String r = StringUtil.toHexString(BufferUtil.toArray(buf)); - - assertEquals("88Ff2a", r); - } - - @Test - public void testDecodeExampleD13() throws Exception - { - ByteBuffer buf = ByteBuffer.wrap(StringUtil.fromHexString("882aFf")); - buf.position(1); - - assertEquals(42, NBitIntegerParser.decode(buf, 8)); - } -} diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/EncodableEntry.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/EncodableEntry.java index bc202e09807..542232dca9c 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/EncodableEntry.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/EncodableEntry.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http3.qpack.internal; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.Objects; import org.eclipse.jetty.http.HttpField; @@ -173,7 +174,7 @@ public abstract class EncodableEntry { buffer.put((byte)0x00); NBitIntegerEncoder.encode(buffer, 7, value.length()); - buffer.put(value.getBytes()); + buffer.put(value.getBytes(StandardCharsets.ISO_8859_1)); } } @@ -229,13 +230,12 @@ public abstract class EncodableEntry } else { - // TODO: What charset should we be using? (this applies to the instruction generators as well). buffer.put((byte)(0x20 | allowIntermediary)); NBitIntegerEncoder.encode(buffer, 3, name.length()); - buffer.put(name.getBytes()); + buffer.put(name.getBytes(StandardCharsets.ISO_8859_1)); buffer.put((byte)0x00); NBitIntegerEncoder.encode(buffer, 7, value.length()); - buffer.put(value.getBytes()); + buffer.put(value.getBytes(StandardCharsets.ISO_8859_1)); } } @@ -268,7 +268,6 @@ public abstract class EncodableEntry } } - // TODO: pass in the HTTP version to avoid hard coding HTTP3? private static class PreEncodedEntry extends EncodableEntry { private final PreEncodedHttpField _httpField; diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/IndexedNameEntryInstruction.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/IndexedNameEntryInstruction.java index 35c225fa9f1..555703192b1 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/IndexedNameEntryInstruction.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/IndexedNameEntryInstruction.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http3.qpack.internal.instruction; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import org.eclipse.jetty.http.compression.HuffmanEncoder; import org.eclipse.jetty.http.compression.NBitIntegerEncoder; @@ -72,7 +73,7 @@ public class IndexedNameEntryInstruction implements Instruction { buffer.put((byte)(0x00)); NBitIntegerEncoder.encode(buffer, 7, _value.length()); - buffer.put(_value.getBytes()); + buffer.put(_value.getBytes(StandardCharsets.ISO_8859_1)); } BufferUtil.flipToFlush(buffer, 0); diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/LiteralNameEntryInstruction.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/LiteralNameEntryInstruction.java index 2caad6044b6..8ebed39a0c0 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/LiteralNameEntryInstruction.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/LiteralNameEntryInstruction.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.http3.qpack.internal.instruction; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.compression.HuffmanEncoder; @@ -69,7 +70,7 @@ public class LiteralNameEntryInstruction implements Instruction { buffer.put((byte)(0x40)); NBitIntegerEncoder.encode(buffer, 5, _name.length()); - buffer.put(_name.getBytes()); + buffer.put(_name.getBytes(StandardCharsets.ISO_8859_1)); } if (_huffmanValue) @@ -82,7 +83,7 @@ public class LiteralNameEntryInstruction implements Instruction { buffer.put((byte)(0x00)); NBitIntegerEncoder.encode(buffer, 5, _value.length()); - buffer.put(_value.getBytes()); + buffer.put(_value.getBytes(StandardCharsets.ISO_8859_1)); } BufferUtil.flipToFlush(buffer, 0); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java b/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java new file mode 100644 index 00000000000..9aa2d783cd6 --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java @@ -0,0 +1,279 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.util; + +import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.Charset; +import java.nio.charset.CharsetDecoder; +import java.nio.charset.CodingErrorAction; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Objects; + +/** + *

Build a string from a sequence of bytes.

+ *

Implementations of this interface are optimized for processing a mix of calls to already decoded + * character based appends (e.g. {@link #append(char)} and calls to undecoded byte methods (e.g. {@link #append(byte)}. + * This is particularly useful for decoding % encoded strings that are mostly already decoded but may contain + * escaped byte sequences that are not decoded. The standard {@link CharsetDecoder} API is not well suited for this + * use-case.

+ *

Any coding errors in the string will be reported by a {@link CharacterCodingException} thrown + * from the {@link #build()} method.

+ * @see Utf8StringBuilder for UTF-8 decoding with replacement of coding errors and/or fast fail behaviour. + * @see CharsetDecoder for decoding arbitrary {@link Charset}s with control over {@link CodingErrorAction}. + */ +public interface CharsetStringBuilder +{ + void append(byte b); + + void append(char c); + + default void append(byte[] bytes) + { + append(bytes, 0, bytes.length); + } + + default void append(byte[] b, int offset, int length) + { + int end = offset + length; + for (int i = offset; i < end; i++) + append(b[i]); + } + + default void append(CharSequence chars, int offset, int length) + { + int end = offset + length; + for (int i = offset; i < end; i++) + append(chars.charAt(i)); + } + + default void append(ByteBuffer buf) + { + int end = buf.position() + buf.remaining(); + while (buf.position() < end) + append(buf.get()); + } + + /** + *

Build the completed string and reset the buffer.

+ * @return The decoded built string which must be complete in regard to any multibyte sequences. + * @throws CharacterCodingException If the bytes cannot be correctly decoded or a multibyte sequence is incomplete. + */ + String build() throws CharacterCodingException; + + void reset(); + + static CharsetStringBuilder forCharset(Charset charset) + { + Objects.requireNonNull(charset); + if (charset == StandardCharsets.ISO_8859_1) + return new Iso8859StringBuilder(); + if (charset == StandardCharsets.US_ASCII) + return new UsAsciiStringBuilder(); + + // Use a CharsetDecoder that defaults to CodingErrorAction#REPORT + return new DecoderStringBuilder(charset.newDecoder()); + } + + class Iso8859StringBuilder implements CharsetStringBuilder + { + private final StringBuilder _builder = new StringBuilder(); + + @Override + public void append(byte b) + { + _builder.append((char)(0xff & b)); + } + + @Override + public void append(char c) + { + _builder.append(c); + } + + @Override + public void append(CharSequence chars, int offset, int length) + { + _builder.append(chars, offset, length); + } + + @Override + public String build() + { + String s = _builder.toString(); + _builder.setLength(0); + return s; + } + + @Override + public void reset() + { + _builder.setLength(0); + } + } + + class UsAsciiStringBuilder implements CharsetStringBuilder + { + private final StringBuilder _builder = new StringBuilder(); + + @Override + public void append(byte b) + { + if (b < 0) + throw new IllegalArgumentException(); + _builder.append((char)b); + } + + @Override + public void append(char c) + { + _builder.append(c); + } + + @Override + public void append(CharSequence chars, int offset, int length) + { + _builder.append(chars, offset, length); + } + + @Override + public String build() + { + String s = _builder.toString(); + _builder.setLength(0); + return s; + } + + @Override + public void reset() + { + _builder.setLength(0); + } + } + + class DecoderStringBuilder implements CharsetStringBuilder + { + private final CharsetDecoder _decoder; + private final StringBuilder _stringBuilder = new StringBuilder(32); + private ByteBuffer _buffer = ByteBuffer.allocate(32); + + public DecoderStringBuilder(CharsetDecoder charsetDecoder) + { + _decoder = charsetDecoder; + } + + private void ensureSpace(int needed) + { + int space = _buffer.remaining(); + if (space < needed) + { + int position = _buffer.position(); + _buffer = ByteBuffer.wrap(Arrays.copyOf(_buffer.array(), _buffer.capacity() + needed - space + 32)).position(position); + } + } + + @Override + public void append(byte b) + { + ensureSpace(1); + _buffer.put(b); + } + + @Override + public void append(char c) + { + if (_buffer.position() > 0) + { + try + { + // Append any data already in the decoder + _stringBuilder.append(_decoder.decode(_buffer.flip())); + _buffer.clear(); + } + catch (CharacterCodingException e) + { + // This will be thrown only if the decoder is configured to REPORT, + // otherwise errors will be ignored or replaced and we will not catch here. + throw new RuntimeException(e); + } + } + _stringBuilder.append(c); + } + + @Override + public void append(CharSequence chars, int offset, int length) + { + if (_buffer.position() > 0) + { + try + { + // Append any data already in the decoder + _stringBuilder.append(_decoder.decode(_buffer.flip())); + _buffer.clear(); + } + catch (CharacterCodingException e) + { + // This will be thrown only if the decoder is configured to REPORT, + // otherwise errors will be ignored or replaced and we will not catch here. + throw new RuntimeException(e); + } + } + _stringBuilder.append(chars, offset, offset + length); + } + + @Override + public void append(byte[] b, int offset, int length) + { + ensureSpace(length); + _buffer.put(b, offset, length); + } + + @Override + public void append(ByteBuffer buf) + { + ensureSpace(buf.remaining()); + _buffer.put(buf); + } + + @Override + public String build() throws CharacterCodingException + { + try + { + if (_buffer.position() > 0) + { + CharSequence decoded = _decoder.decode(_buffer.flip()); + _buffer.clear(); + if (_stringBuilder.length() == 0) + return decoded.toString(); + _stringBuilder.append(decoded); + } + return _stringBuilder.toString(); + } + finally + { + _stringBuilder.setLength(0); + } + } + + @Override + public void reset() + { + _stringBuilder.setLength(0); + } + } +} + +