diff --git a/crates/test-programs/src/bin/p2_cli_http_headers.rs b/crates/test-programs/src/bin/p2_cli_http_headers.rs index 1d0939264344..c5c21c1fe32a 100644 --- a/crates/test-programs/src/bin/p2_cli_http_headers.rs +++ b/crates/test-programs/src/bin/p2_cli_http_headers.rs @@ -1,31 +1,71 @@ -fn main() { - let fields = wasip2::http::types::Fields::new(); +use test_programs::p3::wasi as wasip3; +fn main() { match std::env::args().nth(1).as_deref() { - Some("append") => { + Some("p2-append") => { + let fields = wasip2::http::types::Fields::new(); + for i in 0.. { + if fields.append(&format!("a{i}"), b"a").is_err() { + break; + } + } + } + Some("p2-append-empty") => { + let fields = wasip2::http::types::Fields::new(); + for i in 0.. { + if fields.append(&format!("a{i}"), b"").is_err() { + break; + } + } + } + Some("p2-append-same") => { + let fields = wasip2::http::types::Fields::new(); + loop { + if fields.append("a", b"b").is_err() { + break; + } + } + } + Some("p2-append-same-empty") => { + let fields = wasip2::http::types::Fields::new(); + loop { + if fields.append("a", b"").is_err() { + break; + } + } + } + Some("p3-append") => { + let fields = wasip3::http::types::Fields::new(); for i in 0.. { if fields.append(&format!("a{i}"), b"a").is_err() { break; } } } - Some("append-empty") => { + Some("p3-append-empty") => { + let fields = wasip3::http::types::Fields::new(); for i in 0.. { if fields.append(&format!("a{i}"), b"").is_err() { break; } } } - Some("append-same") => loop { - if fields.append("a", b"b").is_err() { - break; + Some("p3-append-same") => { + let fields = wasip3::http::types::Fields::new(); + loop { + if fields.append("a", b"b").is_err() { + break; + } } - }, - Some("append-same-empty") => loop { - if fields.append("a", b"").is_err() { - break; + } + Some("p3-append-same-empty") => { + let fields = wasip3::http::types::Fields::new(); + loop { + if fields.append("a", b"").is_err() { + break; + } } - }, + } other => panic!("unknown test {other:?}"), } diff --git a/crates/wasi-http/src/field_map.rs b/crates/wasi-http/src/field_map.rs new file mode 100644 index 000000000000..556e87b6d496 --- /dev/null +++ b/crates/wasi-http/src/field_map.rs @@ -0,0 +1,357 @@ +use http::header::Entry; +use http::{HeaderMap, HeaderName, HeaderValue}; +use std::fmt; +use std::ops::Deref; +use std::sync::Arc; +use wasmtime::Result; + +/// A wrapper around [`http::HeaderMap`] which implements `wasi:http` semantics. +/// +/// The main differences from [`http::HeaderMap`] and this type are: +/// +/// * A slimmed down mutability API to just what `wasi:http` needs. +/// * `FieldMap` is cheaply clone-able with the internal `HeaderMap` being +/// behind an `Arc`. +/// * `FieldMap` is either immutable or mutable. Mutations on immutable values +/// are rejected with an error. Mutations on mutable values will never panic +/// unlike `HeaderMap` and additionally require a limit to be set on the size +/// of the map. +/// +/// Overall the intention is that this is a slim wrapper around +/// [`http::HeaderMap`] with slightly different ownership, panic, and error +/// semantics. +#[derive(Debug, Clone)] +pub struct FieldMap { + map: Arc, + limit: Limit, + size: usize, +} + +#[derive(Debug, Clone)] +enum Limit { + Mutable(usize), + Immutable, +} + +impl Default for FieldMap { + fn default() -> Self { + Self::new_immutable(HeaderMap::default()) + } +} + +impl FieldMap { + /// Creates a new immutable `FieldMap` from the provided + /// [`http::HeaderMap`]. + /// + /// The returned value cannot be mutated and attempting to mutate it will + /// return an error. + pub fn new_immutable(map: HeaderMap) -> Self { + let size = Self::content_size(&map); + Self { + map: Arc::new(map), + size, + limit: Limit::Immutable, + } + } + + /// Creates a new, empty, mutable `FieldMap`. + /// + /// Mutations are allowed on the returned value and up to `limit` bytes of + /// memory (roughly) may be consumed by this map. + pub fn new_mutable(limit: usize) -> Self { + Self { + map: Arc::new(HeaderMap::new()), + size: 0, + limit: Limit::Mutable(limit), + } + } + + /// Calculate the content size of a `HeaderMap`. This is a sum of the size + /// of all of the keys and all of the values. + pub(crate) fn content_size(map: &HeaderMap) -> usize { + let mut sum = 0; + for key in map.keys() { + sum += header_name_size(key); + } + for value in map.values() { + sum += header_value_size(value); + } + sum + } + + /// Sets the header `key` to the `values` list provided. + /// + /// Removes the previous value, if any. + /// + /// If `values` is empty then this removes the header `key`. + // + // FIXME(WebAssembly/WASI#900): is this the right behavior? + pub fn set(&mut self, key: HeaderName, values: Vec) -> Result<(), FieldMapError> { + let (map, limit, size) = self.mutable()?; + let key_size = header_name_size(&key); + let values_size = values.iter().map(header_value_size).sum::(); + let mut values = values.into_iter(); + let mut entry = match map.try_entry(key)? { + Entry::Vacant(e) => match values.next() { + Some(v) => { + update_size(size, limit, *size + values_size + key_size)?; + e.try_insert_entry(v)? + } + None => return Ok(()), + }, + Entry::Occupied(mut e) => { + let prev_values_size = e.iter().map(header_value_size).sum::(); + let _prev = match values.next() { + Some(v) => { + update_size(size, limit, *size - prev_values_size + values_size)?; + e.insert(v); + } + None => { + update_size(size, limit, *size - prev_values_size - key_size)?; + e.remove(); + return Ok(()); + } + }; + e + } + }; + for value in values { + entry.append(value); + } + Ok(()) + } + + /// Remove all values associated with a key in a map. + /// + /// Returns an empty list if the key is not already present within the map. + pub fn remove_all(&mut self, key: HeaderName) -> Result, FieldMapError> { + let (map, _limit, size) = self.mutable()?; + match map.try_entry(key)? { + Entry::Vacant { .. } => Ok(Vec::new()), + Entry::Occupied(e) => { + let (name, value_drain) = e.remove_entry_mult(); + let mut removed = header_name_size(&name); + let values = value_drain.collect::>(); + for v in values.iter() { + removed += header_value_size(v); + } + *size -= removed; + Ok(values) + } + } + } + + fn mutable(&mut self) -> Result<(&mut HeaderMap, usize, &mut usize), FieldMapError> { + match self.limit { + Limit::Immutable => Err(FieldMapError::Immutable), + Limit::Mutable(limit) => Ok((Arc::make_mut(&mut self.map), limit, &mut self.size)), + } + } + + /// Add a value associated with a key to the map. + /// + /// If `key` is already present within the map then `value` is appended to + /// the list of values it already has. + pub fn append(&mut self, key: HeaderName, value: HeaderValue) -> Result { + let (map, limit, size) = self.mutable()?; + let key_size = header_name_size(&key); + let val_size = header_value_size(&value); + let new_size = if !map.contains_key(&key) { + *size + key_size + val_size + } else { + *size + val_size + }; + update_size(size, limit, new_size)?; + let already_present = map.try_append(key, value)?; + self.size = new_size; + Ok(already_present) + } + + /// Flags this map as mutable, allowing mutations which can allocate as much + /// as `limit` memory, in bytes, for this entire map (roughly). + pub fn set_mutable(&mut self, limit: usize) { + self.limit = Limit::Mutable(limit); + } + + /// Flags this map as immutable, forbidding all further mutations. + pub fn set_immutable(&mut self) { + self.limit = Limit::Immutable; + } +} + +/// Returns the size, in accounting cost, to consider for `name`. +/// +/// This includes both the byte length of the `name` itself as well as the size +/// of the data structure itself as it'll reside within a `HeaderMap`. +fn header_name_size(name: &HeaderName) -> usize { + name.as_str().len() + size_of::() +} + +/// Same as `header_name_size`, but for values. +/// +/// This notably includes the size of `HeaderValue` itself to ensure that all +/// headers have a nonzero size as otherwise this would never limit addition of +/// an empty header value. +fn header_value_size(value: &HeaderValue) -> usize { + value.len() + size_of::() +} + +fn update_size(size: &mut usize, limit: usize, new: usize) -> Result<(), FieldMapError> { + if new > limit { + Err(FieldMapError::TotalSizeTooBig) + } else { + *size = new; + Ok(()) + } +} + +// Note that `DerefMut` is specifically omitted here to force all mutations +// through the `FieldMap` wrapper. +impl Deref for FieldMap { + type Target = HeaderMap; + + fn deref(&self) -> &HeaderMap { + &self.map + } +} + +impl From for HeaderMap { + fn from(map: FieldMap) -> Self { + Arc::unwrap_or_clone(map.map) + } +} + +/// Errors that can happen when mutating/operating on a [`FieldMap`]. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum FieldMapError { + /// A mutation was attempted when the map is not mutable. + Immutable, + /// The map has too many fields and is not allowed to add more. + /// + /// Note that this is currently a limitation inherited from + /// [`http::HeaderMap`]. + TooManyFields, + /// The map's total size, of keys and values, is too large. + TotalSizeTooBig, + /// An invalid header name was attempted to be added. + InvalidHeaderName, +} + +impl fmt::Display for FieldMapError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let s = match self { + FieldMapError::Immutable => "cannot mutate an immutable field map", + FieldMapError::TooManyFields => "too many fields in the field map", + FieldMapError::TotalSizeTooBig => "total size of fields exceeds limit", + FieldMapError::InvalidHeaderName => "invalid header name", + }; + f.write_str(s) + } +} + +impl std::error::Error for FieldMapError {} + +impl From for FieldMapError { + fn from(_: http::header::MaxSizeReached) -> Self { + Self::TooManyFields + } +} + +impl From for FieldMapError { + fn from(_: http::header::InvalidHeaderName) -> Self { + Self::InvalidHeaderName + } +} + +#[cfg(test)] +mod tests { + use super::{FieldMap, FieldMapError}; + + #[test] + fn test_immutable() { + let mut map = FieldMap::default(); + assert_eq!( + map.set("foo".parse().unwrap(), vec!["bar".parse().unwrap()]), + Err(FieldMapError::Immutable) + ); + assert_eq!( + map.append("foo".parse().unwrap(), "bar".parse().unwrap()), + Err(FieldMapError::Immutable) + ); + assert_eq!( + map.remove_all("foo".parse().unwrap()), + Err(FieldMapError::Immutable) + ); + } + + #[test] + fn test_limits() { + let mut map = FieldMap::new_mutable(100); + loop { + match map.append("foo".parse().unwrap(), "bar".parse().unwrap()) { + Ok(_) => {} + Err(FieldMapError::TotalSizeTooBig) => break, + Err(e) => panic!("unexpected error: {e}"), + } + } + + map = FieldMap::new_mutable(100); + for i in 0.. { + match map.set( + "foo".parse().unwrap(), + (0..i).map(|j| format!("bar{j}").parse().unwrap()).collect(), + ) { + Ok(_) => {} + Err(FieldMapError::TotalSizeTooBig) => break, + Err(e) => panic!("unexpected error: {e}"), + } + } + + map = FieldMap::new_mutable(100); + for i in 0.. { + match map.set( + format!("foo{i}").parse().unwrap(), + vec!["bar".parse().unwrap()], + ) { + Ok(_) => {} + Err(FieldMapError::TotalSizeTooBig) => break, + Err(e) => panic!("unexpected error: {e}"), + } + } + } + + #[test] + fn test_size() -> Result<(), FieldMapError> { + let mut map = FieldMap::new_mutable(2000); + let name: http::HeaderName = "foo".parse().unwrap(); + + map.append(name.clone(), "bar".parse().unwrap())?; + assert!(map.size > 0); + map.remove_all(name.clone())?; + assert_eq!(map.size, 0); + + map.set(name.clone(), vec!["bar".parse().unwrap()])?; + assert!(map.size > 0); + map.remove_all(name.clone())?; + assert_eq!(map.size, 0); + + map.set(name.clone(), vec![])?; + assert_eq!(map.size, 0); + map.set(name.clone(), vec!["bar".parse().unwrap()])?; + assert!(map.size > 0); + map.set(name.clone(), vec![])?; + assert_eq!(map.size, 0); + + map.set(name.clone(), vec!["bar".parse().unwrap()])?; + assert!(map.size > 0); + map.set( + name.clone(), + vec!["bar".parse().unwrap(), "baz".parse().unwrap()], + )?; + assert!(map.size > 0); + map.remove_all(name.clone())?; + assert_eq!(map.size, 0); + + Ok(()) + } +} diff --git a/crates/wasi-http/src/lib.rs b/crates/wasi-http/src/lib.rs index cdf8648ac37a..b61c11c6466c 100644 --- a/crates/wasi-http/src/lib.rs +++ b/crates/wasi-http/src/lib.rs @@ -12,6 +12,7 @@ use http::{HeaderName, header}; mod ctx; +mod field_map; #[cfg(feature = "component-model-async")] pub mod handler; pub mod io; @@ -21,6 +22,7 @@ pub mod p2; pub mod p3; pub use ctx::*; +pub use field_map::*; /// Extract the `Content-Length` header value from a [`http::HeaderMap`], returning `None` if it's not /// present. This function will return `Err` if it's not possible to parse the `Content-Length` diff --git a/crates/wasi-http/src/p2/bindings.rs b/crates/wasi-http/src/p2/bindings.rs index e870ea033042..f3e8b3503576 100644 --- a/crates/wasi-http/src/p2/bindings.rs +++ b/crates/wasi-http/src/p2/bindings.rs @@ -26,11 +26,12 @@ mod generated { "wasi:http/types.response-outparam": types::HostResponseOutparam, "wasi:http/types.outgoing-request": types::HostOutgoingRequest, "wasi:http/types.incoming-request": types::HostIncomingRequest, - "wasi:http/types.fields": types::HostFields, + "wasi:http/types.fields": crate::FieldMap, "wasi:http/types.request-options": types::HostRequestOptions, }, trappable_error_type: { "wasi:http/types.error-code" => crate::p2::HttpError, + "wasi:http/types.header-error" => crate::p2::HeaderError, }, }); } diff --git a/crates/wasi-http/src/p2/body.rs b/crates/wasi-http/src/p2/body.rs index 5d8c929b4278..99dbb949e8f8 100644 --- a/crates/wasi-http/src/p2/body.rs +++ b/crates/wasi-http/src/p2/body.rs @@ -1,7 +1,7 @@ //! Implementation of the `wasi:http/types` interface's various body types. +use crate::FieldMap; use crate::p2::bindings::http::types; -use crate::p2::types::FieldMap; use bytes::Bytes; use http_body::{Body, Frame}; use http_body_util::BodyExt; @@ -25,7 +25,6 @@ pub type HyperOutgoingBody = UnsyncBoxBody; #[derive(Debug)] pub struct HostIncomingBody { body: IncomingBodyState, - field_size_limit: usize, /// An optional worker task to keep alive while this body is being read. /// This ensures that if the parent of this body is dropped before the body /// then the backing data behind this worker is kept alive. @@ -34,15 +33,10 @@ pub struct HostIncomingBody { impl HostIncomingBody { /// Create a new `HostIncomingBody` with the given `body` and a per-frame timeout - pub fn new( - body: HyperIncomingBody, - between_bytes_timeout: Duration, - field_size_limit: usize, - ) -> HostIncomingBody { + pub fn new(body: HyperIncomingBody, between_bytes_timeout: Duration) -> HostIncomingBody { let body = BodyWithTimeout::new(body, between_bytes_timeout); HostIncomingBody { body: IncomingBodyState::Start(body), - field_size_limit, worker: None, } } @@ -325,7 +319,7 @@ pub enum HostFutureTrailers { /// /// Note that `Ok(None)` means that there were no trailers for this request /// while `Ok(Some(_))` means that trailers were found in the request. - Done(Result, types::ErrorCode>), + Done(Result, types::ErrorCode>), /// Trailers have been consumed by `future-trailers.get`. Consumed, @@ -347,7 +341,7 @@ impl Pollable for HostFutureTrailers { // Trailers were read for us and here they are, so store the // result. Ok(StreamEnd::Trailers(Some(t))) => { - *self = Self::Done(Ok(Some(FieldMap::new(t, body.field_size_limit)))); + *self = Self::Done(Ok(Some(t))); } // The body wasn't fully read and was dropped before trailers // were reached. It's up to us now to complete the body. @@ -379,7 +373,7 @@ impl Pollable for HostFutureTrailers { // If this frame is a data frame ignore it as we're only // interested in trailers. if let Ok(header_map) = frame.into_trailers() { - break Ok(Some(FieldMap::new(header_map, body.field_size_limit))); + break Ok(Some(header_map)); } } } @@ -529,7 +523,7 @@ impl HostOutgoingBody { } let message = if let Some(ts) = trailers { - FinishMessage::Trailers(ts.into_inner()) + FinishMessage::Trailers(ts.into()) } else { FinishMessage::Finished }; diff --git a/crates/wasi-http/src/p2/error.rs b/crates/wasi-http/src/p2/error.rs index 9bc1ebd6f057..7dae4ee30f46 100644 --- a/crates/wasi-http/src/p2/error.rs +++ b/crates/wasi-http/src/p2/error.rs @@ -1,4 +1,5 @@ -use crate::p2::bindings::http::types::ErrorCode; +use crate::FieldMapError; +use crate::p2::bindings::http::types::{self, ErrorCode}; use std::error::Error; use std::fmt; use wasmtime::component::ResourceTableError; @@ -58,6 +59,81 @@ impl fmt::Display for HttpError { impl Error for HttpError {} +/// A [`Result`] type where the error type defaults to [`HeaderError`]. +pub type HeaderResult = Result; + +/// A `wasi:http`-specific error type used to represent either a trap or an +/// [`types::HeaderError`]. +/// +/// Modeled after [`TrappableError`](wasmtime_wasi::TrappableError). +#[repr(transparent)] +pub struct HeaderError { + err: wasmtime::Error, +} + +impl HeaderError { + /// Create a new `HeaderError` that represents a trap. + pub fn trap(err: impl Into) -> HeaderError { + HeaderError { err: err.into() } + } + + /// Downcast this error to an [`ErrorCode`]. + pub fn downcast(self) -> wasmtime::Result { + self.err.downcast() + } + + /// Downcast this error to a reference to an [`ErrorCode`] + pub fn downcast_ref(&self) -> Option<&types::HeaderError> { + self.err.downcast_ref() + } +} + +impl From for HeaderError { + fn from(error: types::HeaderError) -> Self { + Self { err: error.into() } + } +} + +impl From for HeaderError { + fn from(error: ResourceTableError) -> Self { + HeaderError::trap(error) + } +} + +impl From for HeaderError { + fn from(_: http::header::InvalidHeaderName) -> Self { + HeaderError::from(types::HeaderError::InvalidSyntax) + } +} + +impl From for HeaderError { + fn from(_: http::header::InvalidHeaderValue) -> Self { + HeaderError::from(types::HeaderError::InvalidSyntax) + } +} + +impl From for HeaderError { + fn from(err: FieldMapError) -> Self { + match err { + FieldMapError::Immutable => types::HeaderError::Immutable.into(), + FieldMapError::InvalidHeaderName => types::HeaderError::InvalidSyntax.into(), + FieldMapError::TooManyFields | FieldMapError::TotalSizeTooBig => HeaderError::trap(err), + } + } +} + +impl fmt::Debug for HeaderError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.err.fmt(f) + } +} + +impl fmt::Display for HeaderError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.err.fmt(f) + } +} + #[cfg(feature = "default-send-request")] pub(crate) fn dns_error(rcode: String, info_code: u16) -> ErrorCode { ErrorCode::DnsError(crate::p2::bindings::http::types::DnsErrorPayload { diff --git a/crates/wasi-http/src/p2/http_impl.rs b/crates/wasi-http/src/p2/http_impl.rs index 45b4312044d9..9510f7a44d6f 100644 --- a/crates/wasi-http/src/p2/http_impl.rs +++ b/crates/wasi-http/src/p2/http_impl.rs @@ -76,7 +76,7 @@ impl outgoing_handler::Host for WasiHttpCtxView<'_> { builder = builder.uri(uri.build().map_err(http_request_error)?); - for (k, v) in req.headers.as_ref().iter() { + for (k, v) in req.headers.iter() { builder = builder.header(k, v); } diff --git a/crates/wasi-http/src/p2/mod.rs b/crates/wasi-http/src/p2/mod.rs index 6f42d9260c26..72d0f7637fe6 100644 --- a/crates/wasi-http/src/p2/mod.rs +++ b/crates/wasi-http/src/p2/mod.rs @@ -233,9 +233,7 @@ pub mod bindings; pub mod body; pub mod types; -pub use self::error::{ - HttpError, HttpResult, http_request_error, hyper_request_error, hyper_response_error, -}; +pub use self::error::*; /// A trait which provides hooks into internal WASI HTTP operations. /// diff --git a/crates/wasi-http/src/p2/types.rs b/crates/wasi-http/src/p2/types.rs index d53290acd306..dd845ad7b2e5 100644 --- a/crates/wasi-http/src/p2/types.rs +++ b/crates/wasi-http/src/p2/types.rs @@ -1,17 +1,15 @@ //! Implements the base structure that will provide the implementation of the //! wasi-http API. +use crate::FieldMap; use crate::p2::{ WasiHttpCtxView, WasiHttpHooks, bindings::http::types::{self, ErrorCode, Method, Scheme}, body::{HostIncomingBody, HyperIncomingBody, HyperOutgoingBody}, }; use bytes::Bytes; -use http::header::{HeaderMap, HeaderName, HeaderValue}; use http_body_util::BodyExt; use hyper::body::Body; -use std::any::Any; -use std::fmt; use std::time::Duration; use wasmtime::component::Resource; use wasmtime::{Result, bail}; @@ -19,8 +17,11 @@ use wasmtime_wasi::p2::Pollable; use wasmtime_wasi::runtime::AbortOnDropJoinHandle; /// Removes forbidden headers from a [`FieldMap`]. -pub(crate) fn remove_forbidden_headers(hooks: &mut dyn WasiHttpHooks, headers: &mut FieldMap) { - let forbidden_keys = Vec::from_iter(headers.as_ref().keys().filter_map(|name| { +pub(crate) fn remove_forbidden_headers( + hooks: &mut dyn WasiHttpHooks, + headers: &mut http::HeaderMap, +) { + let forbidden_keys = Vec::from_iter(headers.keys().filter_map(|name| { if hooks.is_forbidden_header(name) { Some(name.clone()) } else { @@ -29,7 +30,7 @@ pub(crate) fn remove_forbidden_headers(hooks: &mut dyn WasiHttpHooks, headers: & })); for name in forbidden_keys { - headers.remove_all(&name); + headers.remove(&name); } } @@ -113,14 +114,12 @@ impl WasiHttpCtxView<'_> { B: Body + Send + 'static, B::Error: Into, { - let field_size_limit = self.ctx.field_size_limit; - let (parts, body) = req.into_parts(); + let (mut parts, body) = req.into_parts(); let body = body.map_err(Into::into).boxed_unsync(); let body = HostIncomingBody::new( body, // TODO: this needs to be plumbed through std::time::Duration::from_millis(600 * 1000), - field_size_limit, ); let authority = match parts.uri.authority() { Some(authority) => authority.to_string(), @@ -130,8 +129,8 @@ impl WasiHttpCtxView<'_> { }, }; - let mut headers = FieldMap::new(parts.headers, field_size_limit); - remove_forbidden_headers(self.hooks, &mut headers); + remove_forbidden_headers(self.hooks, &mut parts.headers); + let headers = FieldMap::new_immutable(parts.headers); let req = HostIncomingRequest { method: parts.method, @@ -185,7 +184,7 @@ impl TryFrom for hyper::Response { let mut builder = hyper::Response::builder().status(resp.status); - *builder.headers_mut().unwrap() = resp.headers.map; + *builder.headers_mut().unwrap() = resp.headers.into(); match resp.body { Some(body) => builder.body(body), @@ -237,153 +236,6 @@ pub struct HostIncomingResponse { pub body: Option, } -/// The concrete type behind a `wasi:http/types.fields` resource. -#[derive(Debug)] -pub enum HostFields { - /// A reference to the fields of a parent entry. - Ref { - /// The parent resource rep. - parent: u32, - - /// The function to get the fields from the parent. - // NOTE: there's not failure in the result here because we assume that HostFields will - // 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)) -> &'a mut FieldMap, - }, - /// An owned version of the fields. - Owned { - /// The fields themselves. - fields: FieldMap, - }, -} - -/// An owned version of `HostFields`. A wrapper on http `HeaderMap` that -/// keeps a running tally of memory consumed by header names and values. -#[derive(Debug, Clone)] -pub struct FieldMap { - map: HeaderMap, - limit: usize, - size: usize, -} - -/// Error given when a `FieldMap` has exceeded the size limit. -#[derive(Debug)] -pub struct FieldSizeLimitError { - /// The erroring `FieldMap` operation would require this content size - pub(crate) size: usize, - /// The limit set on `FieldMap` content size - pub(crate) limit: usize, -} -impl fmt::Display for FieldSizeLimitError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Field size limit {} exceeded: {}", self.limit, self.size) - } -} -impl std::error::Error for FieldSizeLimitError {} - -impl FieldMap { - /// Construct a `FieldMap` from a `HeaderMap` and a size limit. - /// - /// Construction with a `HeaderMap` which exceeds the size limit is - /// allowed, but subsequent operations to expand the resource use will - /// fail. - pub fn new(map: HeaderMap, limit: usize) -> Self { - let size = Self::content_size(&map); - Self { map, size, limit } - } - /// Construct an empty `FieldMap` - pub fn empty(limit: usize) -> Self { - Self { - map: HeaderMap::new(), - size: 0, - limit, - } - } - /// Get the `HeaderMap` out of the `FieldMap` - pub fn into_inner(self) -> HeaderMap { - self.map - } - /// Calculate the content size of a `HeaderMap`. This is a sum of the size - /// of all of the keys and all of the values. - pub(crate) fn content_size(map: &HeaderMap) -> usize { - let mut sum = 0; - for key in map.keys() { - sum += header_name_size(key); - } - for value in map.values() { - sum += header_value_size(value); - } - sum - } - /// Remove all values associated with a key in a map. - /// - /// Returns an empty list if the key is not already present within the map. - pub fn remove_all(&mut self, key: &HeaderName) -> Vec { - use http::header::Entry; - match self.map.try_entry(key) { - Ok(Entry::Vacant { .. }) | Err(_) => Vec::new(), - Ok(Entry::Occupied(e)) => { - let (name, value_drain) = e.remove_entry_mult(); - let mut removed = header_name_size(&name); - let values = value_drain.collect::>(); - for v in values.iter() { - removed += header_value_size(v); - } - self.size -= removed; - values - } - } - } - /// Add a value associated with a key to the map. - /// - /// If `key` is already present within the map then `value` is appended to - /// the list of values it already has. - pub fn append(&mut self, key: &HeaderName, value: HeaderValue) -> Result { - let key_size = header_name_size(key); - let val_size = header_value_size(&value); - let new_size = if !self.map.contains_key(key) { - self.size + key_size + val_size - } else { - self.size + val_size - }; - if new_size > self.limit { - bail!(FieldSizeLimitError { - limit: self.limit, - size: new_size - }) - } - self.size = new_size; - Ok(self.map.try_append(key, value)?) - } -} - -/// Returns the size, in accounting cost, to consider for `name`. -/// -/// This includes both the byte length of the `name` itself as well as the size -/// of the data structure itself as it'll reside within a `HeaderMap`. -fn header_name_size(name: &HeaderName) -> usize { - name.as_str().len() + size_of::() -} - -/// Same as `header_name_size`, but for values. -/// -/// This notably includes the size of `HeaderValue` itself to ensure that all -/// headers have a nonzero size as otherwise this would never limit addition of -/// an empty header value. -fn header_value_size(value: &HeaderValue) -> usize { - value.len() + size_of::() -} - -// We impl AsRef, but not AsMut, because any modifications of the -// underlying HeaderMap must account for changes in size -impl AsRef for FieldMap { - fn as_ref(&self) -> &HeaderMap { - &self.map - } -} - /// A handle to a future incoming response. pub type FutureIncomingResponseHandle = AbortOnDropJoinHandle>>; diff --git a/crates/wasi-http/src/p2/types_impl.rs b/crates/wasi-http/src/p2/types_impl.rs index 3bf7e377cc07..e403784a7b52 100644 --- a/crates/wasi-http/src/p2/types_impl.rs +++ b/crates/wasi-http/src/p2/types_impl.rs @@ -1,18 +1,17 @@ //! Implementation for the `wasi:http/types` interface. +use crate::FieldMap; use crate::get_content_length; -use crate::p2::bindings::http::types::{self, Headers, Method, Scheme, StatusCode, Trailers}; +use crate::p2::bindings::http::types::{self, Method, Scheme, StatusCode, Trailers}; use crate::p2::body::{HostFutureTrailers, HostIncomingBody, HostOutgoingBody, StreamContext}; use crate::p2::types::{ - FieldMap, FieldSizeLimitError, HostFields, HostFutureIncomingResponse, HostIncomingRequest, - HostIncomingResponse, HostOutgoingRequest, HostOutgoingResponse, HostResponseOutparam, - remove_forbidden_headers, + HostFutureIncomingResponse, HostIncomingRequest, HostIncomingResponse, HostOutgoingRequest, + HostOutgoingResponse, HostResponseOutparam, remove_forbidden_headers, }; -use crate::p2::{HttpError, HttpResult, WasiHttpCtxView}; -use std::any::Any; +use crate::p2::{HeaderError, HeaderResult, HttpError, HttpResult, WasiHttpCtxView}; +use http::{HeaderName, HeaderValue}; use std::str::FromStr; -use wasmtime::bail; -use wasmtime::component::{Resource, ResourceTable, ResourceTableError}; +use wasmtime::component::Resource; use wasmtime::{error::Context as _, format_err}; use wasmtime_wasi::p2::{DynInputStream, DynOutputStream, DynPollable}; @@ -21,6 +20,10 @@ impl types::Host for WasiHttpCtxView<'_> { err.downcast() } + fn convert_header_error(&mut self, err: HeaderError) -> wasmtime::Result { + err.downcast() + } + fn http_error_code( &mut self, err: wasmtime::component::Resource, @@ -30,129 +33,52 @@ impl types::Host for WasiHttpCtxView<'_> { } } -/// Take ownership of the underlying [`FieldMap`] associated with this fields resource. If the -/// fields resource references another fields, the returned [`FieldMap`] will be cloned. -fn move_fields( - table: &mut ResourceTable, - id: Resource, -) -> Result { - match table.delete(id)? { - HostFields::Ref { parent, get_fields } => { - let entry = table.get_any_mut(parent)?; - Ok(get_fields(entry).clone()) - } - - HostFields::Owned { fields } => Ok(fields), - } -} - -fn get_fields<'a>( - table: &'a mut ResourceTable, - id: &Resource, -) -> wasmtime::Result<&'a FieldMap> { - let fields = table.get(&id)?; - if let HostFields::Ref { parent, get_fields } = *fields { - let entry = table.get_any_mut(parent)?; - return Ok(get_fields(entry)); - } - - match table.get_mut(&id)? { - HostFields::Owned { fields } => Ok(fields), - // NB: ideally the `if let` above would go here instead. That makes - // the borrow-checker unhappy. Unclear why. If you, dear reader, can - // refactor this to remove the `unreachable!` please do. - HostFields::Ref { .. } => unreachable!(), - } -} - -fn get_fields_mut<'a>( - table: &'a mut ResourceTable, - id: &Resource, -) -> wasmtime::Result> { - match table.get_mut(&id)? { - HostFields::Owned { fields } => Ok(Ok(fields)), - HostFields::Ref { .. } => Ok(Err(types::HeaderError::Immutable)), - } -} - impl types::HostFields for WasiHttpCtxView<'_> { - fn new(&mut self) -> wasmtime::Result> { + fn new(&mut self) -> wasmtime::Result> { let limit = self.ctx.field_size_limit; let id = self .table - .push(HostFields::Owned { - fields: FieldMap::empty(limit), - }) + .push(FieldMap::new_mutable(limit)) .context("[new_fields] pushing fields")?; Ok(id) } - fn from_list( - &mut self, - entries: Vec<(String, Vec)>, - ) -> wasmtime::Result, types::HeaderError>> { - let mut fields = hyper::HeaderMap::new(); + fn from_list(&mut self, entries: Vec<(String, Vec)>) -> HeaderResult> { + let mut fields = FieldMap::new_mutable(self.ctx.field_size_limit); for (header, value) in entries { - let header = match hyper::header::HeaderName::from_bytes(header.as_bytes()) { - Ok(header) => header, - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - }; - + let header = HeaderName::from_bytes(header.as_bytes())?; if self.hooks.is_forbidden_header(&header) { - return Ok(Err(types::HeaderError::Forbidden)); + return Err(types::HeaderError::Forbidden.into()); } - - let value = match hyper::header::HeaderValue::from_bytes(&value) { - Ok(value) => value, - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - }; - - fields.append(header, value); - } - - let size = FieldMap::content_size(&fields); - if size > self.ctx.field_size_limit { - bail!(FieldSizeLimitError { - size, - limit: self.ctx.field_size_limit, - }); + let value = HeaderValue::from_bytes(&value)?; + fields.append(header, value)?; } - let fields = FieldMap::new(fields, self.ctx.field_size_limit); - let id = self - .table - .push(HostFields::Owned { fields }) - .context("[new_fields] pushing fields")?; - Ok(Ok(id)) + Ok(self.table.push(fields)?) } - fn drop(&mut self, fields: Resource) -> wasmtime::Result<()> { + fn drop(&mut self, fields: Resource) -> wasmtime::Result<()> { self.table .delete(fields) .context("[drop_fields] deleting fields")?; Ok(()) } - fn get( - &mut self, - fields: Resource, - name: String, - ) -> wasmtime::Result>> { - let fields = get_fields(self.table, &fields).context("[fields_get] getting fields")?; + fn get(&mut self, fields: Resource, name: String) -> wasmtime::Result>> { + let fields = self.table.get(&fields)?; - let header = match hyper::header::HeaderName::from_bytes(name.as_bytes()) { + let header = match HeaderName::from_bytes(name.as_bytes()) { Ok(header) => header, Err(_) => return Ok(vec![]), }; - if !fields.as_ref().contains_key(&header) { + if !fields.contains_key(&header) { return Ok(vec![]); } let res = fields - .as_ref() .get_all(&header) .into_iter() .map(|val| val.as_bytes().to_owned()) @@ -160,121 +86,81 @@ impl types::HostFields for WasiHttpCtxView<'_> { Ok(res) } - fn has(&mut self, fields: Resource, name: String) -> wasmtime::Result { - let fields = get_fields(self.table, &fields).context("[fields_get] getting fields")?; + fn has(&mut self, fields: Resource, name: String) -> wasmtime::Result { + let fields = self.table.get(&fields)?; - match hyper::header::HeaderName::from_bytes(name.as_bytes()) { - Ok(header) => Ok(fields.as_ref().contains_key(&header)), + match HeaderName::from_bytes(name.as_bytes()) { + Ok(header) => Ok(fields.contains_key(&header)), Err(_) => Ok(false), } } fn set( &mut self, - fields: Resource, + fields: Resource, name: String, byte_values: Vec>, - ) -> wasmtime::Result> { - let header = match hyper::header::HeaderName::from_bytes(name.as_bytes()) { - Ok(header) => header, - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - }; + ) -> HeaderResult<()> { + let header = HeaderName::from_bytes(name.as_bytes())?; if self.hooks.is_forbidden_header(&header) { - return Ok(Err(types::HeaderError::Forbidden)); + return Err(types::HeaderError::Forbidden.into()); } let mut values = Vec::with_capacity(byte_values.len()); for value in byte_values { - match hyper::header::HeaderValue::from_bytes(&value) { - Ok(value) => values.push(value), - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - } + values.push(HeaderValue::from_bytes(&value)?); } - match get_fields_mut(self.table, &fields).context("[fields_set] getting mutable fields")? { - Ok(fields) => { - fields.remove_all(&header); - for value in values { - fields.append(&header, value)?; - } - Ok(Ok(())) - } - Err(e) => Ok(Err(e)), - } + let fields = self.table.get_mut(&fields)?; + fields.set(header, values)?; + Ok(()) } - fn delete( - &mut self, - fields: Resource, - name: String, - ) -> wasmtime::Result> { - let header = match hyper::header::HeaderName::from_bytes(name.as_bytes()) { - Ok(header) => header, - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - }; + fn delete(&mut self, fields: Resource, name: String) -> HeaderResult<()> { + let header = HeaderName::from_bytes(name.as_bytes())?; if self.hooks.is_forbidden_header(&header) { - return Ok(Err(types::HeaderError::Forbidden)); + return Err(types::HeaderError::Forbidden.into()); } - Ok(get_fields_mut(self.table, &fields)?.map(|fields| { - fields.remove_all(&header); - })) + let fields = self.table.get_mut(&fields)?; + fields.remove_all(header)?; + Ok(()) } fn append( &mut self, - fields: Resource, + fields: Resource, name: String, value: Vec, - ) -> wasmtime::Result> { - let header = match hyper::header::HeaderName::from_bytes(name.as_bytes()) { - Ok(header) => header, - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - }; + ) -> HeaderResult<()> { + let header = HeaderName::from_bytes(name.as_bytes())?; if self.hooks.is_forbidden_header(&header) { - return Ok(Err(types::HeaderError::Forbidden)); + return Err(types::HeaderError::Forbidden.into()); } - let value = match hyper::header::HeaderValue::from_bytes(&value) { - Ok(value) => value, - Err(_) => return Ok(Err(types::HeaderError::InvalidSyntax)), - }; + let value = HeaderValue::from_bytes(&value)?; - match get_fields_mut(self.table, &fields) - .context("[fields_append] getting mutable fields")? - { - Ok(fields) => { - fields.append(&header, value)?; - Ok(Ok(())) - } - Err(e) => Ok(Err(e)), - } + let fields = self.table.get_mut(&fields)?; + fields.append(header, value)?; + Ok(()) } - fn entries( - &mut self, - fields: Resource, - ) -> wasmtime::Result)>> { - Ok(get_fields(self.table, &fields)? - .as_ref() + fn entries(&mut self, fields: Resource) -> wasmtime::Result)>> { + Ok(self + .table + .get(&fields)? .iter() .map(|(name, value)| (name.as_str().to_owned(), value.as_bytes().to_owned())) .collect()) } - fn clone(&mut self, fields: Resource) -> wasmtime::Result> { - let fields = get_fields(self.table, &fields) - .context("[fields_clone] getting fields")? - .clone(); - - let id = self - .table - .push(HostFields::Owned { fields }) - .context("[fields_clone] pushing fields")?; - + fn clone(&mut self, fields: Resource) -> wasmtime::Result> { + let mut fields = self.table.get(&fields)?.clone(); + fields.set_mutable(self.ctx.field_size_limit); + let id = self.table.push(fields)?; Ok(id) } } @@ -306,22 +192,9 @@ impl types::HostIncomingRequest for WasiHttpCtxView<'_> { fn headers( &mut self, id: Resource, - ) -> wasmtime::Result> { - let _ = self.table.get(&id)?; - - fn get_fields(elem: &mut dyn Any) -> &mut FieldMap { - &mut elem.downcast_mut::().unwrap().headers - } - - let headers = self.table.push_child( - HostFields::Ref { - parent: id.rep(), - get_fields, - }, - &id, - )?; - - Ok(headers) + ) -> wasmtime::Result> { + let req = self.table.get(&id)?; + Ok(self.table.push(req.headers.clone())?) } fn consume( @@ -348,9 +221,10 @@ impl types::HostIncomingRequest for WasiHttpCtxView<'_> { impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { fn new( &mut self, - headers: Resource, + headers: Resource, ) -> wasmtime::Result> { - let headers = move_fields(self.table, headers)?; + let mut headers = self.table.delete(headers)?; + headers.set_immutable(); self.table .push(HostOutgoingRequest { @@ -379,7 +253,7 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { return Ok(Err(())); } - let size = match get_content_length(req.headers.as_ref()) { + let size = match get_content_length(&req.headers) { Ok(size) => size, Err(..) => return Ok(Err(())), }; @@ -504,27 +378,9 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { fn headers( &mut self, request: wasmtime::component::Resource, - ) -> wasmtime::Result> { - let _ = self - .table - .get(&request) - .context("[outgoing_request_headers] getting request")?; - - fn get_fields(elem: &mut dyn Any) -> &mut FieldMap { - &mut elem - .downcast_mut::() - .unwrap() - .headers - } - - let id = self.table.push_child( - HostFields::Ref { - parent: request.rep(), - get_fields, - }, - &request, - )?; - + ) -> wasmtime::Result> { + let req = self.table.get(&request)?; + let id = self.table.push(req.headers.clone())?; Ok(id) } } @@ -557,7 +413,7 @@ impl types::HostResponseOutparam for WasiHttpCtxView<'_> { &mut self, _id: Resource, _status: u16, - _headers: Resource, + _headers: Resource, ) -> HttpResult<()> { Err(HttpError::trap(format_err!("not implemented"))) } @@ -583,24 +439,9 @@ impl types::HostIncomingResponse for WasiHttpCtxView<'_> { fn headers( &mut self, response: Resource, - ) -> wasmtime::Result> { - let _ = self - .table - .get(&response) - .context("[incoming_response_headers] getting response")?; - - fn get_fields(elem: &mut dyn Any) -> &mut FieldMap { - &mut elem.downcast_mut::().unwrap().headers - } - - let id = self.table.push_child( - HostFields::Ref { - parent: response.rep(), - get_fields, - }, - &response, - )?; - + ) -> wasmtime::Result> { + let resp = self.table.get(&response)?; + let id = self.table.push(resp.headers.clone())?; Ok(id) } @@ -665,7 +506,7 @@ impl types::HostFutureTrailers for WasiHttpCtxView<'_> { remove_forbidden_headers(self.hooks, &mut fields); - let ts = self.table.push(HostFields::Owned { fields })?; + let ts = self.table.push(FieldMap::new_immutable(fields))?; Ok(Some(Ok(Ok(Some(ts))))) } @@ -705,9 +546,10 @@ impl types::HostIncomingBody for WasiHttpCtxView<'_> { impl types::HostOutgoingResponse for WasiHttpCtxView<'_> { fn new( &mut self, - headers: Resource, + headers: Resource, ) -> wasmtime::Result> { - let fields = move_fields(self.table, headers)?; + let mut fields = self.table.delete(headers)?; + fields.set_immutable(); let id = self.table.push(HostOutgoingResponse { status: http::StatusCode::OK, @@ -730,7 +572,7 @@ impl types::HostOutgoingResponse for WasiHttpCtxView<'_> { return Ok(Err(())); } - let size = match get_content_length(resp.headers.as_ref()) { + let size = match get_content_length(&resp.headers) { Ok(size) => size, Err(..) => return Ok(Err(())), }; @@ -770,22 +612,9 @@ impl types::HostOutgoingResponse for WasiHttpCtxView<'_> { fn headers( &mut self, id: Resource, - ) -> wasmtime::Result> { - // Trap if the outgoing-response doesn't exist. - let _ = self.table.get(&id)?; - - fn get_fields(elem: &mut dyn Any) -> &mut FieldMap { - let resp = elem.downcast_mut::().unwrap(); - &mut resp.headers - } - - Ok(self.table.push_child( - HostFields::Ref { - parent: id.rep(), - get_fields, - }, - &id, - )?) + ) -> wasmtime::Result> { + let resp = self.table.get(&id)?; + Ok(self.table.push(resp.headers.clone())?) } fn drop(&mut self, id: Resource) -> wasmtime::Result<()> { @@ -806,7 +635,6 @@ impl types::HostFutureIncomingResponse for WasiHttpCtxView<'_> { ) -> wasmtime::Result< Option, types::ErrorCode>, ()>>, > { - let field_size_limit = self.ctx.field_size_limit; let resp = self.table.get_mut(&id)?; match resp { @@ -827,17 +655,15 @@ impl types::HostFutureIncomingResponse for WasiHttpCtxView<'_> { Ok(Err(e)) => return Ok(Some(Ok(Err(e)))), }; - let (parts, body) = resp.resp.into_parts(); - - let mut headers = FieldMap::new(parts.headers, field_size_limit); - remove_forbidden_headers(self.hooks, &mut headers); + let (mut parts, body) = resp.resp.into_parts(); + remove_forbidden_headers(self.hooks, &mut parts.headers); + let headers = FieldMap::new_immutable(parts.headers); let resp = self.table.push(HostIncomingResponse { status: parts.status.as_u16(), headers, body: Some({ - let mut body = - HostIncomingBody::new(body, resp.between_bytes_timeout, field_size_limit); + let mut body = HostIncomingBody::new(body, resp.between_bytes_timeout); if let Some(worker) = resp.worker { body.retain_worker(worker); } @@ -878,7 +704,7 @@ impl types::HostOutgoingBody for WasiHttpCtxView<'_> { let body = self.table.delete(id)?; let ts = if let Some(ts) = ts { - Some(move_fields(self.table, ts)?) + Some(self.table.delete(ts)?) } else { None }; diff --git a/crates/wasi-http/src/p3/bindings.rs b/crates/wasi-http/src/p3/bindings.rs index 307d3ff3480a..2e7804ce72fe 100644 --- a/crates/wasi-http/src/p3/bindings.rs +++ b/crates/wasi-http/src/p3/bindings.rs @@ -30,7 +30,7 @@ mod generated { }); mod with { - pub type Fields = crate::p3::MaybeMutable; + pub type Fields = crate::FieldMap; pub type RequestOptions = crate::p3::MaybeMutable; } } diff --git a/crates/wasi-http/src/p3/body.rs b/crates/wasi-http/src/p3/body.rs index 79e27a4f4751..0d2da68dacd6 100644 --- a/crates/wasi-http/src/p3/body.rs +++ b/crates/wasi-http/src/p3/body.rs @@ -1,11 +1,11 @@ -use crate::p3::bindings::http::types::{ErrorCode, Fields, Trailers}; +use crate::FieldMap; +use crate::p3::bindings::http::types::{ErrorCode, Trailers}; use crate::p3::{WasiHttp, WasiHttpCtxView}; use bytes::Bytes; use core::iter; use core::num::NonZeroUsize; use core::pin::Pin; use core::task::{Context, Poll, ready}; -use http::HeaderMap; use http_body::Body as _; use http_body_util::combinators::UnsyncBoxBody; use std::any::{Any, TypeId}; @@ -259,7 +259,7 @@ impl StreamConsumer for UnlimitedGuestBodyConsumer { /// [http_body::Body] implementation for bodies originating in the guest. pub(crate) struct GuestBody { contents_rx: Option>>, - trailers_rx: Option>, ErrorCode>>>, + trailers_rx: Option>, ErrorCode>>>, content_length: Option, } @@ -364,7 +364,7 @@ impl http_body::Body for GuestBody { self.trailers_rx = None; match res { Ok(Ok(Some(trailers))) => Poll::Ready(Some(Ok(http_body::Frame::trailers( - Arc::unwrap_or_clone(trailers), + Arc::unwrap_or_clone(trailers).into(), )))), Ok(Ok(None)) => Poll::Ready(None), Ok(Err(err)) => Poll::Ready(Some(Err(err))), @@ -404,7 +404,7 @@ impl http_body::Body for GuestBody { /// [FutureConsumer] implementation for trailers originating in the guest. struct GuestTrailerConsumer { - tx: Option>, ErrorCode>>>, + tx: Option>, ErrorCode>>>, getter: fn(&mut T) -> WasiHttpCtxView<'_>, } @@ -520,9 +520,11 @@ where return Poll::Ready(Ok(StreamResult::Completed)); } Err(Ok(trailers)) => { - let trailers = (self.getter)(store.data_mut()) + let view = (self.getter)(store.data_mut()); + let trailers = FieldMap::new_immutable(trailers); + let trailers = view .table - .push(Fields::new_mutable(trailers)) + .push(trailers) .context("failed to push trailers to table")?; break 'result Ok(Some(trailers)); } diff --git a/crates/wasi-http/src/p3/host/handler.rs b/crates/wasi-http/src/p3/host/handler.rs index ffa2fea1baa6..ae49bfdfd842 100644 --- a/crates/wasi-http/src/p3/host/handler.rs +++ b/crates/wasi-http/src/p3/host/handler.rs @@ -1,3 +1,4 @@ +use crate::FieldMap; use crate::p3::bindings::http::client::{Host, HostWithStore}; use crate::p3::bindings::http::types::{ErrorCode, Request, Response}; use crate::p3::body::{Body, BodyExt as _}; @@ -97,7 +98,7 @@ impl HostWithStore for WasiHttp { }; let res = Response { status, - headers: Arc::new(headers), + headers: FieldMap::new_immutable(headers), body: Body::Host { body, result_tx: res_result_tx, diff --git a/crates/wasi-http/src/p3/host/types.rs b/crates/wasi-http/src/p3/host/types.rs index 6179bea36d26..aa48d265526c 100644 --- a/crates/wasi-http/src/p3/host/types.rs +++ b/crates/wasi-http/src/p3/host/types.rs @@ -1,3 +1,4 @@ +use crate::FieldMap; use crate::p3::bindings::clocks::monotonic_clock::Duration; use crate::p3::bindings::http::types::{ ErrorCode, FieldName, FieldValue, Fields, HeaderError, Headers, Host, HostFields, HostRequest, @@ -42,9 +43,15 @@ fn push_fields(table: &mut ResourceTable, fields: Fields) -> wasmtime::Result) -> wasmtime::Result { - table + let mut fields = table .delete(fields) - .context("failed to delete fields from table") + .context("failed to delete fields from table")?; + // When fields are passed by ownership to the host that flags them as + // immutable within `wasi:http`, and this semantically means that putting + // fields in a request, then getting them back out, will return an immutable + // view of the headers rather than mutable for example. + fields.set_immutable(); + Ok(fields) } fn get_request<'a>( @@ -179,24 +186,23 @@ impl FutureProducer for GuestBodyResultProducer { impl HostFields for WasiHttpCtxView<'_> { fn new(&mut self) -> wasmtime::Result> { - push_fields(self.table, Fields::new_mutable_default()) + push_fields(self.table, FieldMap::new_mutable(self.ctx.field_size_limit)) } fn from_list( &mut self, entries: Vec<(FieldName, FieldValue)>, ) -> HeaderResult> { - let mut fields = http::HeaderMap::default(); + let mut fields = FieldMap::new_mutable(self.ctx.field_size_limit); for (name, value) in entries { let name = name.parse().or(Err(HeaderError::InvalidSyntax))?; if self.hooks.is_forbidden_header(&name) { return Err(HeaderError::Forbidden.into()); } let value = parse_header_value(&name, value)?; - fields.append(name, value); + fields.append(name, value)?; } - let fields = push_fields(self.table, Fields::new_mutable(fields)) - .map_err(crate::p3::HeaderError::trap)?; + let fields = push_fields(self.table, fields).map_err(crate::p3::HeaderError::trap)?; Ok(fields) } @@ -224,7 +230,7 @@ impl HostFields for WasiHttpCtxView<'_> { name: FieldName, value: Vec, ) -> HeaderResult<()> { - let name = name.parse().or(Err(HeaderError::InvalidSyntax))?; + let name = name.parse().map_err(|_| HeaderError::InvalidSyntax)?; if self.hooks.is_forbidden_header(&name) { return Err(HeaderError::Forbidden.into()); } @@ -233,23 +239,16 @@ impl HostFields for WasiHttpCtxView<'_> { let value = parse_header_value(&name, value)?; values.push(value); } - let fields = get_fields_mut(self.table, &fields)?; - let fields = fields.get_mut().ok_or(HeaderError::Immutable)?; - fields.remove(&name); - for value in values { - fields.append(&name, value); - } + get_fields_mut(self.table, &fields)?.set(name, values)?; Ok(()) } fn delete(&mut self, fields: Resource, name: FieldName) -> HeaderResult<()> { - let name = name.parse().or(Err(HeaderError::InvalidSyntax))?; + let name = name.parse().map_err(|_| HeaderError::InvalidSyntax)?; if self.hooks.is_forbidden_header(&name) { return Err(HeaderError::Forbidden.into()); } - let fields = get_fields_mut(self.table, &fields)?; - let fields = fields.get_mut().ok_or(HeaderError::Immutable)?; - fields.remove(&name); + get_fields_mut(self.table, &fields)?.remove_all(name)?; Ok(()) } @@ -262,12 +261,9 @@ impl HostFields for WasiHttpCtxView<'_> { if self.hooks.is_forbidden_header(&name) { return Err(HeaderError::Forbidden.into()); } - let fields = get_fields_mut(self.table, &fields)?; - let fields = fields.get_mut().ok_or(HeaderError::Immutable)?; - let http::header::Entry::Occupied(entry) = fields.entry(name) else { - return Ok(Vec::default()); - }; - let (.., values) = entry.remove_entry_mult(); + let values = get_fields_mut(self.table, &fields)? + .remove_all(name)? + .into_iter(); Ok(values.map(|value| value.as_bytes().into()).collect()) } @@ -282,9 +278,7 @@ impl HostFields for WasiHttpCtxView<'_> { return Err(HeaderError::Forbidden.into()); } let value = parse_header_value(&name, value)?; - let fields = get_fields_mut(self.table, &fields)?; - let fields = fields.get_mut().ok_or(HeaderError::Immutable)?; - fields.append(name, value); + get_fields_mut(self.table, &fields)?.append(name, value)?; Ok(()) } @@ -301,8 +295,9 @@ impl HostFields for WasiHttpCtxView<'_> { } fn clone(&mut self, fields: Resource) -> wasmtime::Result> { - let fields = get_fields(self.table, &fields)?; - push_fields(self.table, Fields::new_mutable(Arc::clone(fields))) + let mut fields = get_fields(self.table, &fields)?.clone(); + fields.set_mutable(self.ctx.field_size_limit); + push_fields(self.table, fields) } fn drop(&mut self, fields: Resource) -> wasmtime::Result<()> { @@ -348,7 +343,7 @@ impl HostRequestWithStore for WasiHttp { scheme: None, authority: None, path_with_query: None, - headers: headers.into(), + headers, options: options.map(Into::into), body, }; @@ -496,7 +491,7 @@ impl HostRequest for WasiHttpCtxView<'_> { fn get_headers(&mut self, req: Resource) -> wasmtime::Result> { let Request { headers, .. } = get_request(self.table, &req)?; - push_fields(self.table, Fields::new_immutable(Arc::clone(headers))) + push_fields(self.table, headers.clone()) } } @@ -624,7 +619,7 @@ impl HostResponseWithStore for WasiHttp { let headers = delete_fields(table, headers)?; let res = Response { status: http::StatusCode::OK, - headers: headers.into(), + headers, body, }; let res = table @@ -687,7 +682,7 @@ impl HostResponse for WasiHttpCtxView<'_> { fn get_headers(&mut self, res: Resource) -> wasmtime::Result> { let Response { headers, .. } = get_response(self.table, &res)?; - push_fields(self.table, Fields::new_immutable(Arc::clone(headers))) + push_fields(self.table, headers.clone()) } } diff --git a/crates/wasi-http/src/p3/mod.rs b/crates/wasi-http/src/p3/mod.rs index bce694348e59..04e50b3a75a3 100644 --- a/crates/wasi-http/src/p3/mod.rs +++ b/crates/wasi-http/src/p3/mod.rs @@ -22,7 +22,7 @@ pub use request::{Request, RequestOptions}; pub use response::Response; use crate::p3::bindings::http::types::ErrorCode; -use crate::{DEFAULT_FORBIDDEN_HEADERS, WasiHttpCtx}; +use crate::{DEFAULT_FORBIDDEN_HEADERS, FieldMapError, WasiHttpCtx}; use bindings::http::{client, types}; use bytes::Bytes; use core::ops::Deref; @@ -39,6 +39,18 @@ pub(crate) type HttpError = TrappableError; pub(crate) type HeaderResult = Result; pub(crate) type HeaderError = TrappableError; +impl From for HeaderError { + fn from(e: FieldMapError) -> Self { + match e { + FieldMapError::Immutable => types::HeaderError::Immutable.into(), + FieldMapError::InvalidHeaderName => types::HeaderError::InvalidSyntax.into(), + // FIXME(WebAssembly/WASI#889): these ideally would map to an error + // code instead of trapping. + FieldMapError::TooManyFields | FieldMapError::TotalSizeTooBig => HeaderError::trap(e), + } + } +} + pub(crate) type RequestOptionsResult = Result; pub(crate) type RequestOptionsError = TrappableError; diff --git a/crates/wasi-http/src/p3/request.rs b/crates/wasi-http/src/p3/request.rs index 458919f56fd2..aa3ed706abbb 100644 --- a/crates/wasi-http/src/p3/request.rs +++ b/crates/wasi-http/src/p3/request.rs @@ -1,12 +1,12 @@ -use crate::get_content_length; use crate::p3::bindings::http::types::ErrorCode; use crate::p3::body::{Body, BodyExt as _, GuestBody}; use crate::p3::{HttpError, HttpResult, WasiHttpCtxView, WasiHttpView}; +use crate::{FieldMap, get_content_length}; use bytes::Bytes; use core::time::Duration; use http::header::HOST; use http::uri::{Authority, PathAndQuery, Scheme}; -use http::{HeaderMap, HeaderValue, Method, Uri}; +use http::{HeaderValue, Method, Uri}; use http_body_util::BodyExt as _; use http_body_util::combinators::UnsyncBoxBody; use std::sync::Arc; @@ -36,7 +36,7 @@ pub struct Request { /// The path and query of the request. pub path_with_query: Option, /// The request headers. - pub headers: Arc, + pub headers: FieldMap, /// Request options. pub options: Option>, /// Request body. @@ -55,7 +55,7 @@ impl Request { scheme: Option, authority: Option, path_with_query: Option, - headers: impl Into>, + headers: impl Into, options: Option>, body: impl Into>, ) -> ( @@ -119,7 +119,7 @@ impl Request { scheme, authority, path_and_query, - headers, + FieldMap::new_immutable(headers), None, body.map_err(Into::into).boxed_unsync(), ) @@ -156,7 +156,7 @@ impl Request { scheme, authority, path_with_query, - headers, + mut headers, options, body, } = self; @@ -207,9 +207,9 @@ impl Request { } } }; - let mut headers = Arc::unwrap_or_clone(headers); let mut store = store.as_context_mut(); - let WasiHttpCtxView { hooks, .. } = getter(store.data_mut()); + let WasiHttpCtxView { hooks, ctx, .. } = getter(store.data_mut()); + headers.set_mutable(ctx.field_size_limit); if hooks.set_host_header() { let host = if let Some(authority) = authority.as_ref() { HeaderValue::try_from(authority.as_str()) @@ -217,7 +217,7 @@ impl Request { } else { HeaderValue::from_static("") }; - headers.insert(HOST, host); + headers.append(HOST, host).map_err(HttpError::trap)?; } let scheme = match scheme { None => hooks.default_scheme().ok_or(ErrorCode::HttpProtocolError)?, @@ -236,7 +236,7 @@ impl Request { ErrorCode::HttpRequestUriInvalid })?; let mut req = http::Request::builder(); - *req.headers_mut().unwrap() = headers; + *req.headers_mut().unwrap() = headers.into(); let req = req .method(method) .uri(uri) @@ -534,7 +534,7 @@ mod tests { scheme.clone(), Some(Authority::from_static("example.com")), Some(PathAndQuery::from_static("/path?query=1")), - HeaderMap::new(), + FieldMap::default(), None, Full::new(Bytes::from_static(b"body")) .map_err(|x| match x {}) @@ -570,7 +570,7 @@ mod tests { Some(Scheme::HTTP), Some(Authority::from_static("example.com")), None, // <-- should fail, must be Some(_) when authority is set - HeaderMap::new(), + FieldMap::default(), None, Empty::new().map_err(|x| match x {}).boxed_unsync(), ); diff --git a/crates/wasi-http/src/p3/response.rs b/crates/wasi-http/src/p3/response.rs index 9c057138373a..6776faf0523f 100644 --- a/crates/wasi-http/src/p3/response.rs +++ b/crates/wasi-http/src/p3/response.rs @@ -1,12 +1,11 @@ -use crate::get_content_length; use crate::p3::bindings::http::types::ErrorCode; use crate::p3::body::{Body, GuestBody}; use crate::p3::{WasiHttpCtxView, WasiHttpView}; +use crate::{FieldMap, get_content_length}; use bytes::Bytes; -use http::{HeaderMap, StatusCode}; +use http::StatusCode; use http_body_util::BodyExt as _; use http_body_util::combinators::UnsyncBoxBody; -use std::sync::Arc; use wasmtime::AsContextMut; use wasmtime::error::Context as _; @@ -15,7 +14,7 @@ pub struct Response { /// The status of the response. pub status: StatusCode, /// The headers of the response. - pub headers: Arc, + pub headers: FieldMap, /// Response body. pub(crate) body: Body, } @@ -31,7 +30,7 @@ impl TryFrom for http::Response { }: Response, ) -> Result { let mut res = http::Response::builder().status(status); - *res.headers_mut().unwrap() = Arc::unwrap_or_clone(headers); + *res.headers_mut().unwrap() = headers.into(); res.body(body) } } @@ -106,7 +105,7 @@ impl Response { let wasi_response = Response { status: parts.status, - headers: Arc::new(parts.headers), + headers: FieldMap::new_immutable(parts.headers), body: Body::Host { body: body.map_err(Into::into).boxed_unsync(), result_tx, diff --git a/crates/wasi-http/tests/all/p2.rs b/crates/wasi-http/tests/all/p2.rs index 7de12249ed4c..382cf0c03965 100644 --- a/crates/wasi-http/tests/all/p2.rs +++ b/crates/wasi-http/tests/all/p2.rs @@ -19,7 +19,7 @@ use wasmtime_wasi_http::{ io::TokioIo, p2::bindings::http::types::{ErrorCode, Scheme}, p2::body::HyperOutgoingBody, - p2::types::{self, HostFutureIncomingResponse, IncomingResponse, OutgoingRequestConfig}, + p2::types::{HostFutureIncomingResponse, IncomingResponse, OutgoingRequestConfig}, p2::{HttpResult, WasiHttpCtxView, WasiHttpHooks, WasiHttpView}, }; @@ -620,10 +620,10 @@ async fn wasi_http_no_trap_on_early_drop() -> Result<()> { #[test_log::test(tokio::test)] async fn wasi_http_fields_limit_incoming_request() -> Result<()> { - use crate::p2::types::FieldSizeLimitError; use http::{HeaderName, HeaderValue, Request}; use http_body_util::combinators::BoxBody; use hyper::Error; + use wasmtime_wasi_http::FieldMapError; fn request_with_header_size(uri: &str, total: usize) -> Request> { let mut builder = hyper::Request::builder().uri(uri).method(http::Method::GET); @@ -684,7 +684,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { .await .err() .expect("new_fields exceeding the size limit"); - assert!(err.downcast_ref::().is_some()); + assert!(err.downcast_ref::().is_some()); let resp = run_wasi_http( test_programs_artifacts::P2_API_PROXY_COMPONENT, @@ -708,7 +708,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { .await .err() .expect("run_wasi_http should give error"); - assert!(err.downcast_ref::().is_some()); + assert!(err.downcast_ref::().is_some()); Ok(()) } diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index db87e1bca3fb..a3dad513e517 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -2312,44 +2312,59 @@ start a print 1234 #[test] fn p2_cli_http_headers() -> Result<()> { - for test in ["append", "append-empty", "append-same", "append-same-empty"] { + let td = tempfile::TempDir::new()?; + let cwasm = td.path().join("http_headers.cwasm"); + let cwasm = cwasm.to_str().unwrap(); + run_wasmtime(&["compile", P2_CLI_HTTP_HEADERS_COMPONENT, "-o", cwasm])?; + for wasi in ["p2", "p3"] { + for test in ["append", "append-empty", "append-same", "append-same-empty"] { + let err = run_wasmtime(&[ + "run", + "-Shttp,p3", + "-Smax-http-fields-size=1048576", + "--allow-precompiled", + cwasm, + &format!("{wasi}-{test}"), + ]) + .unwrap_err(); + assert!( + err.to_string() + .contains("total size of fields exceeds limit") + || err.to_string().contains("too many fields in the field map"), + "bad error message: {err:?}" + ); + + // gated by default too + let err = run_wasmtime(&[ + "run", + "-Shttp,p3", + "--allow-precompiled", + cwasm, + &format!("{wasi}-{test}"), + ]) + .unwrap_err(); + assert!( + err.to_string() + .contains("total size of fields exceeds limit"), + "bad error message: {err:?}" + ); + } + + // With an extremely large limit Wasmtime still shouldn't panic. let err = run_wasmtime(&[ "run", - "-Shttp", - "-Smax-http-fields-size=1048576", - P2_CLI_HTTP_HEADERS_COMPONENT, - test, + "-Shttp,p3", + &format!("-Smax-http-fields-size={}", 1 << 30), + "--allow-precompiled", + cwasm, + &format!("{wasi}-append"), ]) .unwrap_err(); assert!( - err.to_string() - .contains("Field size limit 1048576 exceeded") - || err.to_string().contains("max size reached"), - "bad error message: {err:?}" - ); - - // gated by default too - let err = - run_wasmtime(&["run", "-Shttp", P2_CLI_HTTP_HEADERS_COMPONENT, test]).unwrap_err(); - assert!( - err.to_string().contains("Field size limit"), + err.to_string().contains("too many fields in the field map"), "bad error message: {err:?}" ); } - - // With an extremely large limit Wasmtime still shouldn't panic. - let err = run_wasmtime(&[ - "run", - "-Shttp", - &format!("-Smax-http-fields-size={}", 1 << 30), - P2_CLI_HTTP_HEADERS_COMPONENT, - "append", - ]) - .unwrap_err(); - assert!( - err.to_string().contains("max size reached"), - "bad error message: {err:?}" - ); Ok(()) }