Repository: bytecodealliance/wasmtime
Pull request #7524 (2023-11-10 18:18:00)

wasi-http: Make child fields immutable
This PR adds a new case to the `header-error` variant, `immutable`, which indicates that an operation that would modify the `fields` value has failed because that `fields` is immutable. All `fields` returned by getter methods on any of the request/response types are considered immutable, while `fields` that have been created directly through the use of the `fields` constructor are considered mutable until ownership is passed to an outgoing request or response.

This change paves the way to runtime validation of the `Content-Length` header, ensuring that it's safe to check the value provided when an outgoing request or response is created.
<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs
index 89455e3aa..00400a07c 100644
--- 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/cli/reactor.wit b/crates/wasi-http/wit/deps/cli/reactor.wit
index 4b829834c..639f75557 100644
--- a/crates/wasi-http/wit/deps/cli/reactor.wit
+++ b/crates/wasi-http/wit/deps/cli/reactor.wit
@@ -3,7 +3,6 @@ package wasi:cli@0.2.0-rc-2023-11-05;
 world reactor {
   import wasi:clocks/wall-clock@0.2.0-rc-2023-11-05;
   import wasi:clocks/monotonic-clock@0.2.0-rc-2023-11-05;
-  import wasi:clocks/timezone@0.2.0-rc-2023-11-05;
   import wasi:filesystem/types@0.2.0-rc-2023-11-05;
   import wasi:filesystem/preopens@0.2.0-rc-2023-11-05;
   import wasi:sockets/instance-network@0.2.0-rc-2023-11-05;
diff --git a/crates/wasi-http/wit/deps/clocks/timezone.wit b/crates/wasi-http/wit/deps/clocks/timezone.wit
deleted file mode 100644
index e717e7b8d..000000000
--- a/crates/wasi-http/wit/deps/clocks/timezone.wit
+++ /dev/null
@@ -1,48 +0,0 @@
-interface timezone {
-    use wall-clock.{datetime};
-
-    /// Return information needed to display the given `datetime`. This includes
-    /// the UTC offset, the time zone name, and a flag indicating whether
-    /// daylight saving time is active.
-    ///
-    /// If the timezone cannot be determined for the given `datetime`, return a
-    /// `timezone-display` for `UTC` with a `utc-offset` of 0 and no daylight
-    /// saving time.
-    display: func(when: datetime) -> timezone-display;
-
-    /// The same as `display`, but only return the UTC offset.
-    utc-offset: func(when: datetime) -> s32;
-
-    /// Information useful for displaying the timezone of a specific `datetime`.
-    ///
-    /// This information may vary within a single `timezone` to reflect daylight
-    /// saving time adjustments.
-    record timezone-display {
-        /// The number of seconds difference between UTC time and the local
-        /// time of the timezone.
-        ///
-        /// The returned value will always be less than 86400 which is the
-        /// number of seconds in a day (24*60*60).
-        ///
-        /// In implementations that do not expose an actual time zone, this
-        /// should return 0.
-        utc-offset: s32,
-
-        /// The abbreviated name of the timezone to display to a user. The name
-        /// `UTC` indicates Coordinated Universal Time. Otherwise, this should
-        /// reference local standards for the name of the time zone.
-        ///
-        /// In implementations that do not expose an actual time zone, this
-        /// should be the string `UTC`.
-        ///
-        /// In time zones that do not have an applicable name, a formatted
-        /// representation of the UTC offset may be returned, such as `-04:00`.
-        name: string,
-
-        /// Whether daylight saving time is active.
-        ///
-        /// In implementations that do not expose an actual time zone, this
-        /// should return false.
-        in-daylight-saving-time: bool,
-    }
-}
diff --git a/crates/wasi-http/wit/deps/clocks/world.wit b/crates/wasi-http/wit/deps/clocks/world.wit
index a53d025b8..046145582 100644
--- a/crates/wasi-http/wit/deps/clocks/world.wit
+++ b/crates/wasi-http/wit/deps/clocks/world.wit
@@ -3,5 +3,4 @@ package wasi:clocks@0.2.0-rc-2023-11-05;
 world imports {
     import monotonic-clock;
     import wall-clock;
-    import timezone;
 }
diff --git a/crates/wasi-http/wit/deps/http/proxy.wit b/crates/wasi-http/wit/deps/http/proxy.wit
index 3fa8b9c63..4bc4eb644 100644
--- a/crates/wasi-http/wit/deps/http/proxy.wit
+++ b/crates/wasi-http/wit/deps/http/proxy.wit
@@ -8,7 +8,6 @@ world proxy {
   /// HTTP proxies have access to time and randomness.
   import wasi:clocks/wall-clock@0.2.0-rc-2023-11-05;
   import wasi:clocks/monotonic-clock@0.2.0-rc-2023-11-05;
-  import wasi:clocks/timezone@0.2.0-rc-2023-11-05;
   import wasi:random/random@0.2.0-rc-2023-11-05;
 
   /// Proxies have standard output and error streams which are expected to
diff --git a/crates/wasi-http/wit/deps/http/types.wit b/crates/wasi-http/wit/deps/http/types.wit
index fac47f61e..4662f3fd6 100644
--- 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/src/preview2/command.rs b/crates/wasi/src/preview2/command.rs
index 314a1a079..6ff304b24 100644
--- a/crates/wasi/src/preview2/command.rs
+++ b/crates/wasi/src/preview2/command.rs
@@ -11,7 +11,6 @@ wasmtime::component::bindgen!({
        "wasi:clocks/monotonic_clock": crate::preview2::bindings::clocks::monotonic_clock,
        "wasi:io/poll": crate::preview2::bindings::io::poll,
        "wasi:io/streams": crate::preview2::bindings::io::streams,
-       "wasi:clocks/timezone": crate::preview2::bindings::clocks::timezone,
        "wasi:clocks/wall_clock": crate::preview2::bindings::clocks::wall_clock,
        "wasi:random/random": crate::preview2::bindings::random::random,
        "wasi:cli/environment": crate::preview2::bindings::cli::environment,
@@ -30,7 +29,6 @@ wasmtime::component::bindgen!({
 pub fn add_to_linker<T: WasiView>(l: &mut wasmtime::component::Linker<T>) -> anyhow::Result<()> {
     crate::preview2::bindings::clocks::wall_clock::add_to_linker(l, |t| t)?;
     crate::preview2::bindings::clocks::monotonic_clock::add_to_linker(l, |t| t)?;
-    crate::preview2::bindings::clocks::timezone::add_to_linker(l, |t| t)?;
     crate::preview2::bindings::filesystem::types::add_to_linker(l, |t| t)?;
     crate::preview2::bindings::filesystem::preopens::add_to_linker(l, |t| t)?;
     crate::preview2::bindings::io::error::add_to_linker(l, |t| t)?;
@@ -72,7 +70,6 @@ pub mod sync {
            "wasi:clocks/monotonic_clock": crate::preview2::bindings::clocks::monotonic_clock,
            "wasi:io/poll": crate::preview2::bindings::sync_io::io::poll,
            "wasi:io/streams": crate::preview2::bindings::sync_io::io::streams,
-           "wasi:clocks/timezone": crate::preview2::bindings::clocks::timezone,
            "wasi:clocks/wall_clock": crate::preview2::bindings::clocks::wall_clock,
            "wasi:random/random": crate::preview2::bindings::random::random,
            "wasi:cli/environment": crate::preview2::bindings::cli::environment,
@@ -93,7 +90,6 @@ pub mod sync {
     ) -> anyhow::Result<()> {
         crate::preview2::bindings::clocks::wall_clock::add_to_linker(l, |t| t)?;
         crate::preview2::bindings::clocks::monotonic_clock::add_to_linker(l, |t| t)?;
-        crate::preview2::bindings::clocks::timezone::add_to_linker(l, |t| t)?;
         crate::preview2::bindings::sync_io::filesystem::types::add_to_linker(l, |t| t)?;
         crate::preview2::bindings::filesystem::preopens::add_to_linker(l, |t| t)?;
         crate::preview2::bindings::io::error::add_to_linker(l, |t| t)?;
diff --git a/crates/wasi/src/preview2/host/clocks.rs b/crates/wasi/src/preview2/host/clocks.rs
index 0b4c7dd36..4467b3681 100644
--- a/crates/wasi/src/preview2/host/clocks.rs
+++ b/crates/wasi/src/preview2/host/clocks.rs
@@ -2,7 +2,6 @@
 
 use crate::preview2::bindings::{
     clocks::monotonic_clock::{self, Duration as WasiDuration, Instant},
-    clocks::timezone::{self, TimezoneDisplay},
     clocks::wall_clock::{self, Datetime},
 };
 use crate::preview2::poll::{subscribe, Subscribe};
@@ -102,13 +101,3 @@ impl Subscribe for Deadline {
         }
     }
 }
-
-impl<T: WasiView> timezone::Host for T {
-    fn display(&mut self, when: Datetime) -> anyhow::Result<TimezoneDisplay> {
-        todo!("timezone display is not implemented")
-    }
-
-    fn utc_offset(&mut self, when: Datetime) -> anyhow::Result<i32> {
-        todo!("timezone utc_offset is not implemented")
-    }
-}
diff --git a/crates/wasi/wit/deps/cli/reactor.wit b/crates/wasi/wit/deps/cli/reactor.wit
index 4b829834c..639f75557 100644
--- a/crates/wasi/wit/deps/cli/reactor.wit
+++ b/crates/wasi/wit/deps/cli/reactor.wit
@@ -3,7 +3,6 @@ package wasi:cli@0.2.0-rc-2023-11-05;
 world reactor {
   import wasi:clocks/wall-clock@0.2.0-rc-2023-11-05;
   import wasi:clocks/monotonic-clock@0.2.0-rc-2023-11-05;
-  import wasi:clocks/timezone@0.2.0-rc-2023-11-05;
   import wasi:filesystem/types@0.2.0-rc-2023-11-05;
   import wasi:filesystem/preopens@0.2.0-rc-2023-11-05;
   import wasi:sockets/instance-network@0.2.0-rc-2023-11-05;
diff --git a/crates/wasi/wit/deps/clocks/timezone.wit b/crates/wasi/wit/deps/clocks/timezone.wit
deleted file mode 100644
index e717e7b8d..000000000
--- a/crates/wasi/wit/deps/clocks/timezone.wit
+++ /dev/null
@@ -1,48 +0,0 @@
-interface timezone {
-    use wall-clock.{datetime};
-
-    /// Return information needed to display the given `datetime`. This includes
-    /// the UTC offset, the time zone name, and a flag indicating whether
-    /// daylight saving time is active.
-    ///
-    /// If the timezone cannot be determined for the given `datetime`, return a
-    /// `timezone-display` for `UTC` with a `utc-offset` of 0 and no daylight
-    /// saving time.
-    display: func(when: datetime) -> timezone-display;
-
-    /// The same as `display`, but only return the UTC offset.
-    utc-offset: func(when: datetime) -> s32;
-
-    /// Information useful for displaying the timezone of a specific `datetime`.
-    ///
-    /// This information may vary within a single `timezone` to reflect daylight
-    /// saving time adjustments.
-    record timezone-display {
-        /// The number of seconds difference between UTC time and the local
-        /// time of the timezone.
-        ///
-        /// The returned value will always be less than 86400 which is the
-        /// number of seconds in a day (24*60*60).
-        ///
-        /// In implementations that do not expose an actual time zone, this
-        /// should return 0.
-        utc-offset: s32,
-
-        /// The abbreviated name of the timezone to display to a user. The name
-        /// `UTC` indicates Coordinated Universal Time. Otherwise, this should
-        /// reference local standards for the name of the time zone.
-        ///
-        /// In implementations that do not expose an actual time zone, this
-        /// should be the string `UTC`.
-        ///
-        /// In time zones that do not have an applicable name, a formatted
-        /// representation of the UTC offset may be returned, such as `-04:00`.
-        name: string,
-
-        /// Whether daylight saving time is active.
-        ///
-        /// In implementations that do not expose an actual time zone, this
-        /// should return false.
-        in-daylight-saving-time: bool,
-    }
-}
diff --git a/crates/wasi/wit/deps/clocks/world.wit b/crates/wasi/wit/deps/clocks/world.wit
index a53d025b8..046145582 100644
--- a/crates/wasi/wit/deps/clocks/world.wit
+++ b/crates/wasi/wit/deps/clocks/world.wit
@@ -3,5 +3,4 @@ package wasi:clocks@0.2.0-rc-2023-11-05;
 world imports {
     import monotonic-clock;
     import wall-clock;
-    import timezone;
 }
diff --git a/crates/wasi/wit/deps/http/proxy.wit b/crates/wasi/wit/deps/http/proxy.wit
index 3fa8b9c63..4bc4eb644 100644
--- a/crates/wasi/wit/deps/http/proxy.wit
+++ b/crates/wasi/wit/deps/http/proxy.wit
@@ -8,7 +8,6 @@ world proxy {
   /// HTTP proxies have access to time and randomness.
   import wasi:clocks/wall-clock@0.2.0-rc-2023-11-05;
   import wasi:clocks/monotonic-clock@0.2.0-rc-2023-11-05;
-  import wasi:clocks/timezone@0.2.0-rc-2023-11-05;
   import wasi:random/random@0.2.0-rc-2023-11-05;
 
   /// Proxies have standard output and error streams which are expected to
diff --git a/crates/wasi/wit/deps/http/types.wit b/crates/wasi/wit/deps/http/types.wit
index fac47f61e..4662f3fd6 100644
--- 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.


Test patch:
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
index 09f173298..65931dd61 100644
--- 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),
+    ));
 }