From d689cc0b4703f43995ea8649291d497a5db4a38f Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Sat, 3 Oct 2026 10:45:23 -0400 Subject: [PATCH] Follow redirects in the client The client follows 301, 302, 303, 307 and 308 redirects, up to `request-options.max-redirects`. 0 returns the redirect response, when none the implementation chooses the limit, 10 for this client. A redirect keeps the method and body, except a 303, or a 301 or 302 for POST, which switches to GET without the body, see RFC 9110 section 15.4. Credentials are not sent when a redirect leaves the origin, and headers describing the body are not sent without the body. New error codes: * redirect-limit-exceeded: the redirect response past the limit, and the number of redirects followed * redirect-requires-body: a redirect that sends the body again. The body was streamed, the client can't send it again. Contains the redirect response, and the method, url, headers and options to send it again with the body. The options have `max-redirects` set to the redirects remaining. Trailers have their own trailers-error-code, error-code contains an http-response, whose trailers can't refer back to error-code. Signed-off-by: Scott Andrews Co-Authored-By: Claude --- Cargo.lock | 3 + components/client/Cargo.toml | 5 + components/client/README.md | 6 + components/client/src/lib.rs | 537 +++++++++++++++++++++++++-- components/client/tests/redirects.rs | 326 ++++++++++++++++ crates/test-harness/src/lib.rs | 141 ++++++- wit/http.wit | 36 +- 7 files changed, 1001 insertions(+), 53 deletions(-) create mode 100644 components/client/tests/redirects.rs diff --git a/Cargo.lock b/Cargo.lock index f3c8f29..32bb3b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -139,7 +139,10 @@ dependencies = [ name = "client" version = "0.1.0" dependencies = [ + "test-harness", + "tokio", "url", + "wasmtime", "wit-bindgen", ] diff --git a/components/client/Cargo.toml b/components/client/Cargo.toml index 25e24c3..59beb36 100644 --- a/components/client/Cargo.toml +++ b/components/client/Cargo.toml @@ -10,3 +10,8 @@ crate-type = ["cdylib"] [dependencies] url = "2.5" wit-bindgen = { workspace = true, features = ["async-spawn"] } + +[dev-dependencies] +test-harness = { workspace = true } +tokio = { workspace = true } +wasmtime = { workspace = true } diff --git a/components/client/README.md b/components/client/README.md index 60ce4d3..5730b58 100644 --- a/components/client/README.md +++ b/components/client/README.md @@ -17,6 +17,12 @@ A higher-level HTTP client that delegates to wasi:http/client. - `trace(url, headers, options)` - `query(url, headers, body, options)` +## Redirects + +Redirects are followed, up to 10 unless `max-redirects` is set in the request options. + +The request body is streamed, so it can't be sent again. A redirect that sends the body again returns a `redirect-requires-body` error with the redirect response, and the method, url, headers and options for the next request, send it with the body to follow the redirect. The body is sent again for a request with a body when the redirect keeps the method, a 307 or 308, or a 301 or 302 for a method other than POST. A 303, or a 301 or 302 for POST, switches to GET without the body, and is followed. + ## The `client` World - exports `componentized:http/client` diff --git a/components/client/src/lib.rs b/components/client/src/lib.rs index d812a1b..fd84c39 100644 --- a/components/client/src/lib.rs +++ b/components/client/src/lib.rs @@ -7,7 +7,7 @@ wit_bindgen::generate!({ }); use exports::componentized::http::client::{ - ErrorCode, Guest, HttpResponse, Method, RequestOptions, + ErrorCode, Guest, HttpResponse, Method, RedirectRequest, RequestOptions, TrailersErrorCode, }; use wasi::http::types::{ @@ -18,37 +18,122 @@ use wit_bindgen::rt::async_support::{FutureReader, StreamReader}; struct HttpClient; +/// The number of redirects followed when `request-options.max-redirects` is not set. +const DEFAULT_MAX_REDIRECTS: u8 = 10; + +/// Request headers that carry credentials, not sent when a redirect leaves the origin. +const CREDENTIAL_HEADERS: [&str; 3] = ["authorization", "cookie", "proxy-authorization"]; + +/// Request headers that describe the body, not sent when a redirect drops the body. +const BODY_HEADERS: [&str; 5] = [ + "content-encoding", + "content-language", + "content-length", + "content-location", + "content-type", +]; + impl HttpClient { async fn request( - method: WasiMethod, + mut method: Method, url: String, - headers: Vec<(String, String)>, - body: Option>, + mut headers: Vec<(String, String)>, + mut body: Option>, options: Option, ) -> Result { + let max_redirects = options + .as_ref() + .and_then(|options| options.max_redirects) + .unwrap_or(DEFAULT_MAX_REDIRECTS); + let mut url = Url::parse(&url).map_err(|e| err(format!("Invalid URL: {e}")))?; + let mut redirects = 0; + loop { + let has_body = body.is_some(); + let (response, send_result) = + Self::send(method, &url, &headers, body.take(), options.as_ref()).await?; + + let status = response.get_status_code(); + let response_headers = read_fields(&response.get_headers()); + match next_step( + status, + &response_headers, + method, + has_body, + redirects, + max_redirects, + ) { + Step::Return => { + return Ok(Self::response( + response, + send_result, + status, + response_headers, + )); + } + Step::TooManyRedirects => { + let response = Self::response(response, send_result, status, response_headers); + return Err(ErrorCode::RedirectLimitExceeded((response, redirects))); + } + Step::RequiresBody { location } => { + let next = redirect_url(&url, &location)?; + let redirect = RedirectRequest { + method, + headers: redirect_headers(headers, &url, &next, true), + url: next.into(), + // the limit was checked, this redirect is within it + options: Some(redirect_options( + options.as_ref(), + max_redirects - redirects - 1, + )), + }; + let response = Self::response(response, send_result, status, response_headers); + return Err(ErrorCode::RedirectRequiresBody((response, redirect))); + } + Step::Follow { + location, + method: next_method, + keep_body, + } => { + let next = redirect_url(&url, &location)?; + headers = redirect_headers(headers, &url, &next, keep_body); + redirects += 1; + method = next_method; + url = next; + } + } + } + } + + /// Send a single request, without following redirects. + async fn send( + method: Method, + url: &Url, + headers: &[(String, String)], + body: Option>, + options: Option<&RequestOptions>, + ) -> Result<(WasiResponse, FutureReader>), ErrorCode> { let request_headers = Fields::new(); - for (name, value) in &headers { + for (name, value) in headers { request_headers .append(name, value.as_bytes()) .map_err(|e| err(format!("Invalid request header {name:?}: {e:?}")))?; } - let parsed = Url::parse(&url).map_err(|e| err(format!("Invalid URL: {e}")))?; - let scheme = match parsed.scheme() { + let scheme = match url.scheme() { "http" => Scheme::Http, "https" => Scheme::Https, other => return Err(err(format!("Unsupported URL scheme: {other}"))), }; - let host = parsed + let host = url .host_str() .ok_or_else(|| err("URL is missing a host".to_string()))?; - let authority = match parsed.port() { + let authority = match url.port() { Some(port) => format!("{host}:{port}"), None => host.to_string(), }; - let path_with_query = match parsed.query() { - Some(q) => format!("{}?{q}", parsed.path()), - None => parsed.path().to_string(), + let path_with_query = match url.query() { + Some(q) => format!("{}?{q}", url.path()), + None => url.path().to_string(), }; let (trailers_tx, trailers_rx) = wit_future::new(|| Ok(None)); @@ -58,7 +143,7 @@ impl HttpClient { let (request, send_result) = WasiRequest::new(request_headers, body, trailers_rx, wasi_options); request - .set_method(&method) + .set_method(&to_wasi_method(method)) .map_err(|()| err("Failed to set request method".to_string()))?; request .set_scheme(Some(&scheme)) @@ -73,19 +158,135 @@ impl HttpClient { let response = wasi::http::client::send(request) .await .map_err(|e| err(format!("HTTP request failed: {e:?}")))?; + Ok((response, send_result)) + } - let status = response.get_status_code(); - let headers = read_fields(&response.get_headers()); - + fn response( + response: WasiResponse, + send_result: FutureReader>, + status: u16, + headers: Vec<(String, String)>, + ) -> HttpResponse { let (body_stream, wasi_trailers) = WasiResponse::consume_body(response, send_result); - - Ok(HttpResponse { + HttpResponse { status, headers, body: body_stream, trailers: map_trailers(wasi_trailers), - }) + } + } +} + +/// What to do with a response. +#[derive(Debug)] +enum Step { + /// Return the response to the caller. + Return, + /// Send a request to the location of the redirect. + Follow { + location: String, + method: Method, + keep_body: bool, + }, + /// The redirect sends the body again, which the caller has to do. + RequiresBody { location: String }, + /// The response is a redirect past the limit. + TooManyRedirects, +} + +/// What to do with a response, after following `redirects` redirects of at most `max_redirects`. +/// +/// A redirect is followed when its location is known, unless it sends the request body again. +/// The body was streamed to the first request, it can't be sent again. +fn next_step( + status: u16, + headers: &[(String, String)], + method: Method, + has_body: bool, + redirects: u8, + max_redirects: u8, +) -> Step { + if max_redirects == 0 { + return Step::Return; } + let Some(location) = redirect_location(status, headers) else { + return Step::Return; + }; + if redirects >= max_redirects { + return Step::TooManyRedirects; + } + let (method, keep_body) = redirect_method(status, method); + if has_body && keep_body { + return Step::RequiresBody { location }; + } + Step::Follow { + location, + method, + keep_body, + } +} + +/// The location to follow for a redirect response, `None` if the response is not a redirect. +fn redirect_location(status: u16, headers: &[(String, String)]) -> Option { + if !matches!(status, 301 | 302 | 303 | 307 | 308) { + return None; + } + headers + .iter() + .find(|(name, _)| name.eq_ignore_ascii_case("location")) + .map(|(_, value)| value.clone()) +} + +/// The method for the request following a redirect, and whether the body is sent again. +/// +/// 303 switches to GET, except for HEAD, as does 301 and 302 for POST, as browsers do. Otherwise +/// the method and body are kept, see RFC 9110 section 15.4. +fn redirect_method(status: u16, method: Method) -> (Method, bool) { + match (status, method) { + (303, Method::Head) => (Method::Head, false), + (303, _) | (301 | 302, Method::Post) => (Method::Get, false), + _ => (method, true), + } +} + +/// The options for the request following a redirect, the original options with the redirects +/// remaining. +fn redirect_options(options: Option<&RequestOptions>, max_redirects: u8) -> RequestOptions { + RequestOptions { + connect_timeout_ms: options.and_then(|options| options.connect_timeout_ms), + first_byte_timeout_ms: options.and_then(|options| options.first_byte_timeout_ms), + between_bytes_timeout_ms: options.and_then(|options| options.between_bytes_timeout_ms), + max_redirects: Some(max_redirects), + } +} + +/// The URL a redirect location refers to, relative to the URL of the request. +fn redirect_url(url: &Url, location: &str) -> Result { + url.join(location) + .map_err(|e| err(format!("Invalid redirect location {location:?}: {e}"))) +} + +/// The request headers for the request following a redirect from `from` to `to`. +/// +/// Credentials are not sent to another origin, and headers describing the body are not sent +/// without the body. +fn redirect_headers( + mut headers: Vec<(String, String)>, + from: &Url, + to: &Url, + keep_body: bool, +) -> Vec<(String, String)> { + if from.origin() != to.origin() { + headers.retain(|(name, _)| !is_one_of(name, &CREDENTIAL_HEADERS)); + } + if !keep_body { + headers.retain(|(name, _)| !is_one_of(name, &BODY_HEADERS)); + } + headers +} + +fn is_one_of(name: &str, names: &[&str]) -> bool { + names.iter().any(|n| name.eq_ignore_ascii_case(n)) } fn err(message: String) -> ErrorCode { @@ -102,20 +303,22 @@ fn read_fields(fields: &Fields) -> Vec<(String, String)> { fn map_trailers( wasi: FutureReader, WasiErrorCode>>, -) -> FutureReader, ErrorCode>> { +) -> FutureReader, TrailersErrorCode>> { let (tx, rx) = wit_future::new(|| Ok(Vec::new())); wit_bindgen::rt::async_support::spawn_local(async move { let resolved = match wasi.await { Ok(Some(t)) => Ok(read_fields(&t)), Ok(None) => Ok(Vec::new()), - Err(e) => Err(err(format!("wasi:http error: {e:?}"))), + Err(e) => Err(TrailersErrorCode::Other(Some(format!( + "wasi:http error: {e:?}" + )))), }; tx.write(resolved); }); rx } -fn wasi_request_options(opts: RequestOptions) -> Result { +fn wasi_request_options(opts: &RequestOptions) -> Result { let r = WasiRequestOptions::new(); if let Some(ms) = opts.connect_timeout_ms { r.set_connect_timeout(Some(ms_to_ns(ms))) @@ -158,7 +361,7 @@ impl Guest for HttpClient { body: Option>, options: Option, ) -> Result { - Self::request(to_wasi_method(method), url, headers, body, options).await + Self::request(method, url, headers, body, options).await } async fn get( @@ -166,7 +369,7 @@ impl Guest for HttpClient { headers: Vec<(String, String)>, options: Option, ) -> Result { - Self::request(WasiMethod::Get, url, headers, None, options).await + Self::request(Method::Get, url, headers, None, options).await } async fn post( @@ -175,7 +378,7 @@ impl Guest for HttpClient { body: StreamReader, options: Option, ) -> Result { - Self::request(WasiMethod::Post, url, headers, Some(body), options).await + Self::request(Method::Post, url, headers, Some(body), options).await } async fn put( @@ -184,7 +387,7 @@ impl Guest for HttpClient { body: StreamReader, options: Option, ) -> Result { - Self::request(WasiMethod::Put, url, headers, Some(body), options).await + Self::request(Method::Put, url, headers, Some(body), options).await } async fn delete( @@ -192,7 +395,7 @@ impl Guest for HttpClient { headers: Vec<(String, String)>, options: Option, ) -> Result { - Self::request(WasiMethod::Delete, url, headers, None, options).await + Self::request(Method::Delete, url, headers, None, options).await } async fn patch( @@ -201,7 +404,7 @@ impl Guest for HttpClient { body: StreamReader, options: Option, ) -> Result { - Self::request(WasiMethod::Patch, url, headers, Some(body), options).await + Self::request(Method::Patch, url, headers, Some(body), options).await } async fn head( @@ -209,7 +412,7 @@ impl Guest for HttpClient { headers: Vec<(String, String)>, options: Option, ) -> Result { - Self::request(WasiMethod::Head, url, headers, None, options).await + Self::request(Method::Head, url, headers, None, options).await } async fn options( @@ -217,7 +420,7 @@ impl Guest for HttpClient { headers: Vec<(String, String)>, options: Option, ) -> Result { - Self::request(WasiMethod::Options, url, headers, None, options).await + Self::request(Method::Options, url, headers, None, options).await } async fn trace( @@ -225,7 +428,7 @@ impl Guest for HttpClient { headers: Vec<(String, String)>, options: Option, ) -> Result { - Self::request(WasiMethod::Trace, url, headers, None, options).await + Self::request(Method::Trace, url, headers, None, options).await } async fn query( @@ -234,15 +437,271 @@ impl Guest for HttpClient { body: StreamReader, options: Option, ) -> Result { - Self::request( - to_wasi_method(Method::Query), - url, - headers, - Some(body), - options, - ) - .await + Self::request(Method::Query, url, headers, Some(body), options).await } } export!(HttpClient); + +#[cfg(test)] +mod tests { + use super::*; + + fn location(value: &str) -> Vec<(String, String)> { + vec![("Location".to_string(), value.to_string())] + } + + fn url(value: &str) -> Url { + Url::parse(value).unwrap() + } + + /// Compare by debug output, the generated `Method` does not implement `PartialEq`. + fn assert_same(actual: impl std::fmt::Debug, expected: impl std::fmt::Debug, context: &str) { + assert_eq!(format!("{actual:?}"), format!("{expected:?}"), "{context}"); + } + + fn header(name: &str) -> (String, String) { + (name.to_string(), "value".to_string()) + } + + #[test] + fn follows_redirects() { + for status in [301, 302, 303, 307, 308] { + assert!( + matches!( + next_step(status, &location("/next"), Method::Get, false, 0, 10), + Step::Follow { ref location, .. } if location == "/next" + ), + "{status}" + ); + } + } + + #[test] + fn returns_other_responses() { + for status in [200, 204, 300, 304, 404, 500] { + assert_same( + next_step(status, &location("/next"), Method::Get, false, 0, 10), + Step::Return, + &format!("{status}"), + ); + } + } + + #[test] + fn returns_redirect_without_location() { + assert_same( + next_step(302, &[], Method::Get, false, 0, 10), + Step::Return, + "", + ); + } + + #[test] + fn zero_max_redirects_returns_redirect() { + assert_same( + next_step(302, &location("/next"), Method::Get, false, 0, 0), + Step::Return, + "", + ); + } + + #[test] + fn exceeding_max_redirects_is_an_error() { + assert!(matches!( + next_step(302, &location("/next"), Method::Get, false, 2, 3), + Step::Follow { .. } + )); + assert_same( + next_step(302, &location("/next"), Method::Get, false, 3, 3), + Step::TooManyRedirects, + "", + ); + } + + #[test] + fn redirect_that_resends_the_body_requires_the_caller() { + for status in [307, 308] { + assert_same( + next_step(status, &location("/next"), Method::Post, true, 0, 10), + Step::RequiresBody { + location: "/next".to_string(), + }, + &format!("{status}"), + ); + } + // without a body there is nothing to resend + assert_same( + next_step(307, &location("/next"), Method::Post, false, 0, 10), + Step::Follow { + location: "/next".to_string(), + method: Method::Post, + keep_body: true, + }, + "", + ); + } + + #[test] + fn moved_redirect_for_a_method_other_than_post_requires_the_body() { + for status in [301, 302] { + for method in [Method::Put, Method::Patch, Method::Delete, Method::Query] { + assert_same( + next_step(status, &location("/next"), method, true, 0, 10), + Step::RequiresBody { + location: "/next".to_string(), + }, + &format!("{status} {method:?}"), + ); + } + // POST switches to GET without the body + assert_same( + next_step(status, &location("/next"), Method::Post, true, 0, 10), + Step::Follow { + location: "/next".to_string(), + method: Method::Get, + keep_body: false, + }, + &format!("{status}"), + ); + } + } + + #[test] + fn redirect_limit_is_checked_before_the_body() { + assert_same( + next_step(307, &location("/next"), Method::Post, true, 3, 3), + Step::TooManyRedirects, + "", + ); + } + + #[test] + fn redirect_that_drops_the_body_is_followed() { + assert_same( + next_step(303, &location("/next"), Method::Put, true, 0, 10), + Step::Follow { + location: "/next".to_string(), + method: Method::Get, + keep_body: false, + }, + "", + ); + } + + #[test] + fn redirect_methods() { + use Method::*; + let cases = [ + (301, Post, Get, false), + (302, Post, Get, false), + (303, Post, Get, false), + (303, Put, Get, false), + (303, Get, Get, false), + (303, Head, Head, false), + (301, Put, Put, true), + (302, Delete, Delete, true), + (307, Post, Post, true), + (308, Put, Put, true), + ]; + for (status, method, expected, keep_body) in cases { + assert_same( + redirect_method(status, method), + (expected, keep_body), + &format!("{status} {method:?}"), + ); + } + } + + #[test] + fn credentials_kept_for_the_same_origin() { + let headers = vec![header("Authorization"), header("Cookie"), header("accept")]; + assert_eq!( + redirect_headers( + headers.clone(), + &url("https://example.com/a"), + &url("https://example.com/b"), + true + ), + headers + ); + } + + #[test] + fn credentials_dropped_for_another_origin() { + let headers = vec![ + header("Authorization"), + header("cookie"), + header("Proxy-Authorization"), + header("accept"), + ]; + for to in [ + "https://example.org/", + "https://other.example.com/", + "http://example.com/", + "https://example.com:8443/", + ] { + assert_eq!( + redirect_headers( + headers.clone(), + &url("https://example.com/"), + &url(to), + true + ), + vec![header("accept")], + "{to}" + ); + } + } + + #[test] + fn body_headers_dropped_with_the_body() { + let headers = vec![ + header("Content-Type"), + header("content-length"), + header("Content-Encoding"), + header("Content-Language"), + header("Content-Location"), + header("accept"), + ]; + let same = url("https://example.com/"); + assert_eq!( + redirect_headers(headers.clone(), &same, &same, false), + vec![header("accept")] + ); + assert_eq!( + redirect_headers(headers.clone(), &same, &same, true), + headers + ); + } + + #[test] + fn redirect_options_copy_the_original_options() { + let original = RequestOptions { + connect_timeout_ms: Some(1), + first_byte_timeout_ms: Some(2), + between_bytes_timeout_ms: None, + max_redirects: Some(5), + }; + assert_same( + redirect_options(Some(&original), 3), + RequestOptions { + connect_timeout_ms: Some(1), + first_byte_timeout_ms: Some(2), + between_bytes_timeout_ms: None, + max_redirects: Some(3), + }, + "", + ); + assert_same( + redirect_options(None, 9), + RequestOptions { + connect_timeout_ms: None, + first_byte_timeout_ms: None, + between_bytes_timeout_ms: None, + max_redirects: Some(9), + }, + "", + ); + } +} diff --git a/components/client/tests/redirects.rs b/components/client/tests/redirects.rs new file mode 100644 index 0000000..8654c39 --- /dev/null +++ b/components/client/tests/redirects.rs @@ -0,0 +1,326 @@ +//! Tests for the redirects followed by `client`, against a scripted upstream. + +use test_harness::{ + Harness, HttpClientErrorCode, HttpClientMethod, HttpClientRedirectRequest, + HttpClientRequestOptions, UpstreamRequest, UpstreamResponse, stream, +}; + +/// Answer each path with the response, and every other path with the default response. +fn routes( + routes: Vec<(&'static str, UpstreamResponse)>, +) -> impl FnMut(&UpstreamRequest) -> UpstreamResponse + Send + 'static { + move |request| { + routes + .iter() + .find(|(path, _)| request.path() == *path) + .map(|(_, response)| response.clone()) + .unwrap_or_default() + } +} + +fn options(max_redirects: Option) -> Option { + Some(HttpClientRequestOptions { + connect_timeout_ms: Some(1_000), + first_byte_timeout_ms: None, + between_bytes_timeout_ms: None, + max_redirects, + }) +} + +fn header(name: &str, value: &str) -> (String, String) { + (name.to_string(), value.to_string()) +} + +/// The method and path of each request sent upstream. +fn sent(client: &test_harness::TestSubject) -> Vec<(String, String)> { + client + .recorder() + .upstream_requests() + .iter() + .map(|request| (request.method.clone(), request.path().to_string())) + .collect() +} + +fn get(path: &str) -> (String, String) { + ("GET".to_string(), path.to_string()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn follows_redirects() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(routes(vec![ + ("/a", UpstreamResponse::redirect(301, "/b")), + ( + "/b", + UpstreamResponse::redirect(302, "https://example.com/c"), + ), + ("/c", UpstreamResponse::redirect(307, "d")), + ("/d", UpstreamResponse::redirect(308, "/e?f=g")), + ])) + .build() + .await?; + let status = client + .run(async |accessor, client| { + let response = client + .componentized_http_client() + .call_get(accessor, "https://example.com/a".to_string(), vec![], None) + .await? + .expect("get"); + Ok(response.status) + }) + .await?; + assert_eq!(status, 200); + assert_eq!( + sent(&client), + vec![get("/a"), get("/b"), get("/c"), get("/d"), get("/e?f=g")] + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn zero_max_redirects_returns_the_redirect() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(routes(vec![("/a", UpstreamResponse::redirect(302, "/b"))])) + .build() + .await?; + let (status, headers) = client + .run(async |accessor, client| { + let response = client + .componentized_http_client() + .call_get( + accessor, + "https://example.com/a".to_string(), + vec![], + options(Some(0)), + ) + .await? + .expect("get"); + Ok((response.status, response.headers)) + }) + .await?; + assert_eq!(status, 302); + assert!(headers.contains(&header("location", "/b")), "{headers:?}"); + assert_eq!(sent(&client), vec![get("/a")]); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn exceeding_max_redirects_is_an_error() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(|_| UpstreamResponse::redirect(302, "/loop")) + .build() + .await?; + let error = client + .run(async |accessor, client| { + let result = client + .componentized_http_client() + .call_get( + accessor, + "https://example.com/start".to_string(), + vec![], + options(Some(2)), + ) + .await?; + Ok(match result { + Err(HttpClientErrorCode::RedirectLimitExceeded((response, redirects))) => { + Some((response.status, redirects)) + } + _ => None, + }) + }) + .await?; + // the redirect response past the limit, after following 2 redirects + assert_eq!(error, Some((302, 2))); + assert_eq!( + sent(&client), + vec![get("/start"), get("/loop"), get("/loop")] + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn see_other_switches_to_get_without_the_body() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(routes(vec![("/a", UpstreamResponse::redirect(303, "/b"))])) + .build() + .await?; + let status = client + .run(async |accessor, client| { + let body = stream(accessor, b"payload".to_vec())?; + let response = client + .componentized_http_client() + .call_post( + accessor, + "https://example.com/a".to_string(), + vec![ + header("content-type", "text/plain"), + header("accept", "text/plain"), + ], + body, + None, + ) + .await? + .expect("post"); + Ok(response.status) + }) + .await?; + assert_eq!(status, 200); + let requests = client.recorder().upstream_requests(); + assert_eq!(requests.len(), 2); + assert_eq!(requests[0].method, "POST"); + assert_eq!(requests[0].body, b"payload"); + assert_eq!(requests[1].method, "GET"); + assert_eq!(requests[1].path(), "/b"); + assert_eq!(requests[1].body, b""); + // headers describing the body are dropped with it + assert_eq!(requests[1].header("content-type"), Vec::<&str>::new()); + assert_eq!(requests[1].header("accept"), vec!["text/plain"]); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn credentials_are_not_sent_to_another_origin() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(routes(vec![ + ("/a", UpstreamResponse::redirect(302, "/b")), + ( + "/b", + UpstreamResponse::redirect(302, "https://example.org/c"), + ), + ])) + .build() + .await?; + client + .run(async |accessor, client| { + client + .componentized_http_client() + .call_get( + accessor, + "https://example.com/a".to_string(), + vec![ + header("authorization", "Bearer secret"), + header("accept", "text/plain"), + ], + None, + ) + .await? + .expect("get"); + Ok(()) + }) + .await?; + let requests = client.recorder().upstream_requests(); + assert_eq!(requests.len(), 3); + // kept for the same origin + assert_eq!(requests[1].header("authorization"), vec!["Bearer secret"]); + // dropped for another origin + assert_eq!(requests[2].uri, "https://example.org/c"); + assert_eq!(requests[2].header("authorization"), Vec::<&str>::new()); + assert_eq!(requests[2].header("accept"), vec!["text/plain"]); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn redirect_that_resends_the_body_is_returned_to_the_caller() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(routes(vec![( + "/start", + UpstreamResponse::redirect(307, "https://example.org/b"), + )])) + .build() + .await?; + let (status, redirect) = client + .run(async |accessor, client| { + let body = stream(accessor, b"payload".to_vec())?; + let result = client + .componentized_http_client() + .call_request( + accessor, + HttpClientMethod::Put, + "https://example.com/start".to_string(), + vec![ + header("authorization", "Bearer secret"), + header("content-type", "text/plain"), + ], + Some(body), + options(Some(5)), + ) + .await?; + Ok(match result { + Err(HttpClientErrorCode::RedirectRequiresBody((response, redirect))) => { + (response.status, redirect) + } + Ok(response) => panic!("expected redirect-requires-body, got {}", response.status), + Err(err) => panic!("expected redirect-requires-body, got {err:?}"), + }) + }) + .await?; + assert_eq!(status, 307); + // returned without following it, the body was already sent + assert_eq!( + sent(&client), + vec![("PUT".to_string(), "/start".to_string())] + ); + assert!( + matches!(redirect.method, HttpClientMethod::Put), + "{:?}", + redirect.method + ); + assert_eq!(redirect.url, "https://example.org/b"); + // the credentials are dropped for another origin, the body headers are kept for the body + assert_eq!(redirect.headers, vec![header("content-type", "text/plain")]); + // the original options, with the redirects remaining after this one of 5 + let options = redirect.options.expect("options"); + assert_eq!(options.connect_timeout_ms, Some(1_000)); + assert_eq!(options.max_redirects, Some(4)); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn caller_resends_the_body_to_follow_the_redirect() -> wasmtime::Result<()> { + let mut client = Harness::new("client") + .upstream(routes(vec![("/a", UpstreamResponse::redirect(308, "/b"))])) + .build() + .await?; + let status = client + .run(async |accessor, client| { + let http = client.componentized_http_client(); + let body = stream(accessor, b"payload".to_vec())?; + let result = http + .call_post( + accessor, + "https://example.com/a".to_string(), + vec![header("content-type", "text/plain")], + body, + None, + ) + .await?; + let HttpClientRedirectRequest { + method, + url, + headers, + options, + } = match result { + Err(HttpClientErrorCode::RedirectRequiresBody((_, redirect))) => redirect, + Ok(response) => panic!("expected redirect-requires-body, got {}", response.status), + Err(err) => panic!("expected redirect-requires-body, got {err:?}"), + }; + // send the body again, as given + let body = stream(accessor, b"payload".to_vec())?; + let response = http + .call_request(accessor, method, url, headers, Some(body), options) + .await? + .expect("request"); + Ok(response.status) + }) + .await?; + assert_eq!(status, 200); + let requests = client.recorder().upstream_requests(); + assert_eq!(requests.len(), 2); + for request in &requests { + assert_eq!(request.method, "POST"); + assert_eq!(request.body, b"payload"); + assert_eq!(request.header("content-type"), vec!["text/plain"]); + } + assert_eq!(requests[1].path(), "/b"); + Ok(()) +} diff --git a/crates/test-harness/src/lib.rs b/crates/test-harness/src/lib.rs index 49c79d5..283a9e5 100644 --- a/crates/test-harness/src/lib.rs +++ b/crates/test-harness/src/lib.rs @@ -59,7 +59,9 @@ pub mod bindings { } pub use bindings::exports::componentized::http::client::{ - ErrorCode as HttpClientErrorCode, Method as HttpClientMethod, + ErrorCode as HttpClientErrorCode, HttpResponse as HttpClientResponse, + Method as HttpClientMethod, RedirectRequest as HttpClientRedirectRequest, + RequestOptions as HttpClientRequestOptions, }; pub use bindings::exports::wasi::http::types::{ErrorCode, Method, Scheme}; pub use bindings::wasi::logging::logging::Level; @@ -112,6 +114,14 @@ pub fn ready( accessor.with(|store| FutureReader::new(store, async move { Ok::<_, wasmtime::Error>(value) })) } +/// A stream for the guest that yields the items, then closes. +pub fn stream( + accessor: &Accessor, + items: Vec, +) -> Result> { + accessor.with(|store| StreamReader::new(store, items)) +} + /// Read every item written to a guest stream, until it closes. pub async fn collect( accessor: &Accessor, @@ -293,11 +303,70 @@ pub struct SentRequest { pub uri: String, } +/// A request the test subject sent upstream, with its headers and body. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UpstreamRequest { + pub method: String, + pub uri: String, + pub headers: Vec<(String, String)>, + pub body: Vec, +} + +impl UpstreamRequest { + /// The values of the named header, ignoring case. + pub fn header(&self, name: &str) -> Vec<&str> { + self.headers + .iter() + .filter(|(n, _)| n.eq_ignore_ascii_case(name)) + .map(|(_, v)| v.as_str()) + .collect() + } + + /// The path and query of the request URI. + pub fn path(&self) -> &str { + self.uri + .parse::() + .ok() + .and_then(|uri| uri.path_and_query().map(|p| p.as_str().len())) + .map(|len| &self.uri[self.uri.len() - len..]) + .unwrap_or(&self.uri) + } +} + +/// The response upstream sends for a request, the body is empty. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UpstreamResponse { + pub status: u16, + pub headers: Vec<(String, String)>, +} + +impl Default for UpstreamResponse { + /// [`UPSTREAM_STATUS`] with [`UPSTREAM_HEADER`]. + fn default() -> Self { + Self { + status: UPSTREAM_STATUS, + headers: vec![(UPSTREAM_HEADER.0.to_string(), UPSTREAM_HEADER.1.to_string())], + } + } +} + +impl UpstreamResponse { + /// A redirect to the location. + pub fn redirect(status: u16, location: &str) -> Self { + Self { + status, + headers: vec![("location".to_string(), location.to_string())], + } + } +} + +type Responder = dyn FnMut(&UpstreamRequest) -> UpstreamResponse + Send; + /// Observations recorded while the test subject runs. #[derive(Clone, Default)] pub struct Recorder { logs: Arc>>, - requests: Arc>>, + requests: Arc>>, } impl Recorder { @@ -331,13 +400,25 @@ impl Recorder { /// Requests the test subject sent upstream, in order. pub fn requests(&self) -> Vec { + self.upstream_requests() + .into_iter() + .map(|request| SentRequest { + method: request.method, + uri: request.uri, + }) + .collect() + } + + /// Requests the test subject sent upstream, in order, with their headers and body. + pub fn upstream_requests(&self) -> Vec { self.requests.lock().unwrap().clone() } } -/// Answers requests sent upstream with a canned response, instead of the network. +/// Answers requests sent upstream instead of the network, by default with a canned response. struct Upstream { recorder: Recorder, + responder: Arc>>, } impl WasiHttpHooks for Upstream { @@ -357,21 +438,39 @@ impl WasiHttpHooks for Upstream { >, > + Send, > { - self.recorder.requests.lock().unwrap().push(SentRequest { - method: request.method().to_string(), - uri: request.uri().to_string(), - }); + let recorder = self.recorder.clone(); + let responder = self.responder.clone(); Box::new(async move { + let (parts, body) = request.into_parts(); + let body = body.collect().await?.to_bytes().to_vec(); + let request = UpstreamRequest { + method: parts.method.to_string(), + uri: parts.uri.to_string(), + headers: parts + .headers + .iter() + .map(|(name, value)| { + ( + name.to_string(), + String::from_utf8_lossy(value.as_bytes()).into_owned(), + ) + }) + .collect(), + body, + }; + let upstream = (responder.lock().unwrap())(&request); + recorder.requests.lock().unwrap().push(request); + // the body is passed through the test subject to the harness host to host, which the // harness can't read, see `resolve` let body = Empty::::new() .map_err(|never| match never {}) .boxed_unsync(); - let response = http::Response::builder() - .status(UPSTREAM_STATUS) - .header(UPSTREAM_HEADER.0, UPSTREAM_HEADER.1) - .body(body) - .expect("canned response"); + let mut response = http::Response::builder().status(upstream.status); + for (name, value) in &upstream.headers { + response = response.header(name, value); + } + let response = response.body(body).expect("upstream response"); let io = Box::new(async { Ok(()) }) as Box> + Send>; Ok((response, io)) @@ -440,9 +539,12 @@ impl UpstreamClient { .requests .lock() .unwrap() - .push(SentRequest { + .push(UpstreamRequest { method: method.to_string(), uri: url, + // not read, the body stream is closed + headers: vec![], + body: vec![], }); if let Some(mut body) = body { body.close(&mut store) @@ -603,6 +705,7 @@ fn add_handler_to_linker(linker: &mut Linker) -> Result<()> { /// Builds a subject instance for a test. pub struct Harness { subject: String, + responder: Box, } impl Harness { @@ -610,9 +713,20 @@ impl Harness { pub fn new(component_name: &str) -> Self { Self { subject: component_name.to_string(), + responder: Box::new(|_| UpstreamResponse::default()), } } + /// Answer the requests sent upstream with `wasi:http` with the responses, instead of the + /// default [`UpstreamResponse`]. + pub fn upstream( + mut self, + responder: impl FnMut(&UpstreamRequest) -> UpstreamResponse + Send + 'static, + ) -> Self { + self.responder = Box::new(responder); + self + } + /// Instantiate the test subject. pub async fn build(self) -> Result { let mut config = Config::new(); @@ -639,6 +753,7 @@ impl Harness { http: WasiHttpCtx::new(), upstream: Upstream { recorder: recorder.clone(), + responder: Arc::new(Mutex::new(self.responder)), }, table: ResourceTable::new(), recorder: recorder.clone(), diff --git a/wit/http.wit b/wit/http.wit index e659997..1bacf0e 100644 --- a/wit/http.wit +++ b/wit/http.wit @@ -16,6 +16,35 @@ interface client { @since(version = 0.1.0-dev) variant error-code { + /// The response was a redirect past `request-options.max-redirects`. + /// Contains the redirect response, and the number of redirects + /// followed before it. + redirect-limit-exceeded(tuple), + /// The response was a redirect that sends the request body again. The + /// body was already streamed, it can't be sent again. Contains the + /// redirect response, and the request to send with the body to follow + /// the redirect. + redirect-requires-body(tuple), + other(option), + } + + /// The request to send to follow a redirect. + @since(version = 0.1.0-dev) + record redirect-request { + method: method, + url: string, + /// The request headers, without credentials when the redirect leaves + /// the origin. + headers: list>, + /// The options of the original request, with `max-redirects` set to + /// the number of redirects remaining. Always set, an option to match + /// the `options` parameter of the request functions. + options: option, + } + + /// An error reading the trailers of a response, after its body. + @since(version = 0.1.0-dev) + variant trailers-error-code { other(option), } @@ -25,6 +54,11 @@ interface client { connect-timeout-ms: option, first-byte-timeout-ms: option, between-bytes-timeout-ms: option, + /// Limit the maximum number of redirects to follow for a single + /// request. 0 disables redirects entirely, the redirect response is + /// returned. When none, the implementation chooses the limit. + /// Exceeding the limit is a `redirect-limit-exceeded` error. + max-redirects: option, } /// A streaming HTTP response. The status and headers are available @@ -35,7 +69,7 @@ interface client { status: u16, headers: list>, body: stream, - trailers: future>, error-code>>, + trailers: future>, trailers-error-code>>, } /// Send an HTTP request with an explicit method. Both the request body and