wasi-http: Make child fields immutable (bytecodealliance#7524)

* Make child fields immutable * Add `get_fields` and remove `FieldMapMutability` Clean up the interface to immutable fields by adding a different accessor. * Clean up the diff
alexcrichton · Nov 12, 2023 · c11bc11 · c11bc11
1 parent e6301d4
commit c11bc11
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 29 deletions.
diff --git a/crates/test-programs/src/bin/http_outbound_request_invalid_header.rs b/crates/test-programs/src/bin/http_outbound_request_invalid_header.rs
@@ -1,4 +1,4 @@
-use test_programs::wasi::http::types::{HeaderError, Headers};
+use test_programs::wasi::http::types::{HeaderError, Headers, OutgoingRequest};
 
 fn main() {
     let hdrs = Headers::new();
@@ -57,4 +57,22 @@ fn main() {
         Headers::from_list(&[("ok-header-name".to_owned(), b"bad\nvalue".to_vec())]),
         Err(HeaderError::InvalidSyntax)
     ));
+
+    let req = OutgoingRequest::new(hdrs);
+    let hdrs = req.headers();
+
+    assert!(matches!(
+        hdrs.set(&"Content-Length".to_owned(), &[b"10".to_vec()]),
+        Err(HeaderError::Immutable),
+    ));
+
+    assert!(matches!(
+        hdrs.append(&"Content-Length".to_owned(), &b"10".to_vec()),
+        Err(HeaderError::Immutable),
+    ));
+
+    assert!(matches!(
+        hdrs.delete(&"Content-Length".to_owned()),
+        Err(HeaderError::Immutable),
+    ));
 }
diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs
@@ -32,10 +32,10 @@ fn move_fields(table: &mut Table, id: Resource<HostFields>) -> wasmtime::Result<
     }
 }
 
-fn get_fields_mut<'a>(
+fn get_fields<'a>(
     table: &'a mut Table,
     id: &Resource<HostFields>,
-) -> wasmtime::Result<&'a mut FieldMap> {
+) -> wasmtime::Result<&'a FieldMap> {
     let fields = table.get(&id)?;
     if let HostFields::Ref { parent, get_fields } = *fields {
         let entry = table.get_any_mut(parent)?;
@@ -51,6 +51,16 @@ fn get_fields_mut<'a>(
     }
 }
 
+fn get_fields_mut<'a>(
+    table: &'a mut Table,
+    id: &Resource<HostFields>,
+) -> wasmtime::Result<Result<&'a mut FieldMap, types::HeaderError>> {
+    match table.get_mut(&id)? {
+        HostFields::Owned { fields } => Ok(Ok(fields)),
+        HostFields::Ref { .. } => Ok(Err(types::HeaderError::Immutable)),
+    }
+}
+
 fn is_forbidden_header<T: WasiHttpView>(view: &mut T, name: &HeaderName) -> bool {
     static FORBIDDEN_HEADERS: [HeaderName; 9] = [
         hyper::header::CONNECTION,
@@ -83,7 +93,7 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
         &mut self,
         entries: Vec<(String, Vec<u8>)>,
     ) -> wasmtime::Result<Result<Resource<HostFields>, types::HeaderError>> {
-        let mut map = hyper::HeaderMap::new();
+        let mut fields = hyper::HeaderMap::new();
 
         for (header, value) in entries {
             let header = match hyper::header::HeaderName::from_bytes(header.as_bytes()) {
@@ -100,12 +110,12 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
                 Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)),
             };
 
-            map.append(header, value);
+            fields.append(header, value);
         }
 
         let id = self
             .table()
-            .push(HostFields::Owned { fields: map })
+            .push(HostFields::Owned { fields })
             .context("[new_fields] pushing fields")?;
 
         Ok(Ok(id))
@@ -128,7 +138,7 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
             Err(_) => return Ok(vec![]),
         };
 
-        let res = get_fields_mut(self.table(), &fields)
+        let res = get_fields(self.table(), &fields)
             .context("[fields_get] getting fields")?
             .get_all(header)
             .into_iter()
@@ -160,14 +170,14 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
             }
         }
 
-        let m =
-            get_fields_mut(self.table(), &fields).context("[fields_set] getting mutable fields")?;
-        m.remove(&header);
-        for value in values {
-            m.append(&header, value);
-        }
-
-        Ok(Ok(()))
+        Ok(get_fields_mut(self.table(), &fields)
+            .context("[fields_set] getting mutable fields")?
+            .map(|fields| {
+                fields.remove(&header);
+                for value in values {
+                    fields.append(&header, value);
+                }
+            }))
     }
 
     fn delete(
@@ -184,9 +194,9 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
             return Ok(Err(types::HeaderError::Forbidden));
         }
 
-        let m = get_fields_mut(self.table(), &fields)?;
-        m.remove(header);
-        Ok(Ok(()))
+        Ok(get_fields_mut(self.table(), &fields)?.map(|fields| {
+            fields.remove(header);
+        }))
     }
 
     fn append(
@@ -209,27 +219,25 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostFields for T {
             Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)),
         };
 
-        let m = get_fields_mut(self.table(), &fields)
-            .context("[fields_append] getting mutable fields")?;
-
-        m.append(header, value);
-        Ok(Ok(()))
+        Ok(get_fields_mut(self.table(), &fields)
+            .context("[fields_append] getting mutable fields")?
+            .map(|fields| {
+                fields.append(header, value);
+            }))
     }
 
     fn entries(
         &mut self,
         fields: Resource<HostFields>,
     ) -> wasmtime::Result<Vec<(String, Vec<u8>)>> {
-        let fields = get_fields_mut(self.table(), &fields)?;
-        let result = fields
+        Ok(get_fields(self.table(), &fields)?
             .iter()
             .map(|(name, value)| (name.as_str().to_owned(), value.as_bytes().to_owned()))
-            .collect();
-        Ok(result)
+            .collect())
     }
 
     fn clone(&mut self, fields: Resource<HostFields>) -> wasmtime::Result<Resource<HostFields>> {
-        let fields = get_fields_mut(self.table(), &fields)
+        let fields = get_fields(self.table(), &fields)
             .context("[fields_clone] getting fields")?
             .clone();
 
@@ -837,7 +845,7 @@ impl<T: WasiHttpView> crate::bindings::http::types::HostOutgoingBody for T {
             .expect("outgoing-body trailer_sender consumed by a non-owning function");
 
         let message = if let Some(ts) = ts {
-            FinishMessage::Trailers(get_fields_mut(self.table(), &ts)?.clone().into())
+            FinishMessage::Trailers(move_fields(self.table(), ts)?)
         } else {
             FinishMessage::Finished
         };

diff --git a/crates/wasi-http/wit/deps/http/types.wit b/crates/wasi-http/wit/deps/http/types.wit
@@ -42,6 +42,7 @@ interface types {
   variant header-error {
     invalid-syntax,
     forbidden,
+    immutable
   }
 
   /// Field keys are always strings.

diff --git a/crates/wasi/wit/deps/http/types.wit b/crates/wasi/wit/deps/http/types.wit
@@ -42,6 +42,7 @@ interface types {
   variant header-error {
     invalid-syntax,
     forbidden,
+    immutable
   }
 
   /// Field keys are always strings.