Add a HeaderMap to HttpError

[hawkw]

There are certain response headers which the HTTP standard specifies
that servers should send along with particular error responses or status
codes. For example, when returning a 405 Method Not Allowed status,
§ 15.5.0 RFC9110 states that:

The origin server MUST generate an Allow header field in a 405
response containing a list of the target resource's currently
supported methods.

Currently, Dropshot does not do this, so we are technically not
compliant with RFC9110.

Similarly, some of Dropshot's extractors return error responses for
which we ought to emit headers. For example, the TypedBody extractor
will return a 415 Unsupported Media Type status, for which § 15.5.16
RFC9110 recommends returning an Accept or Accept-Encoding header
as appropriate:

If the problem was caused by an unsupported content coding, the
Accept-Encoding response header field (Section 12.5.3) ought to be
used to indicate which (if any) content codings would have been
accepted in the request.

On the other hand, if the cause was an unsupported media type, the
Accept response header field (Section 12.5.1) can be used to indicate
which media types would have been accepted in the request.

Note that, unlike the Allow header for 405 responses, this is just a
suggestion --- the RFC doesn't include normative language stating that
we MUST do this. But it's nice if we do.

Errors emitted internally by Dropshot are represented by the HttpError
type. Currently, there is no way for this type to provide headers which
should be added to responses generated from an HttpError. Therefore,
this branch does so by adding a http::HeaderMap to HttpError. When
the HttpError is converted into a response, any headers in the
HeaderMap are added to the response. Adding this field is a breaking
change, so I've added it to the changelog.

Because the http::HeaderMap type is fairly large even when empty, the
HeaderMap is stored in an Option<Box<HeaderMap>> to reduce the size
of HttpErrors, especially in the common case where there is no header
map to add. Unlike containers such as Vec and HashMap, an empty
http::HeaderMap contains a fairly large number of fields, rather than
being a single pointer to a heap allocation containing the actual data.
Thus, we store an Option<Box<HeaderMap>> here, so that the empty case
is just a null pointer. Not doing this results in a clippy warning
on...every function returning a Result<_, HttpError>, which is
unfortunate because even if we ignored that lint in Dropshot, it would
still trigger for consumers of Dropshot's APIs.

As an initial proof of concept, I've added code to dropshot's router
module to add Allow headers with the list of allowed methods when
returning a 405 Method Not Allowed error. There are other places where
we should be returning headers in our error responses, but this felt
like one of the more important ones, since the RFC says we MUST do it.
We can add headers to other errors separately; my priority was to make
the breaking parts of the change first.

For example, note that we now return Allow headers in responses to
requests with invalid methods:

eliza@noctis ~/Code/dropshot $ cargo run --example basic &
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/examples/basic`
Dec 04 18:51:47.826 INFO listening, local_addr: 127.0.0.1:41751

eliza@noctis ~/Code/dropshot $ curl -v http://localhost:41751/counter -X DELETE
* Host localhost:41751 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:41751...
* connect to ::1 port 41751 from ::1 port 53992 failed: Connection refused
*   Trying 127.0.0.1:41751...
* Connected to localhost (127.0.0.1) port 41751
* using HTTP/1.x
> DELETE /counter HTTP/1.1
> Host: localhost:41751
> User-Agent: curl/8.11.0
> Accept: */*
>
* Request completely sent off
Dec 04 18:59:14.913 INFO accepted connection, remote_addr: 127.0.0.1:35084, local_addr: 127.0.0.1:41751
Dec 04 18:59:14.917 INFO request completed, error_message_external: Method Not Allowed, error_message_internal: Method Not Allowed, latency_us: 834, response_code: 405, uri: /counter, method: DELETE, req_id: e1d14d57-ca67-49cc-84b0-d1dc1e0e5793, remote_addr: 127.0.0.1:35084, local_addr: 127.0.0.1:41751
< HTTP/1.1 405 Method Not Allowed
< allow: GET
< allow: PUT
< content-type: application/json
< x-request-id: e1d14d57-ca67-49cc-84b0-d1dc1e0e5793
< content-length: 93
< date: Wed, 04 Dec 2024 18:59:14 GMT
<
{
  "request_id": "e1d14d57-ca67-49cc-84b0-d1dc1e0e5793",
  "message": "Method Not Allowed"
* Connection #0 to host localhost left intact
}%

[hawkw]

Boxing this resolves a Clippy lint about the error type being too large. I figured we shouldn't just disable that lint, since it would presumably also trigger on user code that returns an HttpError, and it seems sad to have an API that results in lints in consumers if they don't go out of their way to disable them.

I also considered making the field private, and requiring it be accessed via methods, since the Option<Box<...>> makes it a bit awkward. However, that would prevent consumers of the API from manually constructing HttpErrors by initializing the other fields, which I've seen a lot of in Omicron, so I figured it was better to just make it public.

[hawkw]

This is a bit awkward. Another option could be to make the method take HeaderName and HeaderValue only, instead of types that can be converted into them; then, it could be infallible, instead of potentially eating the HttpError.

I did it this way because it's similar to the http::Response::builder() API, and allows users to just pass strings as the header name/value, rather than having to convert them in user code. But, returning a Result and maybe throwing away the error is also unfortunate. I could be convinced to change this --- what do y'all think?