From bba4ee78a98236e73540506ffebab91373791a82 Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Wed, 15 Nov 2023 11:58:16 -0800 Subject: [PATCH] Ensure that scheme and authority are populated in the incoming handler (#7545) --- Cargo.lock | 1 + Cargo.toml | 3 +- crates/test-programs/src/bin/api_proxy.rs | 4 ++ .../src/bin/api_proxy_streaming.rs | 2 + crates/wasi-http/tests/all/main.rs | 19 +++++---- src/commands/serve.rs | 39 ++++++++++++++++--- 6 files changed, 54 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2661c1154173..3f698546e8b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3322,6 +3322,7 @@ dependencies = [ "criterion", "env_logger 0.10.0", "filecheck", + "http", "http-body-util", "humantime 2.1.0", "hyper", diff --git a/Cargo.toml b/Cargo.toml index e13b6b7b09d3..bae7248d520c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,7 @@ async-trait = { workspace = true } bytes = { workspace = true } tokio = { workspace = true, optional = true, features = [ "signal", "macros" ] } hyper = { workspace = true, optional = true } +http = { workspace = true, optional = true } http-body-util = { workspace = true, optional = true } [target.'cfg(unix)'.dependencies] @@ -359,7 +360,7 @@ old-cli = [] # CLI subcommands for the `wasmtime` executable. See `wasmtime $cmd --help` # for more information on each subcommand. -serve = ["wasi-http", "component-model", "dep:http-body-util"] +serve = ["wasi-http", "component-model", "dep:http-body-util", "dep:http"] explore = ["dep:wasmtime-explorer"] wast = ["dep:wasmtime-wast"] config = ["cache"] diff --git a/crates/test-programs/src/bin/api_proxy.rs b/crates/test-programs/src/bin/api_proxy.rs index d9ca3936fab0..33114699bcf2 100644 --- a/crates/test-programs/src/bin/api_proxy.rs +++ b/crates/test-programs/src/bin/api_proxy.rs @@ -16,6 +16,10 @@ struct T; impl bindings::exports::wasi::http::incoming_handler::Guest for T { fn handle(request: IncomingRequest, outparam: ResponseOutparam) { + assert!(request.scheme().is_some()); + assert!(request.authority().is_some()); + assert!(request.path_with_query().is_some()); + let header = String::from("custom-forbidden-header"); let req_hdrs = request.headers(); diff --git a/crates/test-programs/src/bin/api_proxy_streaming.rs b/crates/test-programs/src/bin/api_proxy_streaming.rs index b2a354650b83..d03ff362381e 100644 --- a/crates/test-programs/src/bin/api_proxy_streaming.rs +++ b/crates/test-programs/src/bin/api_proxy_streaming.rs @@ -33,6 +33,8 @@ impl bindings::exports::wasi::http::incoming_handler::Guest for Handler { async fn handle_request(request: IncomingRequest, response_out: ResponseOutparam) { let headers = request.headers().entries(); + assert!(request.authority().is_some()); + match (request.method(), request.path_with_query().as_deref()) { (Method::Get, Some("/hash-all")) => { // Send outgoing GET requests to the specified URLs and stream the hashes of the response bodies as diff --git a/crates/wasi-http/tests/all/main.rs b/crates/wasi-http/tests/all/main.rs index 323d41892feb..0479161f8ece 100644 --- a/crates/wasi-http/tests/all/main.rs +++ b/crates/wasi-http/tests/all/main.rs @@ -199,11 +199,10 @@ async fn run_wasi_http( #[test_log::test(tokio::test)] async fn wasi_http_proxy_tests() -> anyhow::Result<()> { - let mut req = hyper::Request::builder().method(http::Method::GET); - - req.headers_mut() - .unwrap() - .append("custom-forbidden-header", "yes".parse().unwrap()); + let req = hyper::Request::builder() + .header("custom-forbidden-header", "yes") + .uri("http://example.com:8080/test-path") + .method(http::Method::GET); let resp = run_wasi_http( test_programs_artifacts::API_PROXY_COMPONENT, @@ -326,7 +325,9 @@ async fn do_wasi_http_hash_all(override_send_request: bool) -> Result<()> { None }; - let mut request = hyper::Request::get("/hash-all"); + let mut request = hyper::Request::builder() + .method(http::Method::GET) + .uri("http://example.com:8080/hash-all"); for path in bodies.keys() { request = request.header("url", format!("{prefix}{path}")); } @@ -448,8 +449,10 @@ async fn do_wasi_http_echo(uri: &str, url_header: Option<&str>) -> Result<()> { .collect::>() }; - let mut request = - hyper::Request::post(&format!("/{uri}")).header("content-type", "application/octet-stream"); + let mut request = hyper::Request::builder() + .method(http::Method::POST) + .uri(format!("http://example.com:8080/{uri}")) + .header("content-type", "application/octet-stream"); if let Some(url_header) = url_header { request = request.header("url", url_header); diff --git a/src/commands/serve.rs b/src/commands/serve.rs index fe5a8f240857..2ce05a0e1368 100644 --- a/src/commands/serve.rs +++ b/src/commands/serve.rs @@ -15,7 +15,8 @@ use wasmtime_wasi::preview2::{ self, StreamError, StreamResult, Table, WasiCtx, WasiCtxBuilder, WasiView, }; use wasmtime_wasi_http::{ - body::HyperOutgoingBody, hyper_response_error, WasiHttpCtx, WasiHttpView, + bindings::http::types as http_types, body::HyperOutgoingBody, hyper_response_error, + WasiHttpCtx, WasiHttpView, }; #[cfg(feature = "wasi-nn")] @@ -356,6 +357,37 @@ impl hyper::service::Service for ProxyHandler { // TODO: need to track the join handle, but don't want to block the response on it tokio::task::spawn(async move { let req_id = inner.next_req_id(); + let (mut parts, body) = req.into_parts(); + + parts.uri = { + let uri_parts = parts.uri.into_parts(); + + let scheme = uri_parts.scheme.unwrap_or(http::uri::Scheme::HTTP); + + let host = if let Some(val) = parts.headers.get(hyper::header::HOST) { + std::str::from_utf8(val.as_bytes()) + .map_err(|_| http_types::ErrorCode::HttpRequestUriInvalid)? + } else { + uri_parts + .authority + .as_ref() + .ok_or(http_types::ErrorCode::HttpRequestUriInvalid)? + .host() + }; + + let path_with_query = uri_parts + .path_and_query + .ok_or(http_types::ErrorCode::HttpRequestUriInvalid)?; + + hyper::Uri::builder() + .scheme(scheme) + .authority(host) + .path_and_query(path_with_query) + .build() + .map_err(|_| http_types::ErrorCode::HttpRequestUriInvalid)? + }; + + let req = hyper::Request::from_parts(parts, body.map_err(hyper_response_error).boxed()); log::info!( "Request {req_id} handling {} to {}", @@ -365,10 +397,7 @@ impl hyper::service::Service for ProxyHandler { let mut store = inner.cmd.new_store(&inner.engine, req_id)?; - let req = store - .data_mut() - .new_incoming_request(req.map(|body| body.map_err(hyper_response_error).boxed()))?; - + let req = store.data_mut().new_incoming_request(req)?; let out = store.data_mut().new_response_outparam(sender)?; let (proxy, _inst) =