From 6724c4061c186b7e69d3b40cf876042c5246f8af Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 6 Oct 2026 13:47:12 -0700 Subject: [PATCH] Limit size of string fields in http request This PR is motivated by string fields in wasip2 `outgoing-request` and waspi3 `request` resources - specifically, the `scheme` (via the `other` variant), `method` (`other` variant), `authority`, and `path_and_query`, being able to use a host allocation of up to `hostcall-fuel` (128M by default) size strings. This PR limits the sum those by default to 16k per request, which I picked a reasonable limit given that many http implementations limit the sum of all of these strings plus the headers anywhere from 8k (akamai), 32k (nginx), to 128k (fastly, cloudflare). The limit is tunable in the construction of `WasiHttpCtx`, the C API, and the wasmtime cli (with `-Smax-http-request-strings-size=`). The limits apply to all requests that come off the wire, as well as those manipulated by the guest, so that we keep the invariant that the guest can proxy (forward) any request it is given. Requests that come off the wire exceeding the limit get rejected with 400 BAD_REQUEST. Along the way, the validation of the host header was made stricter when the request doesn't already have an authority - it now must parse as an `http::uri::Authority`. These changes may end up causing embeddings to reject some requests they previously accepted, but they should be able to tweak the limit to continue accepting any valid requests. New tests demonstrate this new limit on wasip2 and wasip3. There are some incidental changes to the crate's public api for wasip3, and wasip2's HostOutgoingRequest can no longer be constructed outside the crate through the struct fields, but that one didn't strike me as an intentional aspect of the public API. If there are embedder depending on that, we can make all of the new machinery validation machinery pub, but I chose to keep it as an internal implementation detail. Also, this PR noticed that the http fields size limit wasn't settable in the C API, so that setting was added as well. --- crates/c-api/include/wasmtime/_store_class.hh | 31 +++ crates/c-api/include/wasmtime/store.h | 33 +++ crates/c-api/src/store.rs | 34 +++ crates/c-api/tests/wasip2.cc | 23 ++ crates/cli-flags/src/lib.rs | 6 + crates/test-programs/src/bin/p2_api_proxy.rs | 34 ++- .../src/bin/p2_cli_http_request_strings.rs | 25 ++ .../src/bin/p3_cli_http_request_strings.rs | 36 +++ crates/wasi-http/src/ctx.rs | 16 ++ crates/wasi-http/src/handler.rs | 19 +- crates/wasi-http/src/lib.rs | 2 + crates/wasi-http/src/p2/types.rs | 65 ++++-- crates/wasi-http/src/p2/types_impl.rs | 49 ++-- crates/wasi-http/src/p3/host/types.rs | 57 +++-- crates/wasi-http/src/p3/request.rs | 93 +++++++- crates/wasi-http/src/request_strings.rs | 215 ++++++++++++++++++ crates/wasi-http/tests/all/p2.rs | 142 ++++++++++++ crates/wasi-http/tests/all/p3/mod.rs | 3 +- src/common.rs | 3 + tests/all/cli_tests.rs | 187 +++++++++++++++ 20 files changed, 999 insertions(+), 74 deletions(-) create mode 100644 crates/test-programs/src/bin/p2_cli_http_request_strings.rs create mode 100644 crates/test-programs/src/bin/p3_cli_http_request_strings.rs create mode 100644 crates/wasi-http/src/request_strings.rs diff --git a/crates/c-api/include/wasmtime/_store_class.hh b/crates/c-api/include/wasmtime/_store_class.hh index 96029d106a2c..9cb97d0ce413 100644 --- a/crates/c-api/include/wasmtime/_store_class.hh +++ b/crates/c-api/include/wasmtime/_store_class.hh @@ -152,6 +152,37 @@ public: } #endif // WASMTIME_FEATURE_WASI +#ifdef WASMTIME_FEATURE_WASI_HTTP + /// Initializes the WASI HTTP state used by this store. + void set_wasi_http() { wasmtime_context_set_wasi_http(ptr); } + + /// Sets the maximum size, in bytes, of each WASI HTTP `fields` resource + /// (headers and trailers). + /// + /// Fails if `set_wasi_http` has not been called on this store. + Result set_wasi_http_field_size_limit(size_t limit) { + auto *error = wasmtime_context_set_wasi_http_field_size_limit(ptr, limit); + if (error != nullptr) { + return Error(error); + } + return std::monostate(); + } + + /// Sets the maximum combined size, in bytes, of a WASI HTTP request's + /// method, scheme, authority, and path-with-query strings. + /// + /// Fails if `set_wasi_http` has not been called on this store. + Result + set_wasi_http_request_strings_size_limit(size_t limit) { + auto *error = + wasmtime_context_set_wasi_http_request_strings_size_limit(ptr, limit); + if (error != nullptr) { + return Error(error); + } + return std::monostate(); + } +#endif // WASMTIME_FEATURE_WASI_HTTP + /// Configures this store's epoch deadline to be the specified number of /// ticks beyond the engine's current epoch. /// diff --git a/crates/c-api/include/wasmtime/store.h b/crates/c-api/include/wasmtime/store.h index 668fe30bc86e..664bfff84d23 100644 --- a/crates/c-api/include/wasmtime/store.h +++ b/crates/c-api/include/wasmtime/store.h @@ -208,6 +208,39 @@ wasmtime_context_set_wasi(wasmtime_context_t *context, wasi_config_t *wasi); WASM_API_EXTERN void wasmtime_context_set_wasi_http(wasmtime_context_t *context); +/** + * \brief Sets the maximum size, in bytes, of each WASI HTTP `fields` resource + * (headers and trailers). + * + * This is roughly a limit on the in-memory representation of the fields, so it + * needs to be larger than their size on the wire. Operations that would + * exceed it fail. Defaults to 128 KiB. + * + * Returns an error if #wasmtime_context_set_wasi_http has not been called on + * this store. Calling #wasmtime_context_set_wasi_http again resets the limit to + * the default. + */ +WASM_API_EXTERN wasmtime_error_t * +wasmtime_context_set_wasi_http_field_size_limit(wasmtime_context_t *context, + size_t limit); + +/** + * \brief Sets the maximum combined size, in bytes, of a WASI HTTP request's + * method, scheme, authority, and path-with-query strings. + * + * Built-in methods (`GET`, `POST`, ...) and the `http`/`https` schemes don't + * count toward the limit. Guest setters that would exceed it return an error, + * and incoming requests that exceed it are rejected before reaching the guest. + * Defaults to 16 KiB. + * + * Returns an error if #wasmtime_context_set_wasi_http has not been called on + * this store. Calling #wasmtime_context_set_wasi_http again resets the limit to + * the default. + */ +WASM_API_EXTERN wasmtime_error_t * +wasmtime_context_set_wasi_http_request_strings_size_limit( + wasmtime_context_t *context, size_t limit); + #endif // WASMTIME_FEATURE_WASI_HTTP /** diff --git a/crates/c-api/src/store.rs b/crates/c-api/src/store.rs index 3b3c5140d0f8..955bc17e9acb 100644 --- a/crates/c-api/src/store.rs +++ b/crates/c-api/src/store.rs @@ -248,6 +248,40 @@ pub extern "C" fn wasmtime_context_set_wasi_http(mut context: WasmtimeStoreConte context.data_mut().wasi_http = Some(wasmtime_wasi_http::WasiHttpCtx::new()); } +#[cfg(feature = "wasi-http")] +fn with_wasi_http( + mut context: WasmtimeStoreContextMut<'_>, + f: impl FnOnce(&mut wasmtime_wasi_http::WasiHttpCtx), +) -> Option> { + match context.data_mut().wasi_http.as_mut() { + Some(http) => { + f(http); + None + } + None => Some(Box::new(wasmtime_error_t::from(wasmtime::format_err!( + "wasi-http context not set; call `wasmtime_context_set_wasi_http` first" + )))), + } +} + +#[cfg(feature = "wasi-http")] +#[unsafe(no_mangle)] +pub extern "C" fn wasmtime_context_set_wasi_http_field_size_limit( + context: WasmtimeStoreContextMut<'_>, + limit: usize, +) -> Option> { + with_wasi_http(context, |http| http.set_field_size_limit(limit)) +} + +#[cfg(feature = "wasi-http")] +#[unsafe(no_mangle)] +pub extern "C" fn wasmtime_context_set_wasi_http_request_strings_size_limit( + context: WasmtimeStoreContextMut<'_>, + limit: usize, +) -> Option> { + with_wasi_http(context, |http| http.set_request_strings_size_limit(limit)) +} + #[unsafe(no_mangle)] #[cfg(feature = "gc")] pub extern "C" fn wasmtime_context_gc( diff --git a/crates/c-api/tests/wasip2.cc b/crates/c-api/tests/wasip2.cc index 0ab13fb5dc23..493a03d1998b 100644 --- a/crates/c-api/tests/wasip2.cc +++ b/crates/c-api/tests/wasip2.cc @@ -44,3 +44,26 @@ TEST(wasip2, smoke) { linker.add_wasip2().unwrap(); linker.instantiate(context, component).unwrap(); } + +#ifdef WASMTIME_FEATURE_WASI_HTTP +TEST(wasip2, http_limits) { + wasmtime::Engine engine; + wasmtime::Store store(engine); + auto context = store.context(); + + // Setting limits before the WASI HTTP context exists is an error. + auto err = context.set_wasi_http_field_size_limit(1024); + EXPECT_FALSE(err); + EXPECT_NE(err.err().message().find("wasmtime_context_set_wasi_http"), + std::string::npos); + err = context.set_wasi_http_request_strings_size_limit(1024); + EXPECT_FALSE(err); + EXPECT_NE(err.err().message().find("wasmtime_context_set_wasi_http"), + std::string::npos); + + context.set_wasi(wasmtime::WasiConfig()).unwrap(); + context.set_wasi_http(); + context.set_wasi_http_field_size_limit(1024).unwrap(); + context.set_wasi_http_request_strings_size_limit(1024).unwrap(); +} +#endif // WASMTIME_FEATURE_WASI_HTTP diff --git a/crates/cli-flags/src/lib.rs b/crates/cli-flags/src/lib.rs index c551fc30b7a3..d3185cace10c 100644 --- a/crates/cli-flags/src/lib.rs +++ b/crates/cli-flags/src/lib.rs @@ -637,6 +637,12 @@ wasmtime_option_group! { /// `fields` resource (aka `headers` and `trailers`). `fields` methods /// which cause the contents to exceed this size limit will trap. pub max_http_fields_size: Option, + /// Maximum combined size, in bytes, of a wasi-http request's method, + /// scheme, authority, and path-with-query strings. Built-in methods + /// and the `http`/`https` schemes don't count. Guest setters that + /// would exceed this return an error, and `wasmtime serve` answers + /// incoming requests that exceed it with `400 Bad Request`. + pub max_http_request_strings_size: Option, } enum Wasi { diff --git a/crates/test-programs/src/bin/p2_api_proxy.rs b/crates/test-programs/src/bin/p2_api_proxy.rs index 3e2706cdfb8b..e7c7761eb4c0 100644 --- a/crates/test-programs/src/bin/p2_api_proxy.rs +++ b/crates/test-programs/src/bin/p2_api_proxy.rs @@ -1,6 +1,7 @@ use anyhow::{Context, Result}; use test_programs::wasi::http::types::{ - Headers, IncomingRequest, Method, OutgoingBody, OutgoingResponse, ResponseOutparam, + Fields, Headers, IncomingRequest, Method, OutgoingBody, OutgoingRequest, OutgoingResponse, + ResponseOutparam, Scheme, }; struct T; @@ -40,6 +41,11 @@ impl test_programs::proxy::exports::wasi::http::incoming_handler::Guest for T { response_for(r, outparam); return; } + (Method::Get, Some("/rs")) => { + let r = request_strings_handler(&request); + response_for(r, outparam); + return; + } _ => {} } @@ -141,3 +147,29 @@ fn new_fields_handler(request: IncomingRequest) -> Result<()> { Ok(()) } + +/// `/rs` with header `sets: =,...`: on a single fresh outgoing +/// request, set each `field` in order to a valid string of exactly `len` bytes. +/// +/// The path is kept short, and the sets are passed in a header, so that the +/// incoming request itself stays well within a small request strings limit. +fn request_strings_handler(request: &IncomingRequest) -> Result<()> { + let sets = request.headers().get("sets"); + let sets = std::str::from_utf8(sets.first().context("expect a `sets` header")?)?; + let req = OutgoingRequest::new(Fields::new()); + for set in sets.split(',') { + let (field, len) = set + .split_once('=') + .context("expect sets: =,...")?; + let len: usize = len.parse().context("expect len to parse as number")?; + let result = match field { + "method" => req.set_method(&Method::Other("X".repeat(len))), + "path" => req.set_path_with_query(Some(&format!("/{}", "a".repeat(len - 1)))), + "scheme" => req.set_scheme(Some(&Scheme::Other("x".repeat(len)))), + "authority" => req.set_authority(Some(&"a".repeat(len))), + other => anyhow::bail!("unknown field {other:?}"), + }; + result.map_err(|()| anyhow::anyhow!("failed to set {field} of length {len}"))?; + } + Ok(()) +} diff --git a/crates/test-programs/src/bin/p2_cli_http_request_strings.rs b/crates/test-programs/src/bin/p2_cli_http_request_strings.rs new file mode 100644 index 000000000000..c1c08d2b6cf3 --- /dev/null +++ b/crates/test-programs/src/bin/p2_cli_http_request_strings.rs @@ -0,0 +1,25 @@ +use wasip2::http::types::{Fields, Method, OutgoingRequest, Scheme}; + +/// Usage: `=...` where field is one of `method`, `path`, `scheme`, +/// or `authority`. +/// +/// Sets each field, in order, on a single request to a valid string of exactly +/// `len` bytes, printing `ok` or `error received` for each. +fn main() { + let req = OutgoingRequest::new(Fields::new()); + for arg in std::env::args().skip(1) { + let (field, len) = arg.split_once('=').expect("expected ="); + let len: usize = len.parse().expect("len must be a number"); + let result = match field { + "method" => req.set_method(&Method::Other("X".repeat(len))), + "path" => req.set_path_with_query(Some(&format!("/{}", "a".repeat(len - 1)))), + "scheme" => req.set_scheme(Some(&Scheme::Other("x".repeat(len)))), + "authority" => req.set_authority(Some(&"a".repeat(len))), + other => panic!("unknown field {other:?}"), + }; + match result { + Ok(()) => println!("ok"), + Err(()) => println!("error received"), + } + } +} diff --git a/crates/test-programs/src/bin/p3_cli_http_request_strings.rs b/crates/test-programs/src/bin/p3_cli_http_request_strings.rs new file mode 100644 index 000000000000..91e6edd7b685 --- /dev/null +++ b/crates/test-programs/src/bin/p3_cli_http_request_strings.rs @@ -0,0 +1,36 @@ +use test_programs::p3::wasi::http::types::{Fields, Method, Request, Scheme}; +use test_programs::p3::wit_future; + +struct Component; + +test_programs::p3::export!(Component); + +/// Usage: `=...` where field is one of `method`, `path`, `scheme`, +/// or `authority`. +/// +/// Sets each field, in order, on a single request to a valid string of exactly +/// `len` bytes, printing `ok` or `error received` for each. +impl test_programs::p3::exports::wasi::cli::run::Guest for Component { + async fn run() -> Result<(), ()> { + let (_trailers_tx, trailers_rx) = wit_future::new(|| Ok(None)); + let (req, _transmit) = Request::new(Fields::new(), None, trailers_rx, None); + for arg in std::env::args().skip(1) { + let (field, len) = arg.split_once('=').expect("expected ="); + let len: usize = len.parse().expect("len must be a number"); + let result = match field { + "method" => req.set_method(&Method::Other("X".repeat(len))), + "path" => req.set_path_with_query(Some(&format!("/{}", "a".repeat(len - 1)))), + "scheme" => req.set_scheme(Some(&Scheme::Other("x".repeat(len)))), + "authority" => req.set_authority(Some(&"a".repeat(len))), + other => panic!("unknown field {other:?}"), + }; + match result { + Ok(()) => println!("ok"), + Err(()) => println!("error received"), + } + } + Ok(()) + } +} + +fn main() {} diff --git a/crates/wasi-http/src/ctx.rs b/crates/wasi-http/src/ctx.rs index 48bf04416088..076bef0394d0 100644 --- a/crates/wasi-http/src/ctx.rs +++ b/crates/wasi-http/src/ctx.rs @@ -120,11 +120,15 @@ pub struct WasiHttpCtxView<'a> { /// completely full `HeaderMap` doesn't break the bank in terms of memory /// consumption. const DEFAULT_FIELD_SIZE_LIMIT: usize = 128 * 1024; +/// Default limit on the combined size of a request's method, scheme, +/// authority, and path-with-query strings, to limit host memory use. +const DEFAULT_REQUEST_STRINGS_SIZE_LIMIT: usize = 16 * 1024; /// Capture the state necessary for use in the wasi-http API implementation. #[derive(Debug, Clone)] pub struct WasiHttpCtx { pub(crate) field_size_limit: usize, + pub(crate) request_strings_size_limit: usize, } impl WasiHttpCtx { @@ -132,6 +136,7 @@ impl WasiHttpCtx { pub fn new() -> Self { Self { field_size_limit: DEFAULT_FIELD_SIZE_LIMIT, + request_strings_size_limit: DEFAULT_REQUEST_STRINGS_SIZE_LIMIT, } } @@ -145,6 +150,17 @@ impl WasiHttpCtx { pub fn set_field_size_limit(&mut self, limit: usize) { self.field_size_limit = limit; } + + /// Set the maximum combined size, in bytes, of a request's method, + /// scheme, authority, and path-with-query strings. + /// + /// Built-in methods (`GET`, `POST`, ...) and schemes (`http`, `https`) + /// don't count toward this limit. Guest setters which would exceed the + /// limit return an error, and incoming requests which exceed it are + /// rejected with a `400 Bad Request` before reaching the guest. + pub fn set_request_strings_size_limit(&mut self, limit: usize) { + self.request_strings_size_limit = limit; + } } impl Default for WasiHttpCtx { diff --git a/crates/wasi-http/src/handler.rs b/crates/wasi-http/src/handler.rs index 47990c2b7d53..f1dd32a5e476 100644 --- a/crates/wasi-http/src/handler.rs +++ b/crates/wasi-http/src/handler.rs @@ -1000,9 +1000,22 @@ impl<'a, T: Send> Prepared<'a, T> { Proxy::P3(guest) => { let (request, body) = request.into_parts(); let request = http::Request::from_parts(request, body); - let hooks = view(store.data_mut()).hooks; - let (request, request_io_result) = p3::Request::from_http(hooks, request); - let request = view(store.data_mut()).table.push(request)?; + let cx = view(store.data_mut()); + let (request, request_io_result) = match p3::Request::from_http( + cx.ctx, cx.hooks, request, + ) { + Ok(pair) => pair, + Err(e) => { + // As in the p2 case below, the request never + // reaches the guest, so report the failure through + // `tx`. + _ = tx.send(Err(e)); + wasmtime::bail!( + "request was rejected before it could be turned into a guest request" + ); + } + }; + let request = cx.table.push(request)?; Ok(Prepared::P3 { tx, diff --git a/crates/wasi-http/src/lib.rs b/crates/wasi-http/src/lib.rs index 02b61743a6e5..06ad7ffe274a 100644 --- a/crates/wasi-http/src/lib.rs +++ b/crates/wasi-http/src/lib.rs @@ -29,6 +29,8 @@ pub mod p2; #[cfg(feature = "p3")] pub mod p3; mod request_options; +#[cfg(any(feature = "p2", feature = "p3"))] +mod request_strings; pub use ctx::*; #[cfg(feature = "default-send-request")] diff --git a/crates/wasi-http/src/p2/types.rs b/crates/wasi-http/src/p2/types.rs index c6c13c179cab..e60d16d8c771 100644 --- a/crates/wasi-http/src/p2/types.rs +++ b/crates/wasi-http/src/p2/types.rs @@ -9,8 +9,8 @@ use crate::{Error, ErrorResponse, FieldMap, WasiHttpCtxView}; use bytes::Bytes; use http_body_util::BodyExt; use hyper::body::Body; -use wasmtime::Result; use wasmtime::component::Resource; +use wasmtime::error::{Context, Result}; use wasmtime_wasi::p2::Pollable; use wasmtime_wasi::runtime::AbortOnDropJoinHandle; @@ -66,7 +66,6 @@ pub struct HostIncomingRequest { pub(crate) uri: http::uri::Uri, pub(crate) headers: FieldMap, pub(crate) scheme: Scheme, - pub(crate) authority: String, /// The body of the incoming request. pub body: Option, } @@ -83,28 +82,24 @@ impl WasiHttpCtxView<'_> { B::Error: Into, { let (parts, body) = req.into_parts(); + let parts = normalize_authority(parts, &scheme) + .with_context(|| ErrorResponse::new(http::StatusCode::BAD_REQUEST))?; let body = body.map_err(Into::into).boxed_unsync(); let body = HostIncomingBody::new(body); - let authority = match parts.uri.authority() { - Some(authority) => authority.to_string(), - None => match parts.headers.get(http::header::HOST) { - Some(host) => host.to_str()?.to_string(), - None => { - return Err(wasmtime::Error::msg( - "invalid HTTP request missing authority in URI and host header", - ) - .context(ErrorResponse::new(http::StatusCode::BAD_REQUEST))); - } - }, - }; + let mut validator = crate::request_strings::RequestStringsValidator::new(self.ctx); + validator.host_parts( + &parts.method, + parts.uri.scheme(), + parts.uri.authority(), + parts.uri.path_and_query(), + )?; let headers = FieldMap::new_immutable(self.hooks, parts.headers); let req = HostIncomingRequest { method: parts.method, uri: parts.uri, headers, - authority, scheme, body: Some(body), }; @@ -112,6 +107,44 @@ impl WasiHttpCtxView<'_> { } } +/// Ensure `parts.uri` has an authority, taking it from the `Host` header if +/// necessary. A URI with an authority must also have a scheme, so `scheme` +/// fills it in when the URI lacks one. +/// +/// All failures in this function get propogated as a BAD_REQUEST error from the +/// call site. +fn normalize_authority( + parts: http::request::Parts, + scheme: &Scheme, +) -> Result { + if parts.uri.authority().is_some() { + return Ok(parts); + } + if parts.headers.get(http::header::HOST).is_none() { + return Err(wasmtime::Error::msg( + "invalid HTTP request missing authority in URI and host header", + )); + } + let host: http::uri::Authority = parts + .headers + .get(http::header::HOST) + .unwrap() + .to_str()? + .parse()?; + let mut parts = parts; + let mut uri = parts.uri.into_parts(); + uri.authority = Some(host); + if uri.scheme.is_none() { + uri.scheme = Some(match scheme { + Scheme::Http => http::uri::Scheme::HTTP, + Scheme::Https => http::uri::Scheme::HTTPS, + Scheme::Other(s) => s.parse()?, + }); + } + parts.uri = http::uri::Uri::from_parts(uri)?; + Ok(parts) +} + /// The concrete type behind a `wasi:http/types.response-outparam` resource. pub struct HostResponseOutparam { /// The callback sending a response. @@ -202,6 +235,8 @@ pub struct HostOutgoingRequest { pub headers: FieldMap, /// The request body. pub body: Option, + /// Accounting for the size of the method, scheme, authority, and path. + pub(crate) strings: crate::request_strings::RequestStringsValidator, } /// The concrete type behind a `wasi:http/types.incoming-response` resource. diff --git a/crates/wasi-http/src/p2/types_impl.rs b/crates/wasi-http/src/p2/types_impl.rs index 73689f37bc6a..cfeeaa2e6ac2 100644 --- a/crates/wasi-http/src/p2/types_impl.rs +++ b/crates/wasi-http/src/p2/types_impl.rs @@ -9,7 +9,6 @@ use crate::p2::types::{ use crate::p2::{HeaderError, HeaderResult, HttpError, HttpResult}; use crate::{FieldMap, WasiHttpCtxView, get_content_length}; use http::HeaderName; -use std::str::FromStr; use wasmtime::component::Resource; use wasmtime::{error::Context as _, format_err}; use wasmtime_wasi::p2::{DynInputStream, DynOutputStream, DynPollable}; @@ -155,7 +154,12 @@ impl types::HostIncomingRequest for WasiHttpCtxView<'_> { } fn authority(&mut self, id: Resource) -> wasmtime::Result> { let req = self.table.get(&id)?; - Ok(Some(req.authority.clone())) + let a = req + .uri + .authority() + .expect("authority validated at creation") + .to_string(); + Ok(Some(a)) } fn headers( @@ -208,6 +212,7 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { headers, scheme: None, body: None, + strings: crate::request_strings::RequestStringsValidator::new(self.ctx), }) .context("[new_outgoing_request] pushing request") } @@ -263,10 +268,13 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { ) -> wasmtime::Result> { let req = self.table.get_mut(&request)?; - if let Method::Other(s) = &method { - if let Err(_) = http::Method::from_str(s) { - return Ok(Err(())); + match &method { + Method::Other(s) => { + if req.strings.set_other_method(s).is_err() { + return Ok(Err(())); + } } + _ => req.strings.set_builtin_method(), } req.method = method; @@ -288,10 +296,12 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { ) -> wasmtime::Result> { let req = self.table.get_mut(&request)?; - if let Some(s) = path_with_query.as_ref() { - if crate::parse_path_with_query(s).is_none() { - return Ok(Err(())); - } + if req + .strings + .set_path_with_query(path_with_query.as_deref()) + .is_err() + { + return Ok(Err(())); } req.path_with_query = path_with_query; @@ -313,10 +323,13 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { ) -> wasmtime::Result> { let req = self.table.get_mut(&request)?; - if let Some(types::Scheme::Other(s)) = scheme.as_ref() { - if let Err(_) = http::uri::Scheme::from_str(s.as_str()) { - return Ok(Err(())); + match &scheme { + Some(types::Scheme::Other(s)) => { + if req.strings.set_other_scheme(s).is_err() { + return Ok(Err(())); + } } + _ => req.strings.set_builtin_scheme(), } req.scheme = scheme; @@ -340,14 +353,10 @@ impl types::HostOutgoingRequest for WasiHttpCtxView<'_> { // Match p3: reject empty / non-numeric / out-of-range ports that // `http::uri::Authority` alone would accept (see crate::parse_authority). - if let Some(s) = authority { - let Ok(parsed) = crate::parse_authority(s) else { - return Ok(Err(())); - }; - req.authority = Some(parsed.as_str().into()); - } else { - req.authority = None; - } + let Ok(parsed) = req.strings.set_authority(authority) else { + return Ok(Err(())); + }; + req.authority = parsed.map(|a| a.as_str().into()); Ok(Ok(())) } diff --git a/crates/wasi-http/src/p3/host/types.rs b/crates/wasi-http/src/p3/host/types.rs index a8c86b5129d3..28fe7939a654 100644 --- a/crates/wasi-http/src/p3/host/types.rs +++ b/crates/wasi-http/src/p3/host/types.rs @@ -239,6 +239,7 @@ where headers, options: options.map(Into::into), body, + strings: crate::request_strings::RequestStringsValidator::new(cx.ctx), }; let req = cx .table @@ -291,8 +292,18 @@ impl HostRequest for WasiHttpCtxView<'_> { method: Method, ) -> wasmtime::Result> { let req = get_request_mut(self.table, &req)?; - let Ok(method) = method.try_into() else { - return Ok(Err(())); + let method = match method { + Method::Other(m) => match req.strings.set_other_method(&m) { + Ok(m) => m, + Err(()) => return Ok(Err(())), + }, + builtin => { + let Ok(m) = builtin.try_into() else { + return Ok(Err(())); + }; + req.strings.set_builtin_method(); + m + } }; req.method = method; Ok(Ok(())) @@ -311,15 +322,11 @@ impl HostRequest for WasiHttpCtxView<'_> { path_with_query: Option, ) -> wasmtime::Result> { let req = get_request_mut(self.table, &req)?; - - let Some(path_with_query) = path_with_query else { - req.path_with_query = None; - return Ok(Ok(())); - }; - let Some(path_with_query) = crate::parse_path_with_query(&path_with_query) else { + let Ok(path_with_query) = req.strings.set_path_with_query(path_with_query.as_deref()) + else { return Ok(Err(())); }; - req.path_with_query = Some(path_with_query); + req.path_with_query = path_with_query; Ok(Ok(())) } @@ -334,14 +341,24 @@ impl HostRequest for WasiHttpCtxView<'_> { scheme: Option, ) -> wasmtime::Result> { let req = get_request_mut(self.table, &req)?; - let Some(scheme) = scheme else { - req.scheme = None; - return Ok(Ok(())); - }; - let Ok(scheme) = scheme.try_into() else { - return Ok(Err(())); + let scheme = match scheme { + Some(Scheme::Other(s)) => match req.strings.set_other_scheme(&s) { + Ok(s) => Some(s), + Err(()) => return Ok(Err(())), + }, + Some(builtin) => { + let Ok(s) = builtin.try_into() else { + return Ok(Err(())); + }; + req.strings.set_builtin_scheme(); + Some(s) + } + None => { + req.strings.set_builtin_scheme(); + None + } }; - req.scheme = Some(scheme); + req.scheme = scheme; Ok(Ok(())) } @@ -356,14 +373,10 @@ impl HostRequest for WasiHttpCtxView<'_> { authority: Option, ) -> wasmtime::Result> { let req = get_request_mut(self.table, &req)?; - let Some(authority) = authority else { - req.authority = None; - return Ok(Ok(())); - }; - let Ok(authority) = crate::parse_authority(authority) else { + let Ok(authority) = req.strings.set_authority(authority) else { return Ok(Err(())); }; - req.authority = Some(authority); + req.authority = authority; Ok(Ok(())) } diff --git a/crates/wasi-http/src/p3/request.rs b/crates/wasi-http/src/p3/request.rs index 0fe9414fc17d..81ac02a1a5a9 100644 --- a/crates/wasi-http/src/p3/request.rs +++ b/crates/wasi-http/src/p3/request.rs @@ -2,7 +2,7 @@ use crate::p3::bindings::http::types::ErrorCode; use crate::p3::body::{Body, BodyExt as _, GuestBody}; use crate::p3::{HttpError, HttpResult}; use crate::{ - Error, FieldMap, RequestOptions, WasiHttpCtxView, WasiHttpHooks, WasiHttpView, + Error, FieldMap, RequestOptions, WasiHttpCtx, WasiHttpCtxView, WasiHttpHooks, WasiHttpView, get_content_length, }; use bytes::Bytes; @@ -32,6 +32,8 @@ pub struct Request { pub options: Option>, /// Request body. pub(crate) body: Body, + /// Accounting for the size of the method, scheme, authority, and path. + pub(crate) strings: crate::request_strings::RequestStringsValidator, } impl Request { @@ -41,25 +43,42 @@ impl Request { /// a request processing error, if any. /// /// Requests constructed this way will not perform any `Content-Length` validation. - pub fn new( + /// + /// # Errors + /// + /// Returns an error carrying an [`ErrorResponse`](crate::ErrorResponse) + /// with status `400 Bad Request` if the combined size of the method, + /// scheme, authority, and path-with-query exceeds the limit configured in + /// `ctx` (see [`WasiHttpCtx::set_request_strings_size_limit`]). + pub fn new( + ctx: &WasiHttpCtx, method: Method, scheme: Option, authority: Option, path_with_query: Option, - headers: impl Into, + headers: H, options: Option>, body: B, - ) -> ( + ) -> wasmtime::Result<( Self, - impl Future> + Send + 'static, - ) + impl Future> + Send + 'static + use, + )> where B: http_body::Body + Send + 'static, B::Error: Into, + H: Into, { + let mut strings = crate::request_strings::RequestStringsValidator::new(ctx); + strings.host_parts( + &method, + scheme.as_ref(), + authority.as_ref(), + path_with_query.as_ref(), + )?; let (tx, rx) = oneshot::channel(); - ( + Ok(( Self { + strings, method, scheme, authority, @@ -75,7 +94,7 @@ impl Request { let Ok(fut) = rx.await else { return Ok(()) }; Box::into_pin(fut).await }, - ) + )) } /// Construct a new [Request] from [http::Request]. @@ -84,13 +103,18 @@ impl Request { /// a request processing error, if any. /// /// Requests constructed this way will not perform any `Content-Length` validation. + /// + /// # Errors + /// + /// Fails under the same conditions as [`Request::new`]. pub fn from_http( + ctx: &WasiHttpCtx, hooks: &mut dyn WasiHttpHooks, req: http::Request, - ) -> ( + ) -> wasmtime::Result<( Self, impl Future> + Send + 'static + use, - ) + )> where T: http_body::Body + Send + 'static, T::Error: Into, @@ -111,6 +135,7 @@ impl Request { .. } = uri.into_parts(); Self::new( + ctx, method, scheme, authority, @@ -155,6 +180,7 @@ impl Request { mut headers, options, body, + strings: _, } = self; // `Content-Length` header value is validated in `fields` implementation let content_length = match get_content_length(&headers) { @@ -297,6 +323,7 @@ mod tests { for scheme in schemes { let (req, fut) = Request::new( + &WasiHttpCtx::new(), Method::POST, scheme.clone(), Some(Authority::from_static("example.com")), @@ -304,7 +331,7 @@ mod tests { FieldMap::default(), None, Full::new(Bytes::from_static(b"body")).boxed_unsync(), - ); + )?; let mut store = Store::new(&engine, TestCtx::new()); let (http_req, options) = req.into_http(&mut store, async { Ok(()) }).unwrap(); assert_eq!(options, None); @@ -328,9 +355,51 @@ mod tests { Ok(()) } + #[test] + fn test_request_new_strings_limit() -> Result<()> { + let new = + |ctx: &WasiHttpCtx, method: Method, authority: &'static str, path: &'static str| { + Request::new( + ctx, + method, + Some(Scheme::HTTP), + Some(Authority::from_static(authority)), + Some(PathAndQuery::from_static(path)), + FieldMap::default(), + None, + Empty::new().boxed_unsync(), + ) + .map(|_| ()) + }; + let expect_bad_request = |r: Result<()>| { + let err = r.unwrap_err(); + let resp = err.downcast_ref::().unwrap(); + assert_eq!(resp.status(), http::StatusCode::BAD_REQUEST); + }; + + let mut ctx = WasiHttpCtx::new(); + ctx.set_request_strings_size_limit(10); + + // Authority and path total 10; `GET` and `http` don't count. + new(&ctx, Method::GET, "aaaaa", "/bbbb")?; + expect_bad_request(new(&ctx, Method::GET, "aaaaa", "/bbbbb")); + + // A non-built-in method counts. + let method = Method::from_bytes(b"X")?; + new(&ctx, method.clone(), "aaaa", "/bbbb")?; + expect_bad_request(new(&ctx, method, "aaaaa", "/bbbb")); + + // Raising the limit admits the larger request. + ctx.set_request_strings_size_limit(11); + new(&ctx, Method::GET, "aaaaa", "/bbbbb")?; + + Ok(()) + } + #[tokio::test] async fn test_request_into_http_uri_error() -> Result<()> { let (req, fut) = Request::new( + &WasiHttpCtx::new(), Method::GET, Some(Scheme::HTTP), Some(Authority::from_static("example.com")), @@ -338,7 +407,7 @@ mod tests { FieldMap::default(), None, Empty::new().boxed_unsync(), - ); + )?; let mut store = Store::new(&Engine::default(), TestCtx::new()); let result = req .into_http(&mut store, async { diff --git a/crates/wasi-http/src/request_strings.rs b/crates/wasi-http/src/request_strings.rs new file mode 100644 index 000000000000..763d83bc473c --- /dev/null +++ b/crates/wasi-http/src/request_strings.rs @@ -0,0 +1,215 @@ +use crate::{ErrorResponse, WasiHttpCtx}; +use http::uri::{Authority, PathAndQuery, Scheme}; + +/// Accounting to limit the size of the strings making up a request's method, +/// scheme, authority, and path-with-query. +/// +/// Every place a request's strings enter the host, whether set by a guest or +/// read off the wire, goes through [`RequestStringsValidator`]. +/// +/// Built-in methods (`GET`, `POST`, ...) and schemes (`http`, `https`) don't +/// allocate, so they don't count toward the limit. +#[derive(Debug, Clone)] +pub(crate) struct RequestStringsValidator { + limit: usize, + method: usize, + scheme: usize, + authority: usize, + path_with_query: usize, +} + +#[derive(Clone, Copy)] +enum Field { + Method, + Scheme, + Authority, + PathWithQuery, +} + +impl RequestStringsValidator { + /// Create a validator using the limit configured in + /// `ctx`. + pub(crate) fn new(ctx: &WasiHttpCtx) -> Self { + Self { + limit: ctx.request_strings_size_limit, + method: 0, + scheme: 0, + authority: 0, + path_with_query: 0, + } + } + + fn slot(&mut self, field: Field) -> &mut usize { + match field { + Field::Method => &mut self.method, + Field::Scheme => &mut self.scheme, + Field::Authority => &mut self.authority, + Field::PathWithQuery => &mut self.path_with_query, + } + } + + fn total(&self) -> usize { + self.method + self.scheme + self.authority + self.path_with_query + } + + fn set( + &mut self, + field: Field, + len: usize, + parse: impl FnOnce() -> Option, + ) -> Result { + let current = *self.slot(field); + if self.total() - current + len > self.limit { + return Err(()); + } + let value = parse().ok_or(())?; + *self.slot(field) = len; + Ok(value) + } + + /// Set a method not among the built-in variants. + pub(crate) fn set_other_method(&mut self, method: &str) -> Result { + self.set(Field::Method, method.len(), || method.parse().ok()) + } + + /// Set one of the built-in methods, which don't count toward the limit. + pub(crate) fn set_builtin_method(&mut self) { + self.method = 0; + } + + /// Set a scheme other than `http` or `https`. + pub(crate) fn set_other_scheme(&mut self, scheme: &str) -> Result { + self.set(Field::Scheme, scheme.len(), || scheme.parse().ok()) + } + + /// Set no scheme, or one of the built-in schemes, neither of which count + /// toward the limit. + pub(crate) fn set_builtin_scheme(&mut self) { + self.scheme = 0; + } + + pub(crate) fn set_authority( + &mut self, + authority: Option, + ) -> Result, ()> { + match authority { + Some(a) => self + .set(Field::Authority, a.len(), || crate::parse_authority(a).ok()) + .map(Some), + None => { + self.authority = 0; + Ok(None) + } + } + } + + pub(crate) fn set_path_with_query( + &mut self, + path_with_query: Option<&str>, + ) -> Result, ()> { + match path_with_query { + Some(p) => self + .set(Field::PathWithQuery, p.len(), || { + crate::parse_path_with_query(p) + }) + .map(Some), + None => { + self.path_with_query = 0; + Ok(None) + } + } + } + + /// Account for a request built by the host from already-parsed parts, + /// such as one received off the wire. + /// + /// On failure, the returned error carries an [`ErrorResponse`] with status + /// 400 so the request can be rejected before reaching the guest. + pub(crate) fn host_parts( + &mut self, + method: &http::Method, + scheme: Option<&Scheme>, + authority: Option<&Authority>, + path_with_query: Option<&PathAndQuery>, + ) -> wasmtime::Result<()> { + self.method = if is_builtin_method(method) { + 0 + } else { + method.as_str().len() + }; + self.scheme = match scheme { + Some(s) if *s != Scheme::HTTP && *s != Scheme::HTTPS => s.as_str().len(), + _ => 0, + }; + self.authority = authority.map_or(0, |a| a.as_str().len()); + self.path_with_query = path_with_query.map_or(0, |p| p.as_str().len()); + if self.total() > self.limit { + return Err(wasmtime::Error::msg(format!( + "request method, scheme, authority, and path with query total {} bytes, \ + exceeding the size limit of {} bytes", + self.total(), + self.limit, + )) + .context(ErrorResponse::new(http::StatusCode::BAD_REQUEST))); + } + Ok(()) + } +} + +fn is_builtin_method(method: &http::Method) -> bool { + use http::Method as M; + [ + M::GET, + M::HEAD, + M::POST, + M::PUT, + M::DELETE, + M::CONNECT, + M::OPTIONS, + M::TRACE, + M::PATCH, + ] + .contains(method) +} + +#[cfg(test)] +mod tests { + use super::RequestStringsValidator; + + #[test] + fn total_is_limited() { + let mut v = RequestStringsValidator::new(&crate::WasiHttpCtx { + field_size_limit: 0, + request_strings_size_limit: 10, + }); + assert!(v.set_path_with_query(Some("/aaaa")).is_ok()); + assert!(v.set_authority(Some("bbbbb".into())).is_ok()); + assert!(v.set_other_method("C").is_err()); + assert!(v.set_other_scheme("c").is_err()); + + // Built-ins don't count. + v.set_builtin_method(); + v.set_builtin_scheme(); + + // Replacing a field releases its old size. + assert!(v.set_path_with_query(Some("/")).is_ok()); + assert!(v.set_other_method("CCCC").is_ok()); + assert!(v.set_authority(None).is_ok()); + assert!(v.set_other_scheme("dddddd").is_err()); + assert!(v.set_other_scheme("ddddd").is_ok()); + } + + #[test] + fn rejected_values_are_not_recorded() { + let mut v = RequestStringsValidator::new(&crate::WasiHttpCtx { + field_size_limit: 0, + request_strings_size_limit: 10, + }); + // Fits, but fails to parse. + assert!(v.set_authority(Some("a:b".into())).is_err()); + assert!(v.set_path_with_query(Some("/aaaaaaaaa")).is_ok()); + // Over the limit. + assert!(v.set_other_method("X").is_err()); + assert!(v.set_path_with_query(Some("/aaaaaaaaa")).is_ok()); + } +} diff --git a/crates/wasi-http/tests/all/p2.rs b/crates/wasi-http/tests/all/p2.rs index a6731c8cf51c..159edd521f16 100644 --- a/crates/wasi-http/tests/all/p2.rs +++ b/crates/wasi-http/tests/all/p2.rs @@ -154,6 +154,7 @@ async fn run_wasi_http( rejected_authority: Option, early_drop: bool, field_size_limit: Option, + string_size_limit: Option, ) -> wasmtime::Result>, ErrorCode>> { let stdout = MemoryOutputPipe::new(4096); let stderr = MemoryOutputPipe::new(4096); @@ -175,6 +176,9 @@ async fn run_wasi_http( if let Some(limit) = field_size_limit { http.set_field_size_limit(limit); } + if let Some(limit) = string_size_limit { + http.set_request_strings_size_limit(limit); + } let ctx = Ctx { table, wasi, @@ -266,6 +270,7 @@ async fn wasi_http_proxy_tests() -> wasmtime::Result<()> { None, false, None, + None, ) .await?; @@ -385,6 +390,7 @@ async fn do_wasi_http_hash_all(override_send_request: bool) -> Result<()> { None, false, None, + None, ) .await??; @@ -434,6 +440,7 @@ async fn wasi_http_hash_all_with_reject() -> Result<()> { Some("forbidden.com".to_string()), false, None, + None, ) .await??; @@ -555,6 +562,7 @@ async fn do_wasi_http_echo(uri: &str, url_header: Option<&str>) -> Result<()> { None, false, None, + None, ) .await??; @@ -607,6 +615,7 @@ async fn wasi_http_without_port() -> Result<()> { None, false, None, + None, ) .await??; @@ -628,6 +637,7 @@ async fn wasi_http_no_trap_on_early_drop() -> Result<()> { None, true, None, + None, ) .await?; @@ -676,6 +686,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { None, false, Some(255), + None, ) .await .context("request with headers on wire over the size limit")??; @@ -688,6 +699,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { None, false, Some(500), + None, ) .await .context("new_fields with size matching the size limit")??; @@ -700,6 +712,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { None, false, Some(500), + None, ) .await .err() @@ -713,6 +726,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { None, false, Some(500), + None, ) .await??; assert_eq!(resp.status(), 200); @@ -724,6 +738,7 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { None, false, Some(500), + None, ) .await .err() @@ -732,3 +747,130 @@ async fn wasi_http_fields_limit_incoming_request() -> Result<()> { Ok(()) } + +#[test_log::test(tokio::test)] +async fn wasi_http_string_limit() -> Result<()> { + async fn status(sets: &str, limit: Option) -> Result { + // `h` + `/rs` is 4 bytes of the incoming request's own budget. + let req = hyper::Request::builder() + .uri("http://h/rs") + .header("sets", sets) + .method(http::Method::GET) + .body(body::empty())?; + let resp = run_wasi_http( + test_programs_artifacts::P2_API_PROXY_COMPONENT, + req, + None, + None, + false, + None, + limit, + ) + .await + .with_context(|| format!("{sets} limit {limit:?}"))??; + Ok(resp.status()) + } + + for field in ["method", "path", "scheme", "authority"] { + // The `http` crate rejects non-standard schemes over 64 bytes, so use a + // limit below that. + let at = format!("{field}=32"); + let over = format!("{field}=33"); + assert_eq!(status(&at, Some(32)).await?, 200, "{field} at limit"); + assert_eq!(status(&over, Some(32)).await?, 500, "{field} over limit"); + // Replacing a field releases the size of its previous value. + let replaced = format!("{at},{field}=8,{at}"); + assert_eq!(status(&replaced, Some(32)).await?, 200, "{field} replaced"); + } + + // The limit applies to the sum of all four fields. + let sum = "method=8,scheme=8,authority=8,path=8"; + assert_eq!(status(sum, Some(32)).await?, 200, "sum at limit"); + let sum = "method=8,scheme=8,authority=8,path=9"; + assert_eq!(status(sum, Some(32)).await?, 500, "sum over limit"); + + for field in ["method", "path", "authority"] { + let at = format!("{field}=16384"); + let over = format!("{field}=16385"); + assert_eq!(status(&at, None).await?, 200, "{field} at default"); + assert_eq!(status(&over, None).await?, 500, "{field} over default"); + assert_eq!( + status(&over, Some(1 << 20)).await?, + 200, + "{field} large limit" + ); + } + + Ok(()) +} + +#[test_log::test(tokio::test)] +async fn wasi_http_string_limit_incoming_request() -> Result<()> { + use wasmtime_wasi_http::ErrorResponse; + + async fn run(req: hyper::Request>, limit: usize) -> Result<()> { + run_wasi_http( + test_programs_artifacts::P2_API_PROXY_COMPONENT, + req, + None, + None, + false, + None, + Some(limit), + ) + .await??; + Ok(()) + } + fn expect_bad_request(r: Result<()>, ctx: &str) { + let err = r.expect_err(ctx); + let resp = err + .downcast_ref::() + .unwrap_or_else(|| panic!("{ctx}: expected ErrorResponse, got {err:?}")); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST, "{ctx}"); + } + let req = |method: &str, uri: &str, host: Option<&str>| { + let mut b = hyper::Request::builder().method(method).uri(uri); + if let Some(host) = host { + b = b.header(http::header::HOST, host); + } + b.body(body::empty()).unwrap() + }; + + // The limit applies to the sum of the method, scheme, authority, and + // path. `GET` and `http` are built-ins and don't count, so for + // `http://` the total is authority plus path. + let authority = "a".repeat(16); + let path = format!("/{}", "b".repeat(15)); + run(req("GET", &format!("http://{authority}{path}"), None), 32).await?; + expect_bad_request( + run(req("GET", &format!("http://{authority}{path}b"), None), 32).await, + "path pushes total over limit", + ); + expect_bad_request( + run(req("GET", &format!("http://{authority}a{path}"), None), 32).await, + "authority pushes total over limit", + ); + + // An authority taken from the `Host` header, when the URI has none, is + // counted in its place. + run(req("GET", &path, Some(&authority)), 32).await?; + expect_bad_request( + run(req("GET", &path, Some(&format!("{authority}a"))), 32).await, + "host header pushes total over limit", + ); + + // A non-built-in method counts. Unknown methods fall through to the + // guest's default handler, which still succeeds. + let method = "X".repeat(15); + run(req(&method, &format!("http://{authority}/"), None), 32).await?; + expect_bad_request( + run( + req(&format!("{method}X"), &format!("http://{authority}/"), None), + 32, + ) + .await, + "method pushes total over limit", + ); + + Ok(()) +} diff --git a/crates/wasi-http/tests/all/p3/mod.rs b/crates/wasi-http/tests/all/p3/mod.rs index 97fc1c6d98d2..457e55708f98 100644 --- a/crates/wasi-http/tests/all/p3/mod.rs +++ b/crates/wasi-http/tests/all/p3/mod.rs @@ -167,7 +167,8 @@ async fn run_http + 'static>( wasmtime_wasi_http::p3::add_to_linker(&mut linker) .context("failed to link `wasi:http@0.3.x`")?; let service = Service::instantiate_async(&mut store, &component, &linker).await?; - let (req, io) = Request::from_http(&mut store.data_mut().hooks, req); + let data = store.data_mut(); + let (req, io) = Request::from_http(&data.http, &mut data.hooks, req)?; store .run_concurrent(async |store| { let (res, ()) = try_join!( diff --git a/src/common.rs b/src/common.rs index 7433b14db943..480cf03cee1a 100644 --- a/src/common.rs +++ b/src/common.rs @@ -389,6 +389,9 @@ impl RunCommon { if let Some(limit) = self.common.wasi.max_http_fields_size { http.set_field_size_limit(limit); } + if let Some(limit) = self.common.wasi.max_http_request_strings_size { + http.set_request_strings_size_limit(limit); + } Ok(http) } diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 5dfe8b049b49..4ef8fe0bb9b1 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -2141,6 +2141,81 @@ start a print 1234 Ok(()) } + async fn cli_serve_request_strings_limit( + component: &str, + configure: impl FnOnce(&mut Command), + ) -> Result<()> { + let server = WasmtimeServe::new(component, |cmd| { + cmd.arg("-Smax-http-request-strings-size=32"); + configure(cmd); + })?; + + let req = |method: &str, uri: String| { + hyper::Request::builder() + .method(method) + .uri(uri) + .body(String::new()) + .context("failed to make request") + }; + + // The limit applies to the sum of the method, scheme, authority, and + // path. `GET` and `http` are built-ins and don't count. + let authority = "a".repeat(16); + let path = format!("/{}", "b".repeat(15)); + let method = "X".repeat(15); + + // At the limit, every request reaches the guest. + for req in [ + req("GET", format!("http://{authority}{path}"))?, + req(&method, format!("http://{authority}/"))?, + ] { + let resp = server.send_request(req).await?; + assert!(resp.status().is_success(), "{resp:?}"); + assert_eq!(resp.body(), "Hello, WASI!"); + } + + // One byte over the limit, each request is rejected with a 400 before + // the guest runs. + for req in [ + req("GET", format!("http://{authority}{path}b"))?, + req("GET", format!("http://{authority}a{path}"))?, + req(&format!("{method}X"), format!("http://{authority}/"))?, + ] { + let desc = format!("{} {}", req.method(), req.uri()); + let resp = server.send_request(req).await?; + assert_eq!(resp.status(), http::StatusCode::BAD_REQUEST, "{desc}"); + assert!( + resp.body().contains("

400 Bad Request

"), + "{desc}: {resp:?}" + ); + } + + let (_, stderr) = server.finish()?; + assert!( + stderr.contains("exceeding the size limit of 32 bytes"), + "{stderr}" + ); + Ok(()) + } + + #[tokio::test] + async fn p2_cli_serve_request_strings_limit() -> Result<()> { + cli_serve_request_strings_limit(P2_CLI_SERVE_HELLO_WORLD_COMPONENT, |cmd| { + cmd.arg("-Scli"); + }) + .await + } + + #[tokio::test] + #[cfg_attr(not(feature = "component-model-async"), ignore)] + async fn p3_cli_serve_request_strings_limit() -> Result<()> { + cli_serve_request_strings_limit(P3_CLI_SERVE_HELLO_WORLD_COMPONENT, |cmd| { + cmd.arg("-Wcomponent-model-async"); + cmd.arg("-Sp3,cli"); + }) + .await + } + #[test] fn p2_cli_argv0() -> Result<()> { run_wasmtime(&["run", "--argv0=a", P2_CLI_ARGV0, "a"])?; @@ -2524,6 +2599,118 @@ start a print 1234 Ok(()) } + fn run_http_request_strings(component: &str, flags: &[&str]) -> Result<()> { + let td = tempfile::TempDir::new()?; + let cwasm = td.path().join("http_request_strings.cwasm"); + let cwasm = cwasm.to_str().unwrap(); + let mut compile = vec!["compile"]; + compile.extend(flags.iter().filter(|f| f.starts_with("-W"))); + compile.extend([component, "-o", cwasm]); + run_wasmtime(&compile)?; + + let run = |limit: Option, sets: &[&str]| -> Result { + let limit = limit.map(|l| format!("-Smax-http-request-strings-size={l}")); + let mut args = vec!["run"]; + args.extend(flags); + args.extend(limit.as_deref()); + args.extend(["--allow-precompiled", cwasm]); + args.extend(sets); + run_wasmtime(&args) + }; + let expect = |out: String, results: &[bool], ctx: &str| { + let want: String = results + .iter() + .map(|ok| if *ok { "ok\n" } else { "error received\n" }) + .collect(); + assert_eq!(out, want, "{ctx}"); + }; + + for field in ["method", "path", "scheme", "authority"] { + // A single field: equal to the limit passes, one over fails. The + // `http` crate caps non-standard schemes at 64 bytes, so stay below. + let at = format!("{field}=32"); + let over = format!("{field}=33"); + expect( + run(Some(32), &[&at])?, + &[true], + &format!("{field}: at limit"), + ); + expect( + run(Some(32), &[&over])?, + &[false], + &format!("{field}: over limit"), + ); + + // Replacing a field releases the size of its previous value. + let small = format!("{field}=8"); + expect( + run(Some(32), &[&at, &small, &at])?, + &[true, true, true], + &format!("{field}: replaced"), + ); + } + + // The limit applies to the sum of all four fields. + expect( + run(Some(32), &["method=8", "scheme=8", "authority=8", "path=8"])?, + &[true, true, true, true], + "sum at limit", + ); + expect( + run(Some(32), &["method=8", "scheme=8", "authority=8", "path=9"])?, + &[true, true, true, false], + "sum over limit", + ); + // A rejected value isn't counted, so a smaller one still fits. + expect( + run( + Some(32), + &["method=8", "scheme=8", "authority=8", "path=9", "path=8"], + )?, + &[true, true, true, false, true], + "sum after rejection", + ); + + for field in ["method", "path", "authority"] { + // Gated by default too. + let at = format!("{field}=16384"); + let over = format!("{field}=16385"); + expect(run(None, &[&at])?, &[true], &format!("{field}: at default")); + expect( + run(None, &[&over])?, + &[false], + &format!("{field}: over default"), + ); + + // Raising the limit allows longer strings. + expect( + run(Some(1 << 20), &[&over])?, + &[true], + &format!("{field}: large limit"), + ); + } + expect( + run(None, &["method=8192", "path=8193"])?, + &[true, false], + "sum over default", + ); + Ok(()) + } + + #[test] + fn p2_cli_http_request_strings() -> Result<()> { + run_http_request_strings(P2_CLI_HTTP_REQUEST_STRINGS_COMPONENT, &["-Shttp"]) + } + + #[test] + #[cfg_attr(not(feature = "component-model-async"), ignore)] + fn p3_cli_http_request_strings() -> Result<()> { + run_http_request_strings( + P3_CLI_HTTP_REQUEST_STRINGS_COMPONENT, + &["-Shttp,p3", "-Wcomponent-model-async"], + ) + } + #[test] #[cfg_attr(not(feature = "component-model-async"), ignore)] fn p2_cli_invoke_async() -> Result<()> {