hpack: decode header blocks from any InputStream, without copying

[pjfanning]

Follows #1231, which removed the decoder's assumption that a single read() fills the buffer. This removes the remaining reasons the decoder needed a ByteArrayInputStream, and then stops copying the header block.

Why the copy was there

HeaderDecompression did payload.compact.asInputStream, with the comment "only compact ByteString supports InputStream with mark/reset". That was accurate — asInputStream differs by ByteString implementation:

implementation	stream	mark/reset	available()
ByteString1C (compact)	UnsynchronizedByteArrayInputStream	yes	exact
ByteString1 (array slice)	UnsynchronizedByteArrayInputStream	yes	exact
ByteStrings (rope)	SequenceInputStream	no	current chunk only

A HEADERS frame plus CONTINUATION frames is assembled with ++ (HeaderDecompression.scala:158), so it is a rope. And a single-frame payload is normally a slice of a larger network buffer, where ByteString1.compact is ByteString1C(toArray) — a full copy — even though its stream already had everything the decoder needed.

Three dependencies on ByteArrayInputStream semantics remained after #1231:

• decodeULE128 used in.mark(5) / in.reset() to rewind a partially read varint
• the main loop was driven by while (in.available() > 0)
• the literal name and value states waited for available() >= length

On a SequenceInputStream the first throws IOException (so: COMPRESSION_ERROR GOAWAY), and the other two read 0 at a chunk boundary — the loop would exit early and silently truncate the block.

What changed

The decoder now only reads forward:

• Main loop. while (in.available() > 0) becomes while (true). The end of the stream at READ_HEADER_REPRESENTATION is the normal end of a header block and returns; in every other state it is truncated input.
• decodeULE128. No mark/reset. Running out of input inside a varint is a decompression failure rather than a rewind-and-ask-for-more.
• readByte / skipFully helpers. readByte turns the end of the stream into a decompression failure; skipFully skips a run in one go and copes with skip() legitimately returning 0 (the old code relied on the available()-driven loop to retry, which would now spin).
• Literal name/value. The available() >= length guards are gone — fix: read HPACK string literals with readNBytes #1231 made readStringLiteral use readNBytes and compare the length, so a short literal is already reported.

HeaderDecompression then hands the payload straight over: no compact(), so header blocks are no longer copied.

Behaviour change worth reviewing

A header block that ends mid-representation now raises a decompression failure — COMPRESSION_ERROR, which RFC 9113 §4.3 requires for a header block decoding error. Previously decode() returned early and the partially decoded headers were emitted as a normal ParsedHeadersFrame. I think the new behaviour is the correct one, but it is a change, so flagging it rather than burying it.

Related: the decoder no longer supports being fed a block incrementally across decode() calls. That machinery came from the upstream twitter/netty decoder, which was designed for incremental network feeding. parseAndEmit is the only caller and always assembles the complete block first, so none of it was reachable here.

Tests

HpackDecoderSpec gains five cases:

• decoding over a SequenceInputStream that supports neither mark/reset nor a whole-block available() (asserts markSupported() == false), at chunk sizes 4, 3 and 1 — so string literals and varints straddle chunk boundaries
• a block truncated inside a string literal, and one truncated inside a length prefix, both expected to fail

The chunked cases fail on main with a decompression failure and pass here.

• HpackDecoderSpec — 8 passed
• http-core/testOnly org.apache.pekko.http.impl.engine.http2.* — 18 passed
• http2-tests/test — 352 passed, 25 ignored, 26 pending
• scalafmtCheckAll, javafmtCheckAll, headerCheck, http-core/mimaReportBinaryIssues — clean

Unrelated thing noticed while in here

HeaderDecompression.parseAndEmit calls decoder.endHeaderBlock() inside the try, so when a ParsingException escapes the HeaderListener (e.g. a malformed content-type), endHeaderBlock() is skipped and the decoder's reset() never runs — leaving state and headerSize mid-block for the next HEADERS frame on that connection. Pre-existing and untouched here; happy to file it separately.

🤖 Generated with Claude Code