Refactor redirect handling to improve URI resolution and error reporting

Author: mdegel

src/acme.rs

@@ -111,6 +111,13 @@ fn try_get_header<K: http::header::AsHeaderName>(
     headers.get(key).and_then(|x| x.to_str().ok())
 }
 
+fn resolve_uri(base: &Uri, relative: &str) -> Option<Uri> {
+    let base_abs = UriAbsoluteString::try_from(base.to_string()).ok()?;
+    let location_ref = UriReferenceStr::new(relative).ok()?;
+    let resolved = location_ref.resolve_against(&base_abs).to_string();
+    Uri::try_from(resolved).ok()
+}
+
 impl<'a, Http> AcmeClient<'a, Http>
 where
     Http: HttpClient,
@@ -186,15 +193,14 @@ where
 
             if res.status().is_redirection() {
                 if let Some(location) = try_get_header(res.headers(), http::header::LOCATION) {
-                    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;
+                    if let Some(new_u) = resolve_uri(&u, location) {
+                        u = new_u;
+                        continue;
+                    } else {
+                        return Err(RedirectError::InvalidRedirectUri.into());
+                    }
                 } else {
-                    return Err(RedirectError::MissingLocation.into());
+                    return Err(RedirectError::MissingRedirectUri.into());
                 }
             }
 

src/acme/error.rs

@@ -116,20 +116,14 @@ impl NewCertificateError {
 
 #[derive(Debug, Error)]
 pub enum RedirectError {
-    #[error("missing Location header")]
-    MissingLocation,
+    #[error("invalid redirect URI")]
+    InvalidRedirectUri,
+
+    #[error("missing redirect URI")]
+    MissingRedirectUri,
 
     #[error("too many redirects")]
     TooManyRedirects,
-
-    #[error("invalid base URI (IRI creation)")]
-    InvalidBase(#[source] iri_string::types::CreationError<std::string::String>),
-
-    #[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)]
@@ -155,6 +149,9 @@ pub enum RequestError {
     #[error("rate limit exceeded, next attempt in {0:?}")]
     RateLimited(Duration),
 
+    #[error("redirect failed: {0}")]
+    Redirect(#[from] RedirectError),
+
     #[error("cannot serialize request ({0})")]
     RequestFormat(serde_json::Error),
 
@@ -163,9 +160,6 @@ pub enum RequestError {
 
     #[error("cannot sign request body ({0})")]
     Sign(#[from] crate::jws::Error),
-
-    #[error("redirect failed: {0}")]
-    Redirect(#[from] RedirectError),
 }
 
 impl From<HttpClientError> for RequestError {