pjfanning opened a new pull request, #1237: URL: https://github.com/apache/pekko-http/pull/1237
### Motivation [PR #962](https://github.com/apache/pekko-http/pull/962) corrected `Content-Length` rendering for `205 Reset Content` and for `CONNECT` in the 2xx range. Those parts are right — [http4s/http4s#7919](https://github.com/http4s/http4s/issues/7919) cites pekko-http's 205 handling as the reference implementation. But the same PR also made `HEAD` responses drop `Content-Length` unconditionally, which is what #1236 reports. That part looks like collateral rather than intent: #962 deleted six existing assertions across four tests without adding any that argue for the new behaviour, including the test named *"to a HEAD request setting a custom Content-Type and **Content-Length** (default response entity)"*, which now asserted the opposite of its own name, and the round-trip assertions in `HostConnectionPoolSpec` (`shouldEqual 100` → `shouldEqual 0`). The suppression is not required by the spec, and it leaves the codebase inconsistent: - **RFC 9110 §8.6** says a server MAY send `Content-Length` in a HEAD response, and it SHOULD equal what a GET would return. - **RFC 9112 §6.3 rule 1** exempts responses to HEAD requests from body framing entirely, so the header there is pure metadata and cannot desync a persistent connection. - Our own **client** parser explicitly supports reading it — `HttpResponseParser.scala:186-195` turns HEAD + `Content-Length` into `HttpEntity.Default(ct, len, Source.empty)`. - **HTTP/2 was never affected**: `http2/HttpMessageRendering.addContentHeaders` emits `content-length` straight from `entity.contentLengthOption` and never consults `contentLengthAllowed`, so the same application returned the header over h2 but not over h1. - Even within HTTP/1.1, a HEAD + `Chunked` response still renders `Transfer-Encoding: chunked`, so framing metadata was mirrored for chunked but not for content length. The concern behind the original change is real, though: a handler that special-cases HEAD and returns a bare `HttpResponse()` would render `Content-Length: 0`, which misrepresents what GET would return. So this PR does not simply revert the HEAD part. ### Modification Two small edits: 1. `HttpMethods.contentLengthAllowedForHead` returns `forStatus.allowsEntity` instead of `false`, restoring the pre-1.4 predicate. A `Content-Length` *is* permitted on a HEAD response, so the public `HttpMethod.contentLengthAllowed` field no longer claims otherwise. 2. `HttpResponseRendererFactory.renderContentLengthHeader` adds the length-based policy: for HEAD, render only when the declared length is greater than zero. That threshold separates "the application told us a length" from "we derived a length from a body we are about to discard": | HEAD response entity | length | rendered? | |---|---|---| | `Strict(ct, 23 bytes)` | 23 | ✅ real length, body still dropped | | `Default(ct, 100, Source.empty)` | 100 | ✅ the object-store / S3 case from #1236 | | `Strict(ct, empty)` | 0 | ❌ means "no body here", not "0 bytes" | | `CloseDelimited` / `Chunked` | — | ❌ unchanged; no known length | It also matches the client exactly: `HttpResponseParser` honours a HEAD `Content-Length` only `if contentLength > 0`. Server and client now emit and accept the same set. 205, 204, 304 and CONNECT behaviour from #962 is untouched — `contentLengthAllowed(status)` still governs, and 205 keeps its `Content-Length: 0` because it is not a HEAD response. No public signature changes; `sbt +mimaReportBinaryIssues` is clean. ### Result A HEAD response completed with `HttpEntity.Strict` or `HttpEntity.Default(contentType, length, Source.empty)` renders the declared `Content-Length` again, as it did before 1.4.0. A HEAD response with an empty entity renders none. ### Tests - `sbt "http-core / Test / testOnly org.apache.pekko.http.impl.engine.rendering.ResponseRendererSpec org.apache.pekko.http.impl.engine.server.HttpServerSpec org.apache.pekko.http.scaladsl.model.HttpMethodsSpec"` — 110 passed - `sbt "http-core / Test / testOnly org.apache.pekko.http.impl.engine.client.HostConnectionPoolSpec"` — 66 passed - `sbt +mimaReportBinaryIssues` — success - `scalafmt --mode diff-ref=upstream/main` — clean; `git diff --check` — clean - `sbt "http-core / test"` and `sbt "docs / paradox"` — not run to completion locally, left to CI Directional coverage: the restored assertions in `ResponseRendererSpec` (`Content-Length: 23` and `100`), `HttpServerSpec` (`Content-Length: 4`, both the Strict and Default transparent-HEAD cases) and `HostConnectionPoolSpec` (server-renders → client-parses round trip, back to `100`) all fail without the fix. A new `ResponseRendererSpec` case, *"to a transparent HEAD request (empty Strict response entity)"*, guards the other direction by asserting that an empty entity renders no `Content-Length`. ### References Fixes #1236, Refs #962, Refs http4s/http4s#7919 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
