pjfanning opened a new pull request, #1238:
URL: https://github.com/apache/pekko-http/pull/1238
### Motivation
While looking at #1236 I noticed the HTTP/2 engine has no test coverage for
`HEAD` at all — and no `HEAD` handling either. `grep HEAD` over
`http-core/.../impl/engine/http2` and `http2-tests` returns only HEADERS-frame
matches. That absence was hiding three divergences from HTTP/1.1.
### Modification
Adds a `"support HEAD requests"` section to `Http2ServerSpec` covering
request delivery, `content-length` rendering for strict and known-length
entities, the response body, `transparent-head-requests`, and a `304` response.
Two tests assert behaviour that is correct today:
| | HTTP/1.1 | HTTP/2 |
|---|---|---|
| request delivered as `HEAD` with an empty entity | ✅ | ✅ |
| `content-length` on a strict `200` | ✅ | ✅ |
| `content-length` on `Default(ct, 100, Source.empty)` | ✅ (with #1237) | ✅ |
Three pin behaviour that is currently **wrong**, each with a `FIXME` naming
the cause and the relevant RFC, so a fix has to flip the assertion deliberately
rather than silently:
1. **The response entity is sent as DATA frames.** RFC 9110 §9.3.2: *"The
server MUST NOT send content in a response to a HEAD request."* Verified on the
wire — a `HEAD` answered with `HttpEntity(ct, ByteString("abcde"))` produces
`DATA endStream=true data=abcde`. `ResponseRendering` only ever sees the
`HttpResponse` plus its stream-id attribute, so the engine has no way to know
the request was a HEAD. HTTP/1.1 strips the body in
`HttpResponseRendererFactory` via `noEntity`.
2. **`transparent-head-requests` is ignored.** It is applied only at
`HttpServerBluePrint.scala:159`, which is the HTTP/1.1 blueprint, so over
HTTP/2 the handler always sees `HEAD` even with the setting on.
3. **A `304` gets `content-length: 0`.**
`HttpMessageRendering.addContentHeaders` renders `content-length` straight from
`entity.contentLengthOption` and never consults
`HttpMethod.contentLengthAllowed`, so HTTP/2 emits `content-length: 0` where
HTTP/1.1 deliberately omits the header (RFC 9110 §15.4.5: a `304` should carry
the `Content-Length` a `200` would have had, and `0` actively misstates that).
This PR is coverage only — no engine changes. Happy to open issues for the
three gaps and follow up with fixes; item 1 is a spec violation and looks worth
fixing on its own.
### Result
`HEAD` over HTTP/2 is covered, and the three gaps are recorded as assertions
instead of being absent.
### Tests
- `sbt "http2-tests / Test / testOnly
org.apache.pekko.http.impl.engine.http2.Http2ServerSpec"` — 118 passed, 16
pending (pre-existing)
- `scalafmt --mode diff-ref=upstream/main` — clean; `git diff --check` —
clean
Test-only change; no production code touched, so MiMa is unaffected.
### References
Refs #1236, Refs #1237
--
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]
