http: do not send a body in response to HEAD requests #36354

HTTPRequest::WriteReply() sends the reply body regardless of the request method, so a HEAD request is answered with the same body as the equivalent GET. RFC 9110 section 9.3.2 requires the HEAD response to carry the header section of the GET response (so Content-Length describes the body GET would have returned) but no body.

$ printf 'HEAD / HTTP/1.1\r\nHost: localhost\r\n\r\n' | nc 127.0.0.1 18443
HTTP/1.1 405 Method Not Allowed
Date: ...
Content-Length: 41
Content-Type: text/html; charset=ISO-8859-1

JSONRPC server handles only POST requests

A client that follows the specification stops reading after the header section, so the body bytes stay on the connection and are read as the start of the next response. On a direct connection this only confuses the client that sent the HEAD. With a reverse proxy that keeps upstream connections alive (for example nginx with proxy_http_version 1.1 and an upstream { keepalive N; } block) the stray bytes end up in the proxy's connection pool and corrupt the next response to whichever client is served from that connection. Both the 405 reply of the RPC endpoint and the REST endpoints answer HEAD without authentication, so a client that is merely allowed to connect can trigger it. The body is not attacker controlled, so this is a robustness issue rather than response injection.

This is not a regression from #35182: the libevent based server behaved the same way. evhttp_send_reply() writes the output buffer regardless of the method, and evhttp_response_needs_body() only suppresses the Content-Length and Content-Type headers for HEAD (so the old server sent the body without a Content-Length). The needs_body flag in WriteReply() was ported with that same role, which is why this PR leaves it alone and only drops the body: the header section stays identical to the GET response, as the RFC recommends.

The functional tests cover the RPC endpoint (405 path) and a REST endpoint (200 path). They send the HEAD request over a raw socket and read until the server goes quiet, checking that Content-Length equals the GET body length and that no bytes follow the header section. http.client cannot be used for this check because it silently discards a wrongly attached body.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36354.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Do you think there is any real benefit to even supporting HEAD? It might make sense for a webserver but I think we can justify not implementing it at all. I think the METHOD NOT ALLOWED response from the RPC server is correct and in fact, we should probably add a similar check limiting the REST server only to GET requests.

Thanks for taking a look!

I agree we don't need HEAD as a feature, and I have no strong opinion on whether REST should accept it. But I think this change is needed either way, because rejecting HEAD doesn't make the problem go away: a response to a HEAD request can't have a body regardless of its status code. RFC 9112 section 6.3 (rule 1):

Any response to a HEAD request [...] is always terminated by the first empty line after the header fields, regardless of the header fields present in the message, and thus cannot contain a message body or trailer section.

So the 405 from the RPC endpoint is the right answer, but today it is sent with the 41-byte JSONRPC server handles only POST requests body, which a client or proxy will not read and which is then left on the connection. Dropping the body in WriteReply() fixes that for every handler and every error path at once, including a 405 from REST if we add one.

Restricting REST to GET sounds reasonable to me as a separate change. One thing to keep in mind is that /rest/getutxos reads the request body and interface_rest.py calls it with POST, so it would have to be GET plus POST for that endpoint. I'm happy to open a follow-up for that if you think it's worth doing, and to adjust the REST test here accordingly (it currently expects 200 for HEAD /rest/chaininfo.json).

Closing this for now -- I would be more in favor of rejecting unwanted methods from HTTP endpoints rather than implementing more of the HTTP spec.