wasi-http: Reject delete of forbidden headers (#7490)

* Don't delete forbidden headers

* Make delete return a result

* Fix test that wasn't updated for fallible delete changes

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

@@ -15,7 +15,22 @@ 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) {
+    fn handle(request: IncomingRequest, outparam: ResponseOutparam) {
+        let header = String::from("custom-forbidden-header");
+        let req_hdrs = request.headers();
+
+        assert!(
+            !req_hdrs.get(&header).is_empty(),
+            "missing `custom-forbidden-header` from request"
+        );
+
+        assert!(req_hdrs.delete(&header).is_err());
+
+        assert!(
+            !req_hdrs.get(&header).is_empty(),
+            "delete of forbidden header succeeded"
+        );
+
         let hdrs = bindings::wasi::http::types::Headers::new();
         let resp = bindings::wasi::http::types::OutgoingResponse::new(hdrs);
         let body = resp.body().expect("outgoing response");

crates/wasi-http/src/types_impl.rs

@@ -170,15 +170,23 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
         Ok(Ok(()))
     }
 
-    fn delete(&mut self, fields: Resource<HostFields>, name: String) -> wasmtime::Result<()> {
+    fn delete(
+        &mut self,
+        fields: Resource<HostFields>,
+        name: String,
+    ) -> wasmtime::Result<Result<(), types::HeaderError>> {
         let header = match hyper::header::HeaderName::from_bytes(name.as_bytes()) {
             Ok(header) => header,
-            Err(_) => return Ok(()),
+            Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)),
         };
 
+        if is_forbidden_header(self, &header) {
+            return Ok(Err(types::HeaderError::Forbidden));
+        }
+
         let m = get_fields_mut(self.table(), &fields)?;
         m.remove(header);
-        Ok(())
+        Ok(Ok(()))
     }
 
     fn append(

crates/wasi-http/tests/all/main.rs

@@ -199,11 +199,18 @@ async fn run_wasi_http(
 
 #[test_log::test(tokio::test)]
 async fn wasi_http_proxy_tests() -> anyhow::Result<()> {
-    let req = hyper::Request::builder()
-        .method(http::Method::GET)
-        .body(body::empty())?;
+    let mut req = hyper::Request::builder().method(http::Method::GET);
 
-    let resp = run_wasi_http(test_programs_artifacts::API_PROXY_COMPONENT, req, None).await?;
+    req.headers_mut()
+        .unwrap()
+        .append("custom-forbidden-header", "yes".parse().unwrap());
+
+    let resp = run_wasi_http(
+        test_programs_artifacts::API_PROXY_COMPONENT,
+        req.body(body::empty())?,
+        None,
+    )
+    .await?;
 
     match resp {
         Ok(resp) => println!("response: {resp:?}"),

crates/wasi-http/wit/deps/http/types.wit

@@ -88,8 +88,9 @@ interface types {
     set: func(name: field-key, value: list<field-value>) -> result<_, header-error>;
 
     /// Delete all values for a key. Does nothing if no values for the key
-    /// exist.
-    delete: func(name: field-key);
+    /// exist. Returns and error if the `field-key` is syntactically invalid, or
+    /// if the `field-key` is forbidden.
+    delete: func(name: field-key) -> result<_, header-error>;
 
     /// Append a value for a key. Does not change or delete any existing
     /// values for that key.
@@ -98,7 +99,6 @@ interface types {
     /// the name is forbidden.
     append: func(name: field-key, value: field-value) -> result<_, header-error>;
 
-
     /// Retrieve the full set of keys and values in the Fields. Like the
     /// constructor, the list represents each key-value pair. 
     ///

crates/wasi/wit/deps/http/types.wit

@@ -88,8 +88,9 @@ interface types {
     set: func(name: field-key, value: list<field-value>) -> result<_, header-error>;
 
     /// Delete all values for a key. Does nothing if no values for the key
-    /// exist.
-    delete: func(name: field-key);
+    /// exist. Returns and error if the `field-key` is syntactically invalid, or
+    /// if the `field-key` is forbidden.
+    delete: func(name: field-key) -> result<_, header-error>;
 
     /// Append a value for a key. Does not change or delete any existing
     /// values for that key.
@@ -98,7 +99,6 @@ interface types {
     /// the name is forbidden.
     append: func(name: field-key, value: field-value) -> result<_, header-error>;
 
-
     /// Retrieve the full set of keys and values in the Fields. Like the
     /// constructor, the list represents each key-value pair. 
     ///