Move forbidden header validation to send, validate syntax in set/append

Author: elliottt

crates/test-programs/src/bin/api_proxy.rs

@@ -10,17 +10,18 @@ pub mod bindings {
     });
 }
 
-use bindings::wasi::http::types::{FieldsType, IncomingRequest, ResponseOutparam};
+use bindings::wasi::http::types::{IncomingRequest, ResponseOutparam};
 
 struct T;
 
 impl bindings::exports::wasi::http::incoming_handler::Guest for T {
     fn handle(_request: IncomingRequest, outparam: ResponseOutparam) {
-        let hdrs = bindings::wasi::http::types::Headers::new(FieldsType::Response, &[]);
+        let hdrs = bindings::wasi::http::types::Headers::new(&[]);
         let resp = bindings::wasi::http::types::OutgoingResponse::new(200, &hdrs);
         let body = resp.write().expect("outgoing response");
 
-        bindings::wasi::http::types::ResponseOutparam::set(outparam, Ok(resp));
+        bindings::wasi::http::types::ResponseOutparam::set(outparam, Ok(resp))
+            .expect("sending the response");
 
         let out = body.write().expect("outgoing stream");
         out.blocking_write_and_flush(b"hello, world!")

crates/test-programs/src/bin/api_proxy_streaming.rs

@@ -1,6 +1,6 @@
 use anyhow::{bail, Result};
 use bindings::wasi::http::types::{
-    Fields, FieldsType, IncomingRequest, Method, OutgoingBody, OutgoingRequest, OutgoingResponse,
+    Fields, IncomingRequest, Method, OutgoingBody, OutgoingRequest, OutgoingResponse,
     ResponseOutparam, Scheme,
 };
 use futures::{stream, SinkExt, StreamExt, TryStreamExt};
@@ -51,16 +51,13 @@ async fn handle_request(request: IncomingRequest, response_out: ResponseOutparam
 
             let response = OutgoingResponse::new(
                 200,
-                &Fields::new(
-                    FieldsType::Response,
-                    &[("content-type".to_string(), b"text/plain".to_vec())],
-                ),
+                &Fields::new(&[("content-type".to_string(), b"text/plain".to_vec())]),
             );
 
             let mut body =
                 executor::outgoing_body(response.write().expect("response should be writable"));
 
-            ResponseOutparam::set(response_out, Ok(response));
+            ResponseOutparam::set(response_out, Ok(response)).expect("sending the response");
 
             while let Some((url, result)) = results.next().await {
                 let payload = match result {
@@ -79,7 +76,6 @@ async fn handle_request(request: IncomingRequest, response_out: ResponseOutparam
             let response = OutgoingResponse::new(
                 200,
                 &Fields::new(
-                    FieldsType::Response,
                     &headers
                         .into_iter()
                         .filter_map(|(k, v)| (k == "content-type").then_some((k, v)))
@@ -90,7 +86,7 @@ async fn handle_request(request: IncomingRequest, response_out: ResponseOutparam
             let mut body =
                 executor::outgoing_body(response.write().expect("response should be writable"));
 
-            ResponseOutparam::set(response_out, Ok(response));
+            ResponseOutparam::set(response_out, Ok(response)).expect("sending the response");
 
             let mut stream =
                 executor::incoming_body(request.consume().expect("request should be readable"));
@@ -112,11 +108,11 @@ async fn handle_request(request: IncomingRequest, response_out: ResponseOutparam
         }
 
         _ => {
-            let response = OutgoingResponse::new(405, &Fields::new(FieldsType::Response, &[]));
+            let response = OutgoingResponse::new(405, &Fields::new(&[]));
 
             let body = response.write().expect("response should be writable");
 
-            ResponseOutparam::set(response_out, Ok(response));
+            ResponseOutparam::set(response_out, Ok(response)).expect("sending the response");
 
             OutgoingBody::finish(body, None);
         }
@@ -141,7 +137,7 @@ async fn hash(url: &Url) -> Result<String> {
                 String::new()
             }
         )),
-        &Fields::new(FieldsType::Request, &[]),
+        &Fields::new(&[]),
     );
 
     let response = executor::outgoing_request_send(request).await?;

crates/test-programs/src/bin/http_outbound_request_invalid_header.rs

@@ -1,12 +1,24 @@
-use test_programs::wasi::http::types::{FieldsType, Headers};
+use test_programs::wasi::http::types::{Headers, Method, Scheme};
 
 fn main() {
-    let hdrs = Headers::new(FieldsType::Request, &[]);
+    let hdrs = Headers::new(&[]);
     assert!(hdrs
-        .set(&"transfer-encoding".to_owned(), &[b"bad".to_vec()])
+        .append(&"malformed header name".to_owned(), &b"bad".to_vec())
         .is_err());
 
-    assert!(hdrs
-        .append(&"tRaNsFeR-eNcOdInG".to_owned(), &b"bad".to_vec())
-        .is_err());
+    let addr = std::env::var("HTTP_SERVER").unwrap();
+    let err = test_programs::http::request(
+        Method::Get,
+        Scheme::Http,
+        &addr,
+        "/get?some=arg&goes=here",
+        None,
+        Some(&[("transfer-encoding".to_owned(), b"bad".to_vec())]),
+    )
+    .expect_err("invalid request");
+
+    assert_eq!(
+        err.to_string(),
+        "Error::HeaderNameError(\"forbidden header\")"
+    );
 }

crates/test-programs/src/bin/http_outbound_request_response_build.rs

@@ -3,13 +3,10 @@ use test_programs::wasi::http::types as http_types;
 fn main() {
     println!("Called _start");
     {
-        let headers = http_types::Headers::new(
-            http_types::FieldsType::Request,
-            &[(
-                "Content-Type".to_string(),
-                "application/json".to_string().into_bytes(),
-            )],
-        );
+        let headers = http_types::Headers::new(&[(
+            "Content-Type".to_string(),
+            "application/json".to_string().into_bytes(),
+        )]);
         let request = http_types::OutgoingRequest::new(
             &http_types::Method::Get,
             None,
@@ -24,13 +21,10 @@ fn main() {
             .unwrap();
     }
     {
-        let headers = http_types::Headers::new(
-            http_types::FieldsType::Response,
-            &[(
-                "Content-Type".to_string(),
-                "application/text".to_string().into_bytes(),
-            )],
-        );
+        let headers = http_types::Headers::new(&[(
+            "Content-Type".to_string(),
+            "application/text".to_string().into_bytes(),
+        )]);
         let response = http_types::OutgoingResponse::new(200, &headers);
         let outgoing_body = response.write().unwrap();
         let response_body = outgoing_body.write().unwrap();

crates/test-programs/src/http.rs

@@ -43,7 +43,6 @@ pub fn request(
         v.to_string().into_bytes()
     }
     let headers = http_types::Headers::new(
-        http_types::FieldsType::Request,
         &[
             &[
                 ("User-agent".to_string(), header_val("WASI-HTTP/0.0.1")),

crates/wasi-http/src/body.rs

@@ -19,7 +19,6 @@ pub type HyperIncomingBody = BoxBody<Bytes, anyhow::Error>;
 /// one. The HostIncomingBody spawns a task that starts consuming the incoming body, and we don't
 /// want to do that unless the user asks to consume the body.
 pub struct HostIncomingBodyBuilder {
-    pub ty: types::FieldsType,
     pub body: HyperIncomingBody,
     pub between_bytes_timeout: Duration,
 }
@@ -104,7 +103,6 @@ impl HostIncomingBodyBuilder {
         });
 
         HostIncomingBody {
-            ty: self.ty,
             worker,
             stream: Some(HostIncomingBodyStream::new(body_receiver)),
             trailers,
@@ -113,7 +111,6 @@ impl HostIncomingBodyBuilder {
 }
 
 pub struct HostIncomingBody {
-    ty: types::FieldsType,
     pub worker: AbortOnDropJoinHandle<()>,
     pub stream: Option<HostIncomingBodyStream>,
     pub trailers: oneshot::Receiver<Result<hyper::HeaderMap, anyhow::Error>>,
@@ -123,7 +120,6 @@ impl HostIncomingBody {
     pub fn into_future_trailers(self) -> HostFutureTrailers {
         HostFutureTrailers {
             _worker: self.worker,
-            ty: self.ty,
             state: HostFutureTrailersState::Waiting(self.trailers),
         }
     }
@@ -220,7 +216,6 @@ impl Subscribe for HostIncomingBodyStream {
 
 pub struct HostFutureTrailers {
     _worker: AbortOnDropJoinHandle<()>,
-    pub ty: types::FieldsType,
     pub state: HostFutureTrailersState,
 }
 

crates/wasi-http/src/http_impl.rs

@@ -1,6 +1,6 @@
 use crate::bindings::http::{
     outgoing_handler,
-    types::{RequestOptions, Scheme},
+    types::{self as http_types, RequestOptions, Scheme},
 };
 use crate::types::{self, HostFutureIncomingResponse, OutgoingRequest};
 use crate::WasiHttpView;
@@ -77,6 +77,12 @@ impl<T: WasiHttpView> outgoing_handler::Host for T {
             .header(hyper::header::HOST, &authority);
 
         for (k, v) in req.headers.iter() {
+            if self.is_forbidden_request_header(k) {
+                return Ok(Err(http_types::Error::HeaderNameError(
+                    "forbidden header".to_owned(),
+                )));
+            }
+
             builder = builder.header(k, v);
         }
 

crates/wasi-http/src/types.rs

@@ -37,7 +37,6 @@ pub trait WasiHttpView: Send {
     ) -> wasmtime::Result<Resource<HostIncomingRequest>> {
         let (parts, body) = req.into_parts();
         let body = HostIncomingBodyBuilder {
-            ty: types::FieldsType::Request,
             body,
             // TODO: this needs to be plumbed through
             between_bytes_timeout: std::time::Duration::from_millis(600 * 1000),
@@ -284,11 +283,9 @@ pub enum HostFields {
         // always be registered as a child of the entry with the `parent` id. This ensures that the
         // entry will always exist while this `HostFields::Ref` entry exists in the table, thus we
         // don't need to account for failure when fetching the fields ref from the parent.
-        get_fields:
-            for<'a> fn(elem: &'a mut (dyn Any + 'static)) -> (types::FieldsType, &'a mut FieldMap),
+        get_fields: for<'a> fn(elem: &'a mut (dyn Any + 'static)) -> &'a mut FieldMap,
     },
     Owned {
-        ty: types::FieldsType,
         fields: FieldMap,
     },
 }