From 1acf3614e31d998af49bd39fdec56ed420f27a35 Mon Sep 17 00:00:00 2001 From: Marian Degel Date: Sun, 2 Nov 2025 15:07:35 +0000 Subject: [PATCH 1/4] Refactor HttpClient request handling to support redirect logic. --- src/net/http.rs | 265 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 221 insertions(+), 44 deletions(-) diff --git a/src/net/http.rs b/src/net/http.rs index 9152adc..4948aa5 100644 --- a/src/net/http.rs +++ b/src/net/http.rs @@ -9,8 +9,8 @@ use core::ptr::NonNull; use std::io; use bytes::Bytes; -use http::uri::Scheme; -use http::{Request, Response}; +use http::uri::{PathAndQuery, Scheme}; +use http::{Method, Request, Response, StatusCode, Uri}; use http_body::Body; use http_body_util::BodyExt; use nginx_sys::{ngx_log_t, ngx_resolver_t, NGX_LOG_WARN}; @@ -18,6 +18,8 @@ use ngx::allocator::Box; use ngx::async_::resolver::Resolver; use ngx::async_::spawn; use ngx::ngx_log_error; +use std::format; +use std::string::ToString; use thiserror::Error; use super::peer_conn::PeerConnection; @@ -40,6 +42,9 @@ const NGX_ACME_USER_AGENT: &str = constcat::concat!( NGINX_VER, ); +// Maximum number of redirects to follow for a single request. +const MAX_REDIRECTS: usize = 10; + #[allow(async_fn_in_trait)] pub trait HttpClient { type Error: StdError + Send + Sync + 'static; @@ -101,26 +106,41 @@ impl<'a> NgxHttpClient<'a> { impl HttpClient for NgxHttpClient<'_> { type Error = HttpClientError; - async fn request(&self, mut req: Request) -> Result, Self::Error> + async fn request(&self, req: Request) -> Result, Self::Error> where B: Body + Send + 'static, ::Data: Send, ::Error: StdError + Send + Sync, { - let uri = req.uri().clone(); + // Buffer the request body so it can be retransmitted across redirects safely. + let (mut parts, body) = req.into_parts(); + let orig_method = parts.method.clone(); + let mut body_bytes = BodyExt::collect(body) + .await + .map_err(|e| HttpClientError::Body(std::boxed::Box::new(e)))? + .to_bytes(); + + let mut current_uri = parts.uri.clone(); - let authority = uri + // Basic validation of the starting URI. + let mut authority = current_uri .authority() + .cloned() .ok_or(HttpClientError::Uri("missing authority"))?; - let path_and_query = uri - .path_and_query() - .ok_or(HttpClientError::Uri("missing path"))?; + let mut redirects_followed = 0usize; + + loop { + // Prepare request for the current target by setting origin-form URI and required headers. + let path_and_query: PathAndQuery = current_uri + .path_and_query() + .cloned() + .ok_or(HttpClientError::Uri("missing path"))?; - *req.uri_mut() = path_and_query.clone().into(); + parts.uri = Uri::from(path_and_query.clone()); - { - let headers = req.headers_mut(); + // Build a fresh headers map for each attempt to avoid header leakage across hosts. + let mut headers = parts.headers.clone(); headers.insert( http::header::HOST, http::HeaderValue::from_str(authority.as_str()) @@ -134,50 +154,207 @@ impl HttpClient for NgxHttpClient<'_> { http::header::CONNECTION, http::HeaderValue::from_static("close"), ); - } - let ssl = if uri.scheme() == Some(&Scheme::HTTPS) { - Some(self.ssl.as_ref()) - } else { - None - }; + // Ensure Content-Length matches the body we will send; adjust for GET/HEAD. + let sending_method = parts.method.clone(); + let mut send_body = http_body_util::Full::new(body_bytes.clone()); + if sending_method == Method::GET || sending_method == Method::HEAD { + headers.insert( + http::header::CONTENT_LENGTH, + http::HeaderValue::from_static("0"), + ); + // Empty the body for GET/HEAD attempts. + send_body = http_body_util::Full::new(Bytes::new()); + } else { + headers.insert( + http::header::CONTENT_LENGTH, + http::HeaderValue::from_str(&body_bytes.len().to_string()) + .map_err(|_| HttpClientError::Uri("bad content length"))?, + ); + } + + let mut req = Request::new(send_body); + *req.method_mut() = parts.method.clone(); + *req.uri_mut() = parts.uri.clone(); + *req.version_mut() = parts.version; + { + let hm = req.headers_mut(); + hm.clear(); + for (k, v) in headers.iter() { + hm.insert(k, v.clone()); + } + } + *req.extensions_mut() = parts.extensions.clone(); - let mut peer = Box::pin(PeerConnection::new(self.log)?); + // Establish connection for current target. + let ssl = if current_uri.scheme() == Some(&Scheme::HTTPS) { + Some(self.ssl.as_ref()) + } else { + None + }; - peer.as_mut() - .connect_to(authority.as_str(), &self.resolver, ssl) - .await?; + let mut peer = Box::pin(PeerConnection::new(self.log)?); + peer.as_mut() + .connect_to(authority.as_str(), &self.resolver, ssl) + .await?; - if ssl.is_some() && self.ssl_verify { - if let Err(err) = peer.verify_peer() { - let _ = future::poll_fn(|cx| peer.as_mut().poll_shutdown(cx)).await; - return Err(err.into()); + if ssl.is_some() && self.ssl_verify { + if let Err(err) = peer.verify_peer() { + let _ = future::poll_fn(|cx| peer.as_mut().poll_shutdown(cx)).await; + return Err(err.into()); + } } - } - if let Some(c) = peer.connection_mut() { - c.requests += 1; - } + if let Some(c) = peer.connection_mut() { + c.requests += 1; + } + + let (mut sender, conn) = hyper::client::conn::http1::handshake(peer).await?; + + let log = self.log; + spawn(async move { + if let Err(err) = conn.await { + ngx_log_error!(NGX_LOG_WARN, log.as_ptr(), "connection error: {err}"); + } + }) + .detach(); - let (mut sender, conn) = hyper::client::conn::http1::handshake(peer).await?; + let resp = sender.send_request(req).await?; - let log = self.log; - spawn(async move { - if let Err(err) = conn.await { - ngx_log_error!(NGX_LOG_WARN, log.as_ptr(), "connection error: {err}"); + // If not a redirect, return the response with its body collected. + if !is_redirect(resp.status()) { + let (parts, body) = resp.into_parts(); + let body = http_body_util::Limited::new(body, NGX_ACME_MAX_BODY_SIZE) + .collect() + .await + .map_err(HttpClientError::Body)? + .to_bytes(); + return Ok(Response::from_parts(parts, body)); } - }) - .detach(); - let resp = sender.send_request(req).await?; - let (parts, body) = resp.into_parts(); + // For non-GET/HEAD, do not auto-follow redirects. + // ACME JWS 'url' must match the request URL; on redirect the client must re-sign and + // POST to the new Location itself (RFC 8555 §6.2). Return the 3xx so the ACME layer + // can handle it. + if !(parts.method == Method::GET || parts.method == Method::HEAD) { + let (parts_r, body) = resp.into_parts(); + let body = http_body_util::Limited::new(body, NGX_ACME_MAX_BODY_SIZE) + .collect() + .await + .map_err(HttpClientError::Body)? + .to_bytes(); + return Ok(Response::from_parts(parts_r, body)); + } - let body = http_body_util::Limited::new(body, NGX_ACME_MAX_BODY_SIZE) - .collect() - .await - .map_err(HttpClientError::Body)? - .to_bytes(); + // Handle redirect logic. + redirects_followed += 1; + if redirects_followed > MAX_REDIRECTS { + return Err(HttpClientError::Uri("too many redirects")); + } + + let location = resp + .headers() + .get(http::header::LOCATION) + .ok_or(HttpClientError::Uri("redirect without location"))?; - Ok(Response::from_parts(parts, body)) + let new_uri = resolve_location(¤t_uri, location)?; + + // Prevent HTTPS -> HTTP downgrade unless the original request was HTTP. + if current_uri.scheme() == Some(&Scheme::HTTPS) + && new_uri.scheme() == Some(&Scheme::HTTP) + { + return Err(HttpClientError::Uri("insecure redirect from https to http")); + } + + // Adjust method and body according to status code per RFC 9110. + match resp.status() { + StatusCode::SEE_OTHER => { + // 303: switch to GET, except HEAD remains HEAD + parts.method = if orig_method == Method::HEAD { + Method::HEAD + } else { + Method::GET + }; + body_bytes = Bytes::new(); + // Drop content-type if any + parts.headers.remove(http::header::CONTENT_TYPE); + } + StatusCode::MOVED_PERMANENTLY | StatusCode::FOUND => { + // 301/302: preserve method and body + parts.method = orig_method.clone(); + } + StatusCode::PERMANENT_REDIRECT | StatusCode::TEMPORARY_REDIRECT => { + // 308/307: MUST NOT change method + // no-op; parts.method unchanged + } + _ => {} + } + + // Update target for next loop iteration. + authority = new_uri + .authority() + .cloned() + .ok_or(HttpClientError::Uri("bad redirect authority"))?; + current_uri = new_uri; + } + } +} + +fn is_redirect(status: StatusCode) -> bool { + matches!( + status, + StatusCode::MOVED_PERMANENTLY + | StatusCode::FOUND + | StatusCode::SEE_OTHER + | StatusCode::TEMPORARY_REDIRECT + | StatusCode::PERMANENT_REDIRECT + ) +} + +fn resolve_location(base: &Uri, location: &http::HeaderValue) -> Result { + let loc_str = location + .to_str() + .map_err(|_| HttpClientError::Uri("bad location header"))?; + + // Absolute URI + if let Ok(uri) = loc_str.parse::() { + if uri.scheme().is_some() && uri.authority().is_some() { + return Ok(uri); + } + } + + // Relative forms + if loc_str.starts_with('/') { + // absolute-path reference + let mut builder = Uri::builder(); + if let Some(scheme) = base.scheme() { + builder = builder.scheme(scheme.clone()); + } + if let Some(auth) = base.authority() { + builder = builder.authority(auth.clone()); + } + builder + .path_and_query(loc_str) + .build() + .map_err(|_| HttpClientError::Uri("bad redirect path")) + } else { + // path-relative; join with base path directory + let base_path = base.path_and_query().map(|pq| pq.as_str()).unwrap_or("/"); + let joined = if let Some(idx) = base_path.rfind('/') { + format!("{}{}", &base_path[..=idx], loc_str) + } else { + format!("/{}", loc_str) + }; + let mut builder = Uri::builder(); + if let Some(scheme) = base.scheme() { + builder = builder.scheme(scheme.clone()); + } + if let Some(auth) = base.authority() { + builder = builder.authority(auth.clone()); + } + builder + .path_and_query(joined.as_str()) + .build() + .map_err(|_| HttpClientError::Uri("bad relative redirect path")) } } From a508f09c3ec8adb37d69aaa38f476e645cf9d760 Mon Sep 17 00:00:00 2001 From: Marian Degel Date: Fri, 7 Nov 2025 14:01:40 +0000 Subject: [PATCH 2/4] Revert "Refactor HttpClient request handling to support redirect logic." This reverts commit 1acf3614e31d998af49bd39fdec56ed420f27a35. --- src/net/http.rs | 265 ++++++++---------------------------------------- 1 file changed, 44 insertions(+), 221 deletions(-) diff --git a/src/net/http.rs b/src/net/http.rs index 4948aa5..9152adc 100644 --- a/src/net/http.rs +++ b/src/net/http.rs @@ -9,8 +9,8 @@ use core::ptr::NonNull; use std::io; use bytes::Bytes; -use http::uri::{PathAndQuery, Scheme}; -use http::{Method, Request, Response, StatusCode, Uri}; +use http::uri::Scheme; +use http::{Request, Response}; use http_body::Body; use http_body_util::BodyExt; use nginx_sys::{ngx_log_t, ngx_resolver_t, NGX_LOG_WARN}; @@ -18,8 +18,6 @@ use ngx::allocator::Box; use ngx::async_::resolver::Resolver; use ngx::async_::spawn; use ngx::ngx_log_error; -use std::format; -use std::string::ToString; use thiserror::Error; use super::peer_conn::PeerConnection; @@ -42,9 +40,6 @@ const NGX_ACME_USER_AGENT: &str = constcat::concat!( NGINX_VER, ); -// Maximum number of redirects to follow for a single request. -const MAX_REDIRECTS: usize = 10; - #[allow(async_fn_in_trait)] pub trait HttpClient { type Error: StdError + Send + Sync + 'static; @@ -106,41 +101,26 @@ impl<'a> NgxHttpClient<'a> { impl HttpClient for NgxHttpClient<'_> { type Error = HttpClientError; - async fn request(&self, req: Request) -> Result, Self::Error> + async fn request(&self, mut req: Request) -> Result, Self::Error> where B: Body + Send + 'static, ::Data: Send, ::Error: StdError + Send + Sync, { - // Buffer the request body so it can be retransmitted across redirects safely. - let (mut parts, body) = req.into_parts(); - let orig_method = parts.method.clone(); - let mut body_bytes = BodyExt::collect(body) - .await - .map_err(|e| HttpClientError::Body(std::boxed::Box::new(e)))? - .to_bytes(); - - let mut current_uri = parts.uri.clone(); + let uri = req.uri().clone(); - // Basic validation of the starting URI. - let mut authority = current_uri + let authority = uri .authority() - .cloned() .ok_or(HttpClientError::Uri("missing authority"))?; - let mut redirects_followed = 0usize; - - loop { - // Prepare request for the current target by setting origin-form URI and required headers. - let path_and_query: PathAndQuery = current_uri - .path_and_query() - .cloned() - .ok_or(HttpClientError::Uri("missing path"))?; + let path_and_query = uri + .path_and_query() + .ok_or(HttpClientError::Uri("missing path"))?; - parts.uri = Uri::from(path_and_query.clone()); + *req.uri_mut() = path_and_query.clone().into(); - // Build a fresh headers map for each attempt to avoid header leakage across hosts. - let mut headers = parts.headers.clone(); + { + let headers = req.headers_mut(); headers.insert( http::header::HOST, http::HeaderValue::from_str(authority.as_str()) @@ -154,207 +134,50 @@ impl HttpClient for NgxHttpClient<'_> { http::header::CONNECTION, http::HeaderValue::from_static("close"), ); + } - // Ensure Content-Length matches the body we will send; adjust for GET/HEAD. - let sending_method = parts.method.clone(); - let mut send_body = http_body_util::Full::new(body_bytes.clone()); - if sending_method == Method::GET || sending_method == Method::HEAD { - headers.insert( - http::header::CONTENT_LENGTH, - http::HeaderValue::from_static("0"), - ); - // Empty the body for GET/HEAD attempts. - send_body = http_body_util::Full::new(Bytes::new()); - } else { - headers.insert( - http::header::CONTENT_LENGTH, - http::HeaderValue::from_str(&body_bytes.len().to_string()) - .map_err(|_| HttpClientError::Uri("bad content length"))?, - ); - } - - let mut req = Request::new(send_body); - *req.method_mut() = parts.method.clone(); - *req.uri_mut() = parts.uri.clone(); - *req.version_mut() = parts.version; - { - let hm = req.headers_mut(); - hm.clear(); - for (k, v) in headers.iter() { - hm.insert(k, v.clone()); - } - } - *req.extensions_mut() = parts.extensions.clone(); - - // Establish connection for current target. - let ssl = if current_uri.scheme() == Some(&Scheme::HTTPS) { - Some(self.ssl.as_ref()) - } else { - None - }; - - let mut peer = Box::pin(PeerConnection::new(self.log)?); - peer.as_mut() - .connect_to(authority.as_str(), &self.resolver, ssl) - .await?; - - if ssl.is_some() && self.ssl_verify { - if let Err(err) = peer.verify_peer() { - let _ = future::poll_fn(|cx| peer.as_mut().poll_shutdown(cx)).await; - return Err(err.into()); - } - } - - if let Some(c) = peer.connection_mut() { - c.requests += 1; - } - - let (mut sender, conn) = hyper::client::conn::http1::handshake(peer).await?; - - let log = self.log; - spawn(async move { - if let Err(err) = conn.await { - ngx_log_error!(NGX_LOG_WARN, log.as_ptr(), "connection error: {err}"); - } - }) - .detach(); + let ssl = if uri.scheme() == Some(&Scheme::HTTPS) { + Some(self.ssl.as_ref()) + } else { + None + }; - let resp = sender.send_request(req).await?; + let mut peer = Box::pin(PeerConnection::new(self.log)?); - // If not a redirect, return the response with its body collected. - if !is_redirect(resp.status()) { - let (parts, body) = resp.into_parts(); - let body = http_body_util::Limited::new(body, NGX_ACME_MAX_BODY_SIZE) - .collect() - .await - .map_err(HttpClientError::Body)? - .to_bytes(); - return Ok(Response::from_parts(parts, body)); - } + peer.as_mut() + .connect_to(authority.as_str(), &self.resolver, ssl) + .await?; - // For non-GET/HEAD, do not auto-follow redirects. - // ACME JWS 'url' must match the request URL; on redirect the client must re-sign and - // POST to the new Location itself (RFC 8555 §6.2). Return the 3xx so the ACME layer - // can handle it. - if !(parts.method == Method::GET || parts.method == Method::HEAD) { - let (parts_r, body) = resp.into_parts(); - let body = http_body_util::Limited::new(body, NGX_ACME_MAX_BODY_SIZE) - .collect() - .await - .map_err(HttpClientError::Body)? - .to_bytes(); - return Ok(Response::from_parts(parts_r, body)); + if ssl.is_some() && self.ssl_verify { + if let Err(err) = peer.verify_peer() { + let _ = future::poll_fn(|cx| peer.as_mut().poll_shutdown(cx)).await; + return Err(err.into()); } + } - // Handle redirect logic. - redirects_followed += 1; - if redirects_followed > MAX_REDIRECTS { - return Err(HttpClientError::Uri("too many redirects")); - } - - let location = resp - .headers() - .get(http::header::LOCATION) - .ok_or(HttpClientError::Uri("redirect without location"))?; - - let new_uri = resolve_location(¤t_uri, location)?; + if let Some(c) = peer.connection_mut() { + c.requests += 1; + } - // Prevent HTTPS -> HTTP downgrade unless the original request was HTTP. - if current_uri.scheme() == Some(&Scheme::HTTPS) - && new_uri.scheme() == Some(&Scheme::HTTP) - { - return Err(HttpClientError::Uri("insecure redirect from https to http")); - } + let (mut sender, conn) = hyper::client::conn::http1::handshake(peer).await?; - // Adjust method and body according to status code per RFC 9110. - match resp.status() { - StatusCode::SEE_OTHER => { - // 303: switch to GET, except HEAD remains HEAD - parts.method = if orig_method == Method::HEAD { - Method::HEAD - } else { - Method::GET - }; - body_bytes = Bytes::new(); - // Drop content-type if any - parts.headers.remove(http::header::CONTENT_TYPE); - } - StatusCode::MOVED_PERMANENTLY | StatusCode::FOUND => { - // 301/302: preserve method and body - parts.method = orig_method.clone(); - } - StatusCode::PERMANENT_REDIRECT | StatusCode::TEMPORARY_REDIRECT => { - // 308/307: MUST NOT change method - // no-op; parts.method unchanged - } - _ => {} + let log = self.log; + spawn(async move { + if let Err(err) = conn.await { + ngx_log_error!(NGX_LOG_WARN, log.as_ptr(), "connection error: {err}"); } + }) + .detach(); - // Update target for next loop iteration. - authority = new_uri - .authority() - .cloned() - .ok_or(HttpClientError::Uri("bad redirect authority"))?; - current_uri = new_uri; - } - } -} - -fn is_redirect(status: StatusCode) -> bool { - matches!( - status, - StatusCode::MOVED_PERMANENTLY - | StatusCode::FOUND - | StatusCode::SEE_OTHER - | StatusCode::TEMPORARY_REDIRECT - | StatusCode::PERMANENT_REDIRECT - ) -} - -fn resolve_location(base: &Uri, location: &http::HeaderValue) -> Result { - let loc_str = location - .to_str() - .map_err(|_| HttpClientError::Uri("bad location header"))?; + let resp = sender.send_request(req).await?; + let (parts, body) = resp.into_parts(); - // Absolute URI - if let Ok(uri) = loc_str.parse::() { - if uri.scheme().is_some() && uri.authority().is_some() { - return Ok(uri); - } - } + let body = http_body_util::Limited::new(body, NGX_ACME_MAX_BODY_SIZE) + .collect() + .await + .map_err(HttpClientError::Body)? + .to_bytes(); - // Relative forms - if loc_str.starts_with('/') { - // absolute-path reference - let mut builder = Uri::builder(); - if let Some(scheme) = base.scheme() { - builder = builder.scheme(scheme.clone()); - } - if let Some(auth) = base.authority() { - builder = builder.authority(auth.clone()); - } - builder - .path_and_query(loc_str) - .build() - .map_err(|_| HttpClientError::Uri("bad redirect path")) - } else { - // path-relative; join with base path directory - let base_path = base.path_and_query().map(|pq| pq.as_str()).unwrap_or("/"); - let joined = if let Some(idx) = base_path.rfind('/') { - format!("{}{}", &base_path[..=idx], loc_str) - } else { - format!("/{}", loc_str) - }; - let mut builder = Uri::builder(); - if let Some(scheme) = base.scheme() { - builder = builder.scheme(scheme.clone()); - } - if let Some(auth) = base.authority() { - builder = builder.authority(auth.clone()); - } - builder - .path_and_query(joined.as_str()) - .build() - .map_err(|_| HttpClientError::Uri("bad relative redirect path")) + Ok(Response::from_parts(parts, body)) } } From 264d08d6e9a98c7dde8037a74325e9416ff29321 Mon Sep 17 00:00:00 2001 From: Marian Degel Date: Fri, 7 Nov 2025 14:16:45 +0000 Subject: [PATCH 3/4] Add redirect handling to AcmeClient and including relevant error reporting. --- src/acme.rs | 32 ++++++++++++++++++++++++++------ src/acme/error.rs | 9 +++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/acme.rs b/src/acme.rs index 6876326..4a87d31 100644 --- a/src/acme.rs +++ b/src/acme.rs @@ -43,6 +43,9 @@ const MAX_SERVER_RETRY_INTERVAL: Duration = Duration::from_secs(60); static REPLAY_NONCE: http::HeaderName = http::HeaderName::from_static("replay-nonce"); +// Maximum number of redirects to follow for a single request. +const MAX_REDIRECTS: usize = 10; + pub enum NewAccountOutput<'a> { Created(&'a str), Found(&'a str), @@ -170,12 +173,29 @@ where } pub async fn get(&self, url: &Uri) -> Result, RequestError> { - let req = http::Request::builder() - .uri(url) - .method(http::Method::GET) - .header(http::header::CONTENT_LENGTH, 0) - .body(String::new())?; - Ok(self.http.request(req).await?) + let mut u = url.clone(); + + for _ in 0..MAX_REDIRECTS { + let req = http::Request::builder() + .uri(&u) + .method(http::Method::GET) + .header(http::header::CONTENT_LENGTH, 0) + .body(String::new())?; + let res = self.http.request(req).await?; + + if res.status().is_redirection() { + if let Some(location) = try_get_header(res.headers(), http::header::LOCATION) { + u = Uri::try_from(location).map_err(RequestError::UrlParse)?; + continue; + } else { + return Err(RequestError::RedirectNoLocation); + } + } + + return Ok(res); + } + + Err(RequestError::TooManyRedirects) } pub async fn post>( diff --git a/src/acme/error.rs b/src/acme/error.rs index 6e0e626..0dc5700 100644 --- a/src/acme/error.rs +++ b/src/acme/error.rs @@ -145,6 +145,15 @@ pub enum RequestError { #[error("cannot sign request body ({0})")] Sign(#[from] crate::jws::Error), + + #[error("cannot parse URL ({0})")] + UrlParse(#[from] http::uri::InvalidUri), + + #[error("redirect response missing Location header")] + RedirectNoLocation, + + #[error("too many redirects")] + TooManyRedirects, } impl From for RequestError { From d8d7a08e6bcb35c6608e47211afb1282fa1acaf5 Mon Sep 17 00:00:00 2001 From: Marian Degel Date: Sat, 8 Nov 2025 16:07:15 +0000 Subject: [PATCH 4/4] Add redirect error handling and new IRI dependency. --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + src/acme.rs | 14 ++++++++++---- src/acme/error.rs | 28 ++++++++++++++++++++-------- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2464963..6fd758c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -270,6 +270,16 @@ dependencies = [ "want", ] +[[package]] +name = "iri-string" +version = "0.7.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f867b9d1d896b67beb18518eda36fdb77a32ea590de864f1325b294a6d14397" +dependencies = [ + "memchr", + "serde", +] + [[package]] name = "itertools" version = "0.13.0" @@ -341,6 +351,7 @@ dependencies = [ "http-body-util", "http-serde", "hyper", + "iri-string", "libc", "nginx-sys", "ngx", diff --git a/Cargo.toml b/Cargo.toml index 925ef24..12672b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ http-body = "1.0.1" http-body-util = "0.1.3" http-serde = "2.1.1" hyper = { version = "1.6.0", features = ["client", "http1"] } +iri-string = "0.7.9" libc = "0.2.174" nginx-sys = "0.5.0" ngx = { version = "0.5.0", features = ["async", "serde", "std"] } diff --git a/src/acme.rs b/src/acme.rs index 4a87d31..415a88b 100644 --- a/src/acme.rs +++ b/src/acme.rs @@ -10,8 +10,9 @@ use std::collections::VecDeque; use std::string::{String, ToString}; use bytes::Bytes; -use error::{NewAccountError, NewCertificateError, RequestError}; +use error::{NewAccountError, NewCertificateError, RedirectError, RequestError}; use http::Uri; +use iri_string::types::{UriAbsoluteString, UriReferenceStr}; use ngx::allocator::{Allocator, Box}; use ngx::async_::sleep; use ngx::collections::Vec; @@ -185,17 +186,22 @@ where if res.status().is_redirection() { if let Some(location) = try_get_header(res.headers(), http::header::LOCATION) { - u = Uri::try_from(location).map_err(RequestError::UrlParse)?; + let base = UriAbsoluteString::try_from(u.to_string()) + .map_err(RedirectError::InvalidBase)?; + let location_ref = + UriReferenceStr::new(location).map_err(RedirectError::InvalidLocation)?; + let resolved = location_ref.resolve_against(&base).to_string(); + u = Uri::try_from(resolved).map_err(RedirectError::InvalidUri)?; continue; } else { - return Err(RequestError::RedirectNoLocation); + return Err(RedirectError::MissingLocation.into()); } } return Ok(res); } - Err(RequestError::TooManyRedirects) + Err(RedirectError::TooManyRedirects.into()) } pub async fn post>( diff --git a/src/acme/error.rs b/src/acme/error.rs index 0dc5700..b45f62c 100644 --- a/src/acme/error.rs +++ b/src/acme/error.rs @@ -114,6 +114,24 @@ impl NewCertificateError { } } +#[derive(Debug, Error)] +pub enum RedirectError { + #[error("missing Location header")] + MissingLocation, + + #[error("too many redirects")] + TooManyRedirects, + + #[error("invalid base URI (IRI creation)")] + InvalidBase(#[source] iri_string::types::CreationError), + + #[error("invalid redirect location (IRI validation)")] + InvalidLocation(#[source] iri_string::validate::Error), + + #[error("invalid resolved redirect URI")] + InvalidUri(#[source] http::uri::InvalidUri), +} + #[derive(Debug, Error)] pub enum RequestError { #[error(transparent)] @@ -146,14 +164,8 @@ pub enum RequestError { #[error("cannot sign request body ({0})")] Sign(#[from] crate::jws::Error), - #[error("cannot parse URL ({0})")] - UrlParse(#[from] http::uri::InvalidUri), - - #[error("redirect response missing Location header")] - RedirectNoLocation, - - #[error("too many redirects")] - TooManyRedirects, + #[error("redirect failed: {0}")] + Redirect(#[from] RedirectError), } impl From for RequestError {