wasi-http: don't send forbidden headers#136
Conversation
the set of FORBIDDEN_HEADERS is derived from wasmtime, with the addition of Content-Length, and Expect.
I'm surprised to hear you say that; years ago I created a table indexing all HTTP headers, and I definitely sourced the 
I saw for example the |
|
Oh hah, the algorithm is documented here: https://fetch.spec.whatwg.org/#forbidden-request-header. That was easier to find than expected. I think we should probably just adopt this? |
|
By the way, we might even want to upstream this to |
|
Yes, whatwg has published that spec of forbidden headers. The trouble with adopting that list wholesale is that several of those restrictions are Web-platform specific and would be harmful to forbid in a proxy setting, specifically I don't really know one way or another whether the CORS Access-Control- headers, or the Proxy- and Sec-, should be forbidden or not. We also need a way to set |
| // Automatically add a Content-Length header. | ||
| if let Some(len) = body.content_length() { | ||
| let mut buffer = itoa::Buffer::new(); | ||
| wasi_headers | ||
| .append(CONTENT_LENGTH.as_str(), buffer.format(len).as_bytes()) | ||
| .unwrap(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm surprised this works? I'm looking at Jco's http.ts here, and this might cause problems for it? I'd feel a lot better if we had a test for this in wasi-testsuite to greenlight the behavior before we land it here.
|
@pchickey Oh heh, I forgot that this was specifically for the proxy worlds. Since we renamed the world from What I'd like for this PR is for us to document somewhere what we are deciding to do, and why. Ideally that would be upstream in |
| wasi_fields | ||
| .append(key.as_str(), value.as_bytes()) | ||
| .with_context(|| format!("wasi rejected header `{key}: {value:?}`"))? | ||
| if !FORBIDDEN_HEADERS.contains(key) { |
There was a problem hiding this comment.
while technically HTTP headers are case sensitive, ones like Content-Length are not usually handled that way.
I recommend normalizing every entry in FORBIDDEN_HEADERS to be lowercase and also to use key.to_ascii_lowercase() here in this contains call as well.
3 participants
Everyone agrees wasi shouldnt use forbidden headers, but nobody agrees on what they are (yet) WebAssembly/WASI#940.
This PR takes the default wasmtime set, and adds Content-Length and Expect as forbidden headers. Because
HeaderMapdoesn't provide us a mechanism to forbid headers, we instead just silently drop any of the known forbidden headers when translating to a wasifieldsresource.The set of forbidden headers is:
Since this is a pretty significant behavioral change, we will bump the version to 0.7.0 for the next release after this lands.