diff --git a/crates/test-programs/src/bin/http_outbound_request_missing_path_and_query.rs b/crates/test-programs/src/bin/http_outbound_request_missing_path_and_query.rs new file mode 100644 index 000000000000..4c278ade1452 --- /dev/null +++ b/crates/test-programs/src/bin/http_outbound_request_missing_path_and_query.rs @@ -0,0 +1,16 @@ +use test_programs::wasi::http::outgoing_handler::{handle, OutgoingRequest}; +use test_programs::wasi::http::types::{Fields, Method, Scheme}; + +fn main() { + let fields = Fields::new(); + let req = OutgoingRequest::new(fields); + req.set_method(&Method::Get).unwrap(); + req.set_scheme(Some(&Scheme::Https)).unwrap(); + req.set_authority(Some("example.com")).unwrap(); + + // Don't set path/query + // req.set_path_with_query(Some("/")).unwrap(); + + let res = handle(req, None); + assert!(res.is_err()); +} diff --git a/crates/wasi-http/src/error.rs b/crates/wasi-http/src/error.rs new file mode 100644 index 000000000000..7270baaa3d5e --- /dev/null +++ b/crates/wasi-http/src/error.rs @@ -0,0 +1,55 @@ +use crate::bindings::http::types::ErrorCode; +use std::error::Error; +use std::fmt; +use wasmtime_wasi::ResourceTableError; + +pub type HttpResult = Result; + +/// A `wasi:http`-specific error type used to represent either a trap or an +/// [`ErrorCode`]. +/// +/// Modeled after [`TrappableError`](wasmtime_wasi::TrappableError). +#[repr(transparent)] +pub struct HttpError { + err: anyhow::Error, +} + +impl HttpError { + pub fn trap(err: impl Into) -> HttpError { + HttpError { err: err.into() } + } + + pub fn downcast(self) -> anyhow::Result { + self.err.downcast() + } + + pub fn downcast_ref(&self) -> Option<&ErrorCode> { + self.err.downcast_ref() + } +} + +impl From for HttpError { + fn from(error: ErrorCode) -> Self { + Self { err: error.into() } + } +} + +impl From for HttpError { + fn from(error: ResourceTableError) -> Self { + HttpError::trap(error) + } +} + +impl fmt::Debug for HttpError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.err.fmt(f) + } +} + +impl fmt::Display for HttpError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.err.fmt(f) + } +} + +impl Error for HttpError {} diff --git a/crates/wasi-http/src/http_impl.rs b/crates/wasi-http/src/http_impl.rs index 7f4990853326..f25df6a2030e 100644 --- a/crates/wasi-http/src/http_impl.rs +++ b/crates/wasi-http/src/http_impl.rs @@ -17,7 +17,7 @@ impl outgoing_handler::Host for T { &mut self, request_id: Resource, options: Option>, - ) -> wasmtime::Result, types::ErrorCode>> { + ) -> crate::HttpResult> { let opts = options.and_then(|opts| self.table().get(&opts).ok()); let connect_timeout = opts @@ -47,7 +47,7 @@ impl outgoing_handler::Host for T { types::Method::Patch => Method::PATCH, types::Method::Other(m) => match hyper::Method::from_bytes(m.as_bytes()) { Ok(method) => method, - Err(_) => return Ok(Err(types::ErrorCode::HttpRequestMethodInvalid)), + Err(_) => return Err(types::ErrorCode::HttpRequestMethodInvalid.into()), }, }); @@ -56,7 +56,7 @@ impl outgoing_handler::Host for T { Scheme::Https => (true, http::uri::Scheme::HTTPS, 443), // We can only support http/https - Scheme::Other(_) => return Ok(Err(types::ErrorCode::HttpProtocolError)), + Scheme::Other(_) => return Err(types::ErrorCode::HttpProtocolError.into()), }; let authority = if let Some(authority) = req.authority { @@ -94,23 +94,13 @@ impl outgoing_handler::Host for T { .body(body) .map_err(|err| internal_error(err.to_string()))?; - let result = self.send_request(OutgoingRequest { + self.send_request(OutgoingRequest { use_tls, authority, request, connect_timeout, first_byte_timeout, between_bytes_timeout, - }); - - // attempt to downcast the error to a ErrorCode - // so that the guest may handle it - match result { - Ok(response) => Ok(Ok(response)), - Err(err) => match err.downcast::() { - Ok(err) => Ok(Err(err)), - Err(err) => Err(err), - }, - } + }) } } diff --git a/crates/wasi-http/src/lib.rs b/crates/wasi-http/src/lib.rs index c7c8aec448e6..29419d745539 100644 --- a/crates/wasi-http/src/lib.rs +++ b/crates/wasi-http/src/lib.rs @@ -1,6 +1,9 @@ +use crate::bindings::http::types::ErrorCode; +pub use crate::error::{HttpError, HttpResult}; pub use crate::types::{WasiHttpCtx, WasiHttpView}; pub mod body; +mod error; pub mod http_impl; pub mod io; pub mod proxy; @@ -34,27 +37,28 @@ pub mod bindings { "wasi:http/types/incoming-request": super::types::HostIncomingRequest, "wasi:http/types/fields": super::types::HostFields, "wasi:http/types/request-options": super::types::HostRequestOptions, - } + }, + trappable_error_type: { + "wasi:http/types/error-code" => crate::HttpError, + }, }); pub use wasi::http; } -pub(crate) fn dns_error(rcode: String, info_code: u16) -> bindings::http::types::ErrorCode { - bindings::http::types::ErrorCode::DnsError(bindings::http::types::DnsErrorPayload { +pub(crate) fn dns_error(rcode: String, info_code: u16) -> ErrorCode { + ErrorCode::DnsError(bindings::http::types::DnsErrorPayload { rcode: Some(rcode), info_code: Some(info_code), }) } -pub(crate) fn internal_error(msg: String) -> bindings::http::types::ErrorCode { - bindings::http::types::ErrorCode::InternalError(Some(msg)) +pub(crate) fn internal_error(msg: String) -> ErrorCode { + ErrorCode::InternalError(Some(msg)) } /// Translate a [`http::Error`] to a wasi-http `ErrorCode` in the context of a request. -pub fn http_request_error(err: http::Error) -> bindings::http::types::ErrorCode { - use bindings::http::types::ErrorCode; - +pub fn http_request_error(err: http::Error) -> ErrorCode { if err.is::() { return ErrorCode::HttpRequestUriInvalid; } @@ -65,8 +69,7 @@ pub fn http_request_error(err: http::Error) -> bindings::http::types::ErrorCode } /// Translate a [`hyper::Error`] to a wasi-http `ErrorCode` in the context of a request. -pub fn hyper_request_error(err: hyper::Error) -> bindings::http::types::ErrorCode { - use bindings::http::types::ErrorCode; +pub fn hyper_request_error(err: hyper::Error) -> ErrorCode { use std::error::Error; // If there's a source, we might be able to extract a wasi-http error from it. @@ -82,8 +85,7 @@ pub fn hyper_request_error(err: hyper::Error) -> bindings::http::types::ErrorCod } /// Translate a [`hyper::Error`] to a wasi-http `ErrorCode` in the context of a response. -pub fn hyper_response_error(err: hyper::Error) -> bindings::http::types::ErrorCode { - use bindings::http::types::ErrorCode; +pub fn hyper_response_error(err: hyper::Error) -> ErrorCode { use std::error::Error; if err.is_timeout() { diff --git a/crates/wasi-http/src/types.rs b/crates/wasi-http/src/types.rs index 839af33f1f60..469d1f8aa7ce 100644 --- a/crates/wasi-http/src/types.rs +++ b/crates/wasi-http/src/types.rs @@ -5,7 +5,7 @@ use crate::io::TokioIo; use crate::{ bindings::http::types::{self, Method, Scheme}, body::{HostIncomingBody, HyperIncomingBody, HyperOutgoingBody}, - dns_error, hyper_request_error, + dns_error, hyper_request_error, HttpResult, }; use http_body_util::BodyExt; use hyper::header::HeaderName; @@ -63,7 +63,7 @@ pub trait WasiHttpView: Send { fn send_request( &mut self, request: OutgoingRequest, - ) -> wasmtime::Result> + ) -> HttpResult> where Self: Sized, { @@ -121,7 +121,7 @@ pub fn default_send_request( first_byte_timeout, between_bytes_timeout, }: OutgoingRequest, -) -> wasmtime::Result> { +) -> HttpResult> { let handle = wasmtime_wasi::runtime::spawn(async move { let resp = handler( authority, diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs index 07ef93a32996..d8dc5f3d371f 100644 --- a/crates/wasi-http/src/types_impl.rs +++ b/crates/wasi-http/src/types_impl.rs @@ -14,10 +14,14 @@ use std::str::FromStr; use wasmtime::component::{Resource, ResourceTable}; use wasmtime_wasi::{ bindings::io::streams::{InputStream, OutputStream}, - Pollable, + Pollable, ResourceTableError, }; impl crate::bindings::http::types::Host for T { + fn convert_error_code(&mut self, err: crate::HttpError) -> wasmtime::Result { + err.downcast() + } + fn http_error_code( &mut self, err: wasmtime::component::Resource, @@ -49,7 +53,10 @@ fn get_content_length(fields: &FieldMap) -> Result, ()> { /// Take ownership of the underlying [`FieldMap`] associated with this fields resource. If the /// fields resource references another fields, the returned [`FieldMap`] will be cloned. -fn move_fields(table: &mut ResourceTable, id: Resource) -> wasmtime::Result { +fn move_fields( + table: &mut ResourceTable, + id: Resource, +) -> Result { match table.delete(id)? { HostFields::Ref { parent, get_fields } => { let entry = table.get_any_mut(parent)?; @@ -874,7 +881,7 @@ impl crate::bindings::http::types::HostOutgoingBody for T { &mut self, id: Resource, ts: Option>, - ) -> wasmtime::Result> { + ) -> crate::HttpResult<()> { let body = self.table().delete(id)?; let ts = if let Some(ts) = ts { @@ -883,7 +890,8 @@ impl crate::bindings::http::types::HostOutgoingBody for T { None }; - Ok(body.finish(ts)) + body.finish(ts)?; + Ok(()) } fn drop(&mut self, id: Resource) -> wasmtime::Result<()> { diff --git a/crates/wasi-http/tests/all/async_.rs b/crates/wasi-http/tests/all/async_.rs index 1ed1bea7f7b0..371229a1431b 100644 --- a/crates/wasi-http/tests/all/async_.rs +++ b/crates/wasi-http/tests/all/async_.rs @@ -97,3 +97,13 @@ async fn http_outbound_request_content_length() -> Result<()> { let server = Server::http1()?; run(HTTP_OUTBOUND_REQUEST_CONTENT_LENGTH_COMPONENT, &server).await } + +#[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn http_outbound_request_missing_path_and_query() -> Result<()> { + let server = Server::http1()?; + run( + HTTP_OUTBOUND_REQUEST_MISSING_PATH_AND_QUERY_COMPONENT, + &server, + ) + .await +} diff --git a/crates/wasi-http/tests/all/main.rs b/crates/wasi-http/tests/all/main.rs index 800f3a0bc6c7..ecbdd8d7e163 100644 --- a/crates/wasi-http/tests/all/main.rs +++ b/crates/wasi-http/tests/all/main.rs @@ -17,13 +17,13 @@ use wasmtime_wasi_http::{ body::HyperIncomingBody, io::TokioIo, types::{self, HostFutureIncomingResponse, IncomingResponseInternal, OutgoingRequest}, - WasiHttpCtx, WasiHttpView, + HttpResult, WasiHttpCtx, WasiHttpView, }; mod http_server; type RequestSender = Arc< - dyn Fn(&mut Ctx, OutgoingRequest) -> wasmtime::Result> + dyn Fn(&mut Ctx, OutgoingRequest) -> HttpResult> + Send + Sync, >; @@ -59,7 +59,7 @@ impl WasiHttpView for Ctx { fn send_request( &mut self, request: OutgoingRequest, - ) -> wasmtime::Result> { + ) -> HttpResult> { if let Some(rejected_authority) = &self.rejected_authority { let (auth, _port) = request.authority.split_once(':').unwrap(); if auth == rejected_authority { diff --git a/crates/wasi-http/tests/all/sync.rs b/crates/wasi-http/tests/all/sync.rs index ac6567f38e11..27ffb1f8964b 100644 --- a/crates/wasi-http/tests/all/sync.rs +++ b/crates/wasi-http/tests/all/sync.rs @@ -96,3 +96,12 @@ fn http_outbound_request_content_length() -> Result<()> { let server = Server::http1()?; run(HTTP_OUTBOUND_REQUEST_CONTENT_LENGTH_COMPONENT, &server) } + +#[test_log::test] +fn http_outbound_request_missing_path_and_query() -> Result<()> { + let server = Server::http1()?; + run( + HTTP_OUTBOUND_REQUEST_MISSING_PATH_AND_QUERY_COMPONENT, + &server, + ) +}