fix: do not send a response body to HTTP/2 HEAD requests - #1239
Merged
Conversation
pjfanning
requested review from
He-Pin,
jrudolph,
nvollmar,
raboof and
samueleresca
August 29, 2026 09:54
Motivation: The HTTP/2 engine had no notion of the HEAD method, which apache#1238 pinned as three failing expectations: - the response entity was emitted as DATA frames, although RFC 9110 section 9.3.2 says a server MUST NOT send content in a response to a HEAD request - `transparent-head-requests` was applied only by HttpServerBluePrint, so it had no effect over HTTP/2 - a 304 (and a 204) got `content-length: 0`, where HTTP/1.1 omits the header; RFC 9110 section 15.4.5 expects a 304 to carry the content-length a 200 would have had, so a zero is actively misleading The response path only ever sees an HttpResponse plus its stream id, so ResponseRendering cannot know the request method. Carrying it on an attribute would only work for the `bind` API, since `bindFlow` users copy the stream id attribute by hand. Modification: Track the method where it is already known per connection. Http2StreamHandling records the stream ids of incoming HEAD requests when the request HEADERS frame opens the stream, and handleOutgoingCreated cancels the response data and sends the initial headers with endStream set for those streams. The header pairs are left untouched, so the peer still learns the content-length it would have got for a GET. Entries are removed when the response is created and when the stream closes. Because that tracking reads the method off the wire below the HTTP layer, it is unaffected by RequestParsing rewriting HEAD to GET, so transparent-head-requests can now be honoured for HTTP/2 the same way HttpServerBluePrint honours it for HTTP/1.1. ResponseRendering gains the status based part of the rules HTTP/1.1 applies via HttpMethod.contentLengthAllowed, so 1xx, 204 and 304 no longer render a content-length. Result: A HEAD request over HTTP/2 gets headers only, with the content-length the resource would have had, and transparent-head-requests behaves as it does for HTTP/1.1. Tests: - sbt "http2-tests / test" - 349 passed, 25 ignored, 26 pending - sbt +mimaReportBinaryIssues - success - scalafmt --mode diff-ref=upstream/main - clean - git diff --check - clean References: Refs apache#1236, Refs apache#1238
…TP/2 Motivation: PR apache#962 introduced two sets of rules in HttpMethod.contentLengthAllowed: a method specific one for HEAD and CONNECT, and a status based one that applies to every method. HTTP/2 had no coverage for either. The 204 and 304 cases added alongside the HEAD fixes were exercised with a HEAD request only, even though the status based rules do not depend on the method, so a regression on GET would have gone unnoticed. Modification: Move the 204 and 304 cases out of the HEAD section into a new section that drives them with a plain GET, and add the 200 and 205 cases. 205 is the status PR apache#962 was really aimed at: RFC 9112 section 6.3 exempts only 1xx, 204 and 304 from framing, so a 205 must be framed, and it is what http4s/http4s#7919 reports against another server. Add a RequestParsingSpec case pinning that a CONNECT request is rejected, because neither RFC 9113 section 8.5 CONNECT nor RFC 8441 extended CONNECT is supported: the request omits ":scheme" and ":path", which the parser treats as mandatory. That makes it explicit that the CONNECT specific rule in HttpMethods.contentLengthAllowed is unreachable over HTTP/2. Result: The status based rules are covered independently of the request method, and the absence of CONNECT support is recorded rather than assumed. Tests: - sbt "http2-tests / test" - 352 passed, 25 ignored, 26 pending - scalafmt --mode diff-ref=upstream/main - clean - git diff --check - clean References: Refs apache#1236, Refs apache#962
pjfanning
force-pushed
the
http2-head-fixes
branch
from
August 30, 2026 11:04
48263cf to
bf74d55
Compare
nvollmar
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Builds on #1238. That PR added HTTP/2 HEAD coverage and pinned three defects with
FIXMEs; this one fixes them and flips those assertions. It contains #1238's commit, because the tests-only branch lives on a fork and cannot be used as a base here — review the second commit for the fix, or merge #1238 first and this rebases to just that commit.The three defects:
transparent-head-requestswas ignored over HTTP/2 — it is applied only byHttpServerBluePrint, the HTTP/1.1 blueprint.304(and a204) gotcontent-length: 0where HTTP/1.1 omits the header. RFC 9110 §15.4.5 expects a304to carry theContent-Lengtha200would have had, so a made-up zero is actively misleading.The hard part is (1):
ResponseRenderingonly ever sees anHttpResponseplus its stream-id attribute, so it cannot know the request method. Carrying it on an attribute alongsideHttp2.streamIdwould only fix thebindAPI —bindFlowusers copy that attribute by hand, andHttp2ServerSpec's own probes show what such a user looks like.Modification
Track the method where it is already known per connection, below the HTTP layer:
Http2StreamHandlingrecords the stream ids of incoming HEAD requests as the request HEADERS frame opens the stream (:methodis already parsed to anHttpMethodbyHeaderDecompression, andHttp2StreamHandlingis per-connection state, so no new plumbing is needed).handleOutgoingCreatedcancels the response data for those streams and sends the initial headers withendStreamset, via a newHttp2SubStream.withoutData. The header pairs are left untouched, so the peer still learns thecontent-lengthit would have received for a GET — the same split HTTP/1.1 makes between rendering the length and discarding the bytes.Closedtransition inupdateStateAndReturnand theonDownstreamFinishpath), so the set cannot outlive its streams.Because that tracking reads the method off the wire below the HTTP layer, it is unaffected by anything the HTTP layer does to the request — which makes (2) fall out cheaply:
RequestParsingnow rewrites HEAD to GET whentransparent-head-requestsis on, exactly asHttpServerBluePrint.scala:159does for HTTP/1.1, and the body is still stripped because the demux remembers the wire method.For (3),
ResponseRenderinggains the status-based part of the rules HTTP/1.1 applies throughHttpMethod.contentLengthAllowed, so 1xx, 204 and 304 no longer render acontent-length.Works for
bindFlowas well asbind, since nothing depends on the handler propagating an attribute.Result
A HEAD request over HTTP/2 gets headers only, carrying the
content-lengththe resource would have had, andtransparent-head-requestsbehaves as it does for HTTP/1.1.Two deliberate scope limits, both worth a follow-up rather than widening this PR:
Http2.scala:191upgrade path the request is parsed by the HTTP/1.1 parser and injected on stream 1, so it never passes a HEADERS frame through the demux and a HEAD there would still get a body. HEAD combined with an h2c upgrade is very unusual.content-length: 0over HTTP/2, where HTTP/1.1 omits it (see fix: render Content-Length for HEAD responses with a declared length #1237). Aligning that needs the request method at rendering time, which is the plumbing this PR deliberately avoids.Tests
The three
FIXMEtests from #1238 flip to the correct expectations, and three more are added — a streamed response entity (asserting the source is cancelled and no DATA is sent), transparent-head-requests translating to GET while still stripping the body, and a 204.sbt "http2-tests / test"— 349 passed, 25 ignored, 26 pending (ignored/pending are pre-existing)sbt +mimaReportBinaryIssues— successscalafmt --mode diff-ref=upstream/main— clean;git diff --check— cleanAll changed files are
private[http2]internals, so there is no API surface change.References
Refs #1236, Refs #1238
Update: coverage for the rest of PR #962
#962 introduced two sets of rules in
HttpMethod.contentLengthAllowed— a method-specific one (HEAD, CONNECT) and a status-based one that applies to every method. HTTP/2 had coverage for neither. A third commit fills that in:HeadRequestSetup, i.e. driven by a HEAD request — but the status rules don't depend on the method, so a regression on GET would have slipped through. They now live in their own"render content-length according to the response status"section driven by a plain GET, joined by 200 and 205.content-length: 0for it, matching HTTP/1.1 and matching what Ember server emits an unframed 205 Reset Content (no Content-Length, no Transfer-Encoding) http4s/http4s#7919 asks for — previously untested on this side.:schemeand:pathwhichRequestParsingtreats as mandatory (Malformed request: Mandatory pseudo-header ':scheme' missing). ARequestParsingSpeccase pins that, so the CONNECT-specific rule inHttpMethods.contentLengthAllowedis understood to be unreachable over HTTP/2 rather than assumed to apply.sbt "http2-tests / test"— 352 passed, 25 ignored, 26 pending.