Repository navigation
wasi-http: validate the Host header in one place - #14988
Conversation
pchickey
left a comment
pchickey
left a comment
There was a problem hiding this comment.
So, the RFC gives a couple of conditions here and we are detecting them piecemeal. To me, this means its time to create a single helper function get_request_target(&Request) -> Result<Authority>, which has rustdocs saying it enforces the full set of conditions in https://www.rfc-editor.org/info/rfc9112/#section-3.2
A server MUST respond with a 400 (Bad Request) status code to any HTTP/1.1 request message that lacks a Host header field and to any request message that contains more than one Host header field line or a Host header field with an invalid field value.
And some of that can be de-duplicated out of the p3 branch of code into that auxillary function. In the p2 branch it can just let _ = authority; so it builds without warnings when p3 disabled.
3f2b8e6 to
8177934
Compare
xia-chao
changed the title
| // The p2 path builds its own copy of the request, so only the | ||
| // request-target checks of the helper are used here. | ||
| let _ = match get_request_target(&request) { | ||
| Ok(authority) => authority, | ||
| Err(e) => { | ||
| _ = tx.send(Err(e)); | ||
| wasmtime::bail!( | ||
| "request was rejected before it could be turned into a guest request" | ||
| ); | ||
| } |
There was a problem hiding this comment.
can't be just let _ = authority; -- for the 400s the helper returns Err and it has to go through tx or serve sends 500.
pchickey
left a comment
pchickey
left a comment
There was a problem hiding this comment.
Thanks, this restructure helps a lot, I have just a couple more notes for how we should test this to make it super clear we covered all the bases
1339e51 to
758404a
Compare
758404a to
5c22612
Compare
|
Rebased onto the merged #14908: HTTP/2 requests carry the authority in |
5c22612 to
9d7956d
Compare
RFC 9112 section 3.2 has three conditions for the request target. Enforce them in get_request_target and use it from the p2 and p3 request paths.
9d7956d to
2e2038e
Compare
pchickey
left a comment
pchickey
left a comment
There was a problem hiding this comment.
Thanks for seeing this through
serve now derives and validates the request authority in one helper, so a missing, duplicated, malformed, or userinfo-bearing Host header is rejected with 400, and HTTP/2 :authority is checked against Host.
HTTP/2 requests usually carry no Host header, so one is only required for HTTP/1.1, and the two values must agree when both are present.