Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/http/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,25 @@ pub(crate) fn header_map_from_wasi(wasi_fields: Fields) -> Result<HeaderMap, Err
pub(crate) fn header_map_to_wasi(header_map: &HeaderMap) -> Result<Fields, Error> {
let wasi_fields = Fields::new();
for (key, value) in header_map {
// Unwrap because `HeaderMap` has already validated the headers.
wasi_fields
.append(key.as_str(), value.as_bytes())
.with_context(|| format!("wasi rejected header `{key}: {value:?}`"))?
if !FORBIDDEN_HEADERS.contains(key) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

wasi_fields
.append(key.as_str(), value.as_bytes())
.with_context(|| format!("wasi rejected header `{key}: {value:?}`"))?
}
}
Ok(wasi_fields)
}

const FORBIDDEN_HEADERS: [HeaderName; 11] = [
http::header::CONNECTION,
HeaderName::from_static("keep-alive"),
http::header::PROXY_AUTHENTICATE,
http::header::PROXY_AUTHORIZATION,
HeaderName::from_static("proxy-connection"),
http::header::TRANSFER_ENCODING,
http::header::UPGRADE,
http::header::HOST,
HeaderName::from_static("http2-settings"),
http::header::EXPECT,
http::header::CONTENT_LENGTH,
];
9 changes: 0 additions & 9 deletions src/http/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
//! [`http_server`]: crate::http_server

use super::{Body, Error, Response, error::ErrorCode, fields::header_map_to_wasi};
use http::header::CONTENT_LENGTH;
use wasip2::exports::http::incoming_handler::ResponseOutparam;
use wasip2::http::types::OutgoingResponse;

Expand All @@ -44,14 +43,6 @@ impl Responder {
// Consume the `response` and prepare to write the body.
let body = response.into_body().into();

// 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();
}

Comment on lines -47 to -54

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

let wasi_response = OutgoingResponse::new(wasi_headers);

// Unwrap because `StatusCode` has already validated the status.
Expand Down
Loading