Skip to content

Commit

Permalink
Fix handling of authority/scheme in wasmtime serve (#8923) (#8932)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton authored Jul 10, 2024
1 parent 72fe1de commit 0bbdbc4
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 34 deletions.
26 changes: 26 additions & 0 deletions crates/test-programs/src/bin/cli_serve_authority_and_scheme.rs
Original file line number Diff line number Diff line change
@@ -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() {}
3 changes: 2 additions & 1 deletion crates/wasi-http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
//!
Expand Down
24 changes: 21 additions & 3 deletions crates/wasi-http/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -81,6 +82,7 @@ pub trait WasiHttpView: Send {
/// Create a new incoming request resource.
fn new_incoming_request<B>(
&mut self,
scheme: Scheme,
req: hyper::Request<B>,
) -> wasmtime::Result<Resource<HostIncomingRequest>>
where
Expand All @@ -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)?)
}

Expand Down Expand Up @@ -486,6 +488,8 @@ impl TryInto<http::Method> 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<HostIncomingBody>,
}
Expand All @@ -495,10 +499,24 @@ impl HostIncomingRequest {
pub fn new(
view: &mut dyn WasiHttpView,
mut parts: http::request::Parts,
scheme: Scheme,
body: Option<HostIncomingBody>,
) -> Self {
) -> anyhow::Result<Self> {
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,
})
}
}

Expand Down
18 changes: 2 additions & 16 deletions crates/wasi-http/src/types_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,25 +312,11 @@ where
}
fn scheme(&mut self, id: Resource<HostIncomingRequest>) -> wasmtime::Result<Option<Scheme>> {
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<HostIncomingRequest>) -> wasmtime::Result<Option<String>> {
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(
Expand Down
4 changes: 2 additions & 2 deletions crates/wasi-http/tests/all/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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)?;
Expand Down
24 changes: 12 additions & 12 deletions src/commands/serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -400,22 +401,21 @@ async fn handle_request(
) -> Result<hyper::Response<HyperOutgoingBody>> {
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)
Expand Down
31 changes: 31 additions & 0 deletions tests/all/cli_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 0bbdbc4

Please sign in to comment.