From 0bbdbc4869021a7733946d94343f66f7810145f9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Jul 2024 15:37:06 -0500 Subject: [PATCH] Fix handling of authority/scheme in `wasmtime serve` (#8923) (#8932) This commit is aimed at fixing an accidental regression from #8861 where the `wasmtime serve` subcommand stopped reporting some instances of authority and scheme. After discussion in #8878 the new logic implemented is: * Creation of an incoming request now explicitly requires specifying a scheme which is out-of-band information about how the surrounding server received the request. * The authority is stored separately within a request and is inferred from the URI's authority or the `Host` header if present. If neither are present then an error is returned. Closes #8878 --- .../src/bin/cli_serve_authority_and_scheme.rs | 26 ++++++++++++++++ crates/wasi-http/src/lib.rs | 3 +- crates/wasi-http/src/types.rs | 24 ++++++++++++-- crates/wasi-http/src/types_impl.rs | 18 ++--------- crates/wasi-http/tests/all/main.rs | 4 +-- src/commands/serve.rs | 24 +++++++------- tests/all/cli_tests.rs | 31 +++++++++++++++++++ 7 files changed, 96 insertions(+), 34 deletions(-) create mode 100644 crates/test-programs/src/bin/cli_serve_authority_and_scheme.rs diff --git a/crates/test-programs/src/bin/cli_serve_authority_and_scheme.rs b/crates/test-programs/src/bin/cli_serve_authority_and_scheme.rs new file mode 100644 index 000000000000..604edc07c865 --- /dev/null +++ b/crates/test-programs/src/bin/cli_serve_authority_and_scheme.rs @@ -0,0 +1,26 @@ +use test_programs::proxy; +use test_programs::wasi::http::types::{ + Fields, IncomingRequest, OutgoingResponse, ResponseOutparam, Scheme, +}; + +struct T; + +proxy::export!(T); + +impl proxy::exports::wasi::http::incoming_handler::Guest for T { + fn handle(request: IncomingRequest, outparam: ResponseOutparam) { + let authority = request.authority(); + let scheme = request.scheme(); + + assert_eq!(authority.as_deref(), Some("localhost")); + assert!( + matches!(scheme, Some(Scheme::Http)), + "bad scheme: {scheme:?}", + ); + + let resp = OutgoingResponse::new(Fields::new()); + ResponseOutparam::set(outparam, Ok(resp)); + } +} + +fn main() {} diff --git a/crates/wasi-http/src/lib.rs b/crates/wasi-http/src/lib.rs index 8c5c0f509f03..7fb5a9a4cabd 100644 --- a/crates/wasi-http/src/lib.rs +++ b/crates/wasi-http/src/lib.rs @@ -73,6 +73,7 @@ //! use wasmtime::{Config, Engine, Result, Store}; //! use wasmtime_wasi::{WasiCtx, WasiCtxBuilder, WasiView}; //! use wasmtime_wasi_http::bindings::ProxyPre; +//! use wasmtime_wasi_http::bindings::http::types::Scheme; //! use wasmtime_wasi_http::body::HyperOutgoingBody; //! use wasmtime_wasi_http::io::TokioIo; //! use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView}; @@ -146,7 +147,7 @@ //! }, //! ); //! let (sender, receiver) = tokio::sync::oneshot::channel(); -//! let req = store.data_mut().new_incoming_request(req)?; +//! let req = store.data_mut().new_incoming_request(Scheme::Http, req)?; //! let out = store.data_mut().new_response_outparam(sender)?; //! let pre = self.pre.clone(); //! diff --git a/crates/wasi-http/src/types.rs b/crates/wasi-http/src/types.rs index 29dd9375bb63..4f22444d3690 100644 --- a/crates/wasi-http/src/types.rs +++ b/crates/wasi-http/src/types.rs @@ -8,6 +8,7 @@ use crate::{ error::dns_error, hyper_request_error, }; +use anyhow::bail; use bytes::Bytes; use http_body_util::BodyExt; use hyper::body::Body; @@ -81,6 +82,7 @@ pub trait WasiHttpView: Send { /// Create a new incoming request resource. fn new_incoming_request( &mut self, + scheme: Scheme, req: hyper::Request, ) -> wasmtime::Result> where @@ -94,7 +96,7 @@ pub trait WasiHttpView: Send { // TODO: this needs to be plumbed through std::time::Duration::from_millis(600 * 1000), ); - let incoming_req = HostIncomingRequest::new(self, parts, Some(body)); + let incoming_req = HostIncomingRequest::new(self, parts, scheme, Some(body))?; Ok(self.table().push(incoming_req)?) } @@ -486,6 +488,8 @@ impl TryInto for types::Method { /// The concrete type behind a `wasi:http/types/incoming-request` resource. pub struct HostIncomingRequest { pub(crate) parts: http::request::Parts, + pub(crate) scheme: Scheme, + pub(crate) authority: String, /// The body of the incoming request. pub body: Option, } @@ -495,10 +499,24 @@ impl HostIncomingRequest { pub fn new( view: &mut dyn WasiHttpView, mut parts: http::request::Parts, + scheme: Scheme, body: Option, - ) -> Self { + ) -> anyhow::Result { + 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 => bail!("invalid HTTP request missing authority in URI and host header"), + }, + }; + remove_forbidden_headers(view, &mut parts.headers); - Self { parts, body } + Ok(Self { + parts, + authority, + scheme, + body, + }) } } diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs index a1d637322abc..b09cd20ed38f 100644 --- a/crates/wasi-http/src/types_impl.rs +++ b/crates/wasi-http/src/types_impl.rs @@ -312,25 +312,11 @@ where } fn scheme(&mut self, id: Resource) -> wasmtime::Result> { let req = self.table().get(&id)?; - Ok(req.parts.uri.scheme().map(|scheme| { - if scheme == &http::uri::Scheme::HTTP { - return Scheme::Http; - } - - if scheme == &http::uri::Scheme::HTTPS { - return Scheme::Https; - } - - Scheme::Other(req.parts.uri.scheme_str().unwrap().to_owned()) - })) + Ok(Some(req.scheme.clone())) } fn authority(&mut self, id: Resource) -> wasmtime::Result> { let req = self.table().get(&id)?; - Ok(req - .parts - .uri - .authority() - .map(|auth| auth.as_str().to_owned())) + Ok(Some(req.authority.clone())) } fn headers( diff --git a/crates/wasi-http/tests/all/main.rs b/crates/wasi-http/tests/all/main.rs index 1bdd6ff88815..03e7981905fc 100644 --- a/crates/wasi-http/tests/all/main.rs +++ b/crates/wasi-http/tests/all/main.rs @@ -13,7 +13,7 @@ use wasmtime::{ }; use wasmtime_wasi::{self, pipe::MemoryOutputPipe, WasiCtx, WasiCtxBuilder, WasiView}; use wasmtime_wasi_http::{ - bindings::http::types::ErrorCode, + bindings::http::types::{ErrorCode, Scheme}, body::HyperOutgoingBody, io::TokioIo, types::{self, HostFutureIncomingResponse, IncomingResponse, OutgoingRequestConfig}, @@ -166,7 +166,7 @@ async fn run_wasi_http( wasmtime_wasi_http::bindings::Proxy::instantiate_async(&mut store, &component, &linker) .await?; - let req = store.data_mut().new_incoming_request(req)?; + let req = store.data_mut().new_incoming_request(Scheme::Http, req)?; let (sender, receiver) = tokio::sync::oneshot::channel(); let out = store.data_mut().new_response_outparam(sender)?; diff --git a/src/commands/serve.rs b/src/commands/serve.rs index 001688ce7030..949d856c2951 100644 --- a/src/commands/serve.rs +++ b/src/commands/serve.rs @@ -12,6 +12,7 @@ use std::{ use wasmtime::component::Linker; use wasmtime::{Config, Engine, Memory, MemoryType, Store, StoreLimits}; use wasmtime_wasi::{StreamError, StreamResult, WasiCtx, WasiCtxBuilder, WasiView}; +use wasmtime_wasi_http::bindings::http::types::Scheme; use wasmtime_wasi_http::bindings::ProxyPre; use wasmtime_wasi_http::io::TokioIo; use wasmtime_wasi_http::{body::HyperOutgoingBody, WasiHttpCtx, WasiHttpView}; @@ -400,22 +401,21 @@ async fn handle_request( ) -> Result> { let (sender, receiver) = tokio::sync::oneshot::channel(); - let task = tokio::task::spawn(async move { - let req_id = inner.next_req_id(); - - log::info!( - "Request {req_id} handling {} to {}", - req.method(), - req.uri() - ); + let req_id = inner.next_req_id(); - let mut store = inner.cmd.new_store(&inner.engine, req_id)?; + log::info!( + "Request {req_id} handling {} to {}", + req.method(), + req.uri() + ); - let req = store.data_mut().new_incoming_request(req)?; - let out = store.data_mut().new_response_outparam(sender)?; + let mut store = inner.cmd.new_store(&inner.engine, req_id)?; - let proxy = inner.instance_pre.instantiate_async(&mut store).await?; + let req = store.data_mut().new_incoming_request(Scheme::Http, req)?; + let out = store.data_mut().new_response_outparam(sender)?; + let proxy = inner.instance_pre.instantiate_async(&mut store).await?; + let task = tokio::task::spawn(async move { if let Err(e) = proxy .wasi_http_incoming_handler() .call_handle(store, req, out) diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 069a0e532e36..bba532d9aecd 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -1814,6 +1814,37 @@ stderr [1] :: after empty Ok(()) } + + #[tokio::test] + async fn cli_serve_authority_and_scheme() -> Result<()> { + let server = WasmtimeServe::new(CLI_SERVE_AUTHORITY_AND_SCHEME_COMPONENT, |cmd| { + cmd.arg("-Scli"); + })?; + + let resp = server + .send_request( + hyper::Request::builder() + .uri("/") + .header("Host", "localhost") + .body(String::new()) + .context("failed to make request")?, + ) + .await?; + assert!(resp.status().is_success()); + + let resp = server + .send_request( + hyper::Request::builder() + .method("CONNECT") + .uri("http://localhost/") + .body(String::new()) + .context("failed to make request")?, + ) + .await?; + assert!(resp.status().is_success()); + + Ok(()) + } } #[test]