diff --git a/http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala b/http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala new file mode 100644 index 000000000..095430fd3 --- /dev/null +++ b/http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.pekko.http.shaded.com.twitter.hpack + +import java.io.ByteArrayOutputStream +import java.util.concurrent.TimeUnit + +import org.openjdk.jmh.annotations._ + +import org.apache.pekko.util.ByteString + +@State(Scope.Benchmark) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(1) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +class HpackDecoderBenchmark { + + /** `default` is what a browser sends (Huffman where shorter), the other two force one string literal form. */ + @Param(Array("default", "huffman", "raw")) + var mode = "default" + + var decoder: Decoder = null + var headerBlock: ByteString = null + + // a typical browser request; the block is encoded without indexing so that every value is a string literal + // and decoding it does the same work on every call + val headers: Array[(String, String)] = Array( + ":method" -> "GET", + ":scheme" -> "https", + ":authority" -> "www.example.com", + ":path" -> "/api/v1/users/12345/orders?page=2&sort=created_at&direction=desc", + "user-agent" -> + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", + "accept" -> "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8", + "accept-language" -> "en-US,en;q=0.9", + "accept-encoding" -> "gzip, deflate, br", + "cookie" -> "session=ab12cd34ef56ab12cd34ef56ab12cd34ef56ab12; theme=dark; consent=1", + "cache-control" -> "no-cache") + + private val listener: HeaderListener = + (_: String, _: String, parsed: AnyRef, _: Boolean) => parsed + + @Setup + def setup(): Unit = { + val encoder = mode match { + case "default" => new Encoder(4096, false, false, false) + case "huffman" => new Encoder(4096, false, true, false) + case "raw" => new Encoder(4096, false, false, true) + } + val out = new ByteArrayOutputStream() + headers.foreach { case (name, value) => encoder.encodeHeader(out, name, value, false) } + headerBlock = ByteString(out.toByteArray) + decoder = new Decoder(8192, 4096) + } + + @Benchmark + def decodeHeaders(): Unit = { + decoder.decode(headerBlock.asInputStream, listener) + decoder.endHeaderBlock() + } +} diff --git a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java index 63bae0b75..b0a07f021 100644 --- a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java +++ b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java @@ -32,10 +32,10 @@ package org.apache.pekko.http.shaded.com.twitter.hpack; import static org.apache.pekko.http.shaded.com.twitter.hpack.HeaderField.HEADER_ENTRY_OVERHEAD; +import static org.apache.pekko.http.shaded.com.twitter.hpack.HpackUtil.ISO_8859_1; import java.io.IOException; import java.io.InputStream; -import org.apache.pekko.http.impl.util.StringTools; import org.apache.pekko.http.shaded.com.twitter.hpack.HpackUtil.IndexType; public final class Decoder { @@ -67,6 +67,12 @@ public final class Decoder { private int valueLength; private String name; + // scratch arrays for reading and Huffman decoding string literals, reused across header blocks; + // they grow to the longest literal seen on the connection, which the max header (list) size and + // the dynamic table capacity bound + private byte[] literalBuf = new byte[128]; + private byte[] decodedBuf = new byte[128]; + private enum State { READ_HEADER_REPRESENTATION, READ_MAX_DYNAMIC_TABLE_SIZE, @@ -508,21 +514,32 @@ private boolean exceedsMaxHeaderSize(long size) { } private String readStringLiteral(InputStream in, int length) throws IOException { + // read the literal into the reusable scratch array: the String constructor copies anyway, so + // a per-literal array would be a second allocation and copy + literalBuf = ensureCapacity(literalBuf, length); // readNBytes rather than read: InputStream.read(byte[]) is free to return fewer bytes than // requested even when more are available, which would be reported here as a decompression // failure - byte[] buf = in.readNBytes(length); - if (buf.length != length) { + if (in.readNBytes(literalBuf, 0, length) != length) { throw DECOMPRESSION_EXCEPTION; } - final byte[] result; if (huffmanEncoded) { - result = Huffman.DECODER.decode(buf); + decodedBuf = ensureCapacity(decodedBuf, HuffmanDecoder.maxDecodedLength(length)); + int decodedLength = Huffman.DECODER.decode(literalBuf, length, decodedBuf); + // ISO-8859-1 maps every octet to the char of the same value: string literals are opaque + // octets + return new String(decodedBuf, 0, decodedLength, ISO_8859_1); } else { - result = buf; + return new String(literalBuf, 0, length, ISO_8859_1); + } + } + + private static byte[] ensureCapacity(byte[] buf, int length) { + if (buf.length >= length) { + return buf; } - return StringTools.asciiStringFromBytes(result); + return new byte[Math.max(length, buf.length << 1)]; } private static byte readByte(InputStream in) throws IOException { diff --git a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java index 3d338690f..1b6e999be 100644 --- a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java +++ b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java @@ -31,7 +31,6 @@ package org.apache.pekko.http.shaded.com.twitter.hpack; -import java.io.ByteArrayOutputStream; import java.io.IOException; final class HuffmanDecoder { @@ -55,20 +54,33 @@ final class HuffmanDecoder { } /** - * Decompresses the given Huffman coded string literal. + * The shortest code in the HPACK Huffman table is 5 bits long, so a coded string literal of + * n bytes decodes to at most n * 8 / 5 symbols. + */ + static int maxDecodedLength(int codedLength) { + return (int) (((long) codedLength * 8) / 5); + } + + /** + * Decompresses the given Huffman coded string literal into out, which must hold at + * least {@link #maxDecodedLength(int)} bytes for length. The symbols are written by + * index instead of through an OutputStream, whose per-symbol write(int) + * (synchronized on ByteArrayOutputStream) together with the growth and copy-out of + * that stream dominated the cost of decoding. * * @param buf the string literal to be decoded - * @return the output stream for the compressed data - * @throws IOException if an I/O error occurs. In particular, an IOException may be - * thrown if the output stream has been closed. + * @param length the number of bytes of buf that make up the string literal + * @param out the array to write the decoded symbols to, starting at index 0 + * @return the number of symbols written to out + * @throws IOException if the coded data contains the EOS symbol or is not padded with the most + * significant bits of the EOS code */ - public byte[] decode(byte[] buf) throws IOException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - + public int decode(byte[] buf, int length, byte[] out) throws IOException { Node node = root; int current = 0; int bits = 0; - for (int i = 0; i < buf.length; i++) { + int pos = 0; + for (int i = 0; i < length; i++) { int b = buf[i] & 0xFF; current = (current << 8) | b; bits += 8; @@ -80,7 +92,7 @@ public byte[] decode(byte[] buf) throws IOException { if (node.symbol == HpackUtil.HUFFMAN_EOS) { throw EOS_DECODED; } - baos.write(node.symbol); + out[pos++] = (byte) node.symbol; node = root; } } @@ -91,7 +103,7 @@ public byte[] decode(byte[] buf) throws IOException { node = node.children[c]; if (node.isTerminal() && node.bits <= bits) { bits -= node.bits; - baos.write(node.symbol); + out[pos++] = (byte) node.symbol; node = root; } else { break; @@ -106,7 +118,7 @@ public byte[] decode(byte[] buf) throws IOException { throw INVALID_PADDING; } - return baos.toByteArray(); + return pos; } private static final class Node { diff --git a/http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes b/http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes new file mode 100644 index 000000000..82dbf91b5 --- /dev/null +++ b/http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# The shaded HPACK HuffmanDecoder (a package-private class) decodes into a caller-provided array +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.shaded.com.twitter.hpack.HuffmanDecoder.decode") diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala index 2e0d90711..d2fc04e52 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala @@ -63,9 +63,11 @@ class HpackDecoderSpec extends AnyWordSpec with Matchers { out.toByteArray } - private def decode(in: InputStream): Seq[(String, String)] = { + private def decode(in: InputStream): Seq[(String, String)] = + decode(new Decoder(maxHeaderSize, maxHeaderTableSize), in) + + private def decode(decoder: Decoder, in: InputStream): Seq[(String, String)] = { val decoded = ListBuffer.empty[(String, String)] - val decoder = new Decoder(maxHeaderSize, maxHeaderTableSize) decoder.decode(in, new HeaderListener { override def addHeader(name: String, value: String, parsed: AnyRef, sensitive: Boolean): AnyRef = { @@ -116,5 +118,36 @@ class HpackDecoderSpec extends AnyWordSpec with Matchers { // a literal header field with incremental indexing, new name, whose name length never arrives a[IOException] should be thrownBy decode(new ByteArrayInputStream(Array[Byte](0x40))) } + + "decode string literals longer than its initial scratch space, Huffman coded and raw" in { + // the decoder reads literals into reusable arrays that start at 128 bytes; a value of letters is + // Huffman coded, a value of mostly '|' (a 15-bit code) is sent raw because that is shorter + val long = Seq("x-huffman" -> ("abcdefghij" * 50), "x-raw" -> ("|" * 500)) + decode(new ByteArrayInputStream(encode(long: _*))) shouldEqual long + } + + "decode consecutive header blocks of varying literal lengths with the same decoder" in { + // a literal shorter than the previous one must not pick up the tail the previous one left in the + // reused scratch arrays + val decoder = new Decoder(maxHeaderSize, maxHeaderTableSize) + val blocks = Seq( + Seq("x-a" -> ("abcdefghij" * 40)), + Seq("x-b" -> "short", "x-c" -> ("|" * 300)), + Seq("x-d" -> "s", "x-e" -> "|"), + Seq("x-f" -> "")) + blocks.foreach { block => + // a fresh encoder per block, so no block refers to what an earlier one put in the dynamic table + decode(decoder, new ByteArrayInputStream(encode(block: _*))) shouldEqual block + } + } + + "decode a raw string literal carrying opaque octets above 0x7F to the chars of the same value" in { + // literal header field without indexing (0x00), new name: raw name "x-opaque", raw 128-octet value + val name = "x-opaque".getBytes("US-ASCII") + val value = Array.tabulate(128)(i => (0x80 + i).toByte) + // 0x7f 0x01: 7-bit prefix integer 128 = 127 + 1, Huffman flag clear + val block = Array[Byte](0x00, name.length.toByte) ++ name ++ Array[Byte](0x7F, 0x01) ++ value + decode(new ByteArrayInputStream(block)) shouldEqual Seq("x-opaque" -> new String(value, "ISO-8859-1")) + } } } diff --git a/http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala new file mode 100644 index 000000000..b667deb06 --- /dev/null +++ b/http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala @@ -0,0 +1,101 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.pekko.http.shaded.com.twitter.hpack + +import java.io.IOException +import java.nio.charset.StandardCharsets + +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +class HuffmanDecoderSpec extends AnyWordSpec with Matchers { + + private def bytes(hex: String): Array[Byte] = hex.grouped(2).map(Integer.parseInt(_, 16).toByte).toArray + + private def decode(coded: Array[Byte]): Array[Byte] = { + val out = new Array[Byte](HuffmanDecoder.maxDecodedLength(coded.length)) + val length = Huffman.DECODER.decode(coded, coded.length, out) + out.take(length) + } + + /** Huffman codes the octets straight from the HPACK table, independently of HuffmanEncoder. */ + private def encode(octets: Array[Byte]): Array[Byte] = { + val out = new java.io.ByteArrayOutputStream() + var current = 0L + var bits = 0 + octets.foreach { octet => + val symbol = octet & 0xFF + val nbits = HpackUtil.HUFFMAN_CODE_LENGTHS(symbol) + current = (current << nbits) | HpackUtil.HUFFMAN_CODES(symbol) + bits += nbits + while (bits >= 8) { + bits -= 8 + out.write((current >> bits).toInt) + } + } + if (bits > 0) out.write(((current << (8 - bits)) | (0xFF >>> bits)).toInt) + out.toByteArray + } + + "HuffmanDecoder" should { + "decode the string literals from RFC 7541 Appendix C.4" in { + Seq( + "f1e3c2e5f23a6ba0ab90f4ff" -> "www.example.com", + "a8eb10649cbf" -> "no-cache", + "25a849e95ba97d7f" -> "custom-key", + "25a849e95bb8e8b4bf" -> "custom-value").foreach { + case (coded, expected) => + new String(decode(bytes(coded)), StandardCharsets.ISO_8859_1) shouldEqual expected + } + } + + "decode every octet to the symbol of the same value" in { + val allOctets = Array.tabulate(256)(_.toByte) + decode(encode(allOctets)) shouldEqual allOctets + } + + "decode nothing from an empty literal" in { + decode(Array.emptyByteArray) shouldEqual Array.emptyByteArray + } + + "fill the output bound exactly for a literal made of the shortest codes" in { + // '0' has the 5-bit code 00000, so 40 coded bytes carry 64 symbols, the maxDecodedLength bound + val zeros = Array.fill[Byte](64)('0') + val coded = encode(zeros) + coded.length shouldEqual 40 + HuffmanDecoder.maxDecodedLength(coded.length) shouldEqual 64 + decode(coded) shouldEqual zeros + } + + "only read the given number of bytes of the input" in { + val coded = bytes("a8eb10649cbf") ++ Array[Byte](0, 0, 0) + val out = new Array[Byte](HuffmanDecoder.maxDecodedLength(6)) + val length = Huffman.DECODER.decode(coded, 6, out) + new String(out, 0, length, StandardCharsets.ISO_8859_1) shouldEqual "no-cache" + } + + "reject a literal containing the EOS symbol" in { + (the[IOException] thrownBy decode(bytes("ffffffff"))).getMessage shouldEqual "EOS Decoded" + } + + "reject padding that is not the most significant bits of the EOS code" in { + // 00000 decodes '0' and leaves three zero padding bits, which must have been ones + (the[IOException] thrownBy decode(bytes("00"))).getMessage shouldEqual "Invalid Padding" + } + } +}