Fix --max-redirects (#987)

Having more than the max number of redirects
caused lychee to abort the requests, but did not
lead to an error.

Related: https://github.com/lycheeverse/lychee-action/issues/164

lychee-bin/src/options.rs

@@ -42,8 +42,9 @@ const HELP_MSG_CONFIG_FILE: &str = formatcp!(
 const TIMEOUT_STR: &str = concatcp!(DEFAULT_TIMEOUT_SECS);
 const RETRY_WAIT_TIME_STR: &str = concatcp!(DEFAULT_RETRY_WAIT_TIME_SECS);
 
-#[derive(Debug, Deserialize, Clone)]
+#[derive(Debug, Deserialize, Default, Clone)]
 pub(crate) enum Format {
+    #[default]
     Compact,
     Detailed,
     Json,
@@ -65,12 +66,6 @@ impl FromStr for Format {
     }
 }
 
-impl Default for Format {
-    fn default() -> Self {
-        Format::Compact
-    }
-}
-
 // Macro for generating default functions to be used by serde
 macro_rules! default_function {
     ( $( $name:ident : $T:ty = $e:expr; )* ) => {

lychee-bin/tests/cli.rs

@@ -1159,4 +1159,19 @@ mod cli {
 
         Ok(())
     }
+
+    #[test]
+    fn test_prevent_too_many_redirects() -> Result<()> {
+        let mut cmd = main_command();
+        let url = "https://httpstat.us/308";
+
+        cmd.write_stdin(url)
+            .arg("--max-redirects")
+            .arg("0")
+            .arg("-")
+            .assert()
+            .failure();
+
+        Ok(())
+    }
 }

lychee-lib/src/client.rs

@@ -643,6 +643,7 @@ mod tests {
     use http::{header::HeaderMap, StatusCode};
     use reqwest::header;
     use tempfile::tempdir;
+    use wiremock::matchers::path;
 
     use super::ClientBuilder;
     use crate::{mock_server, test_utils::get_mock_client_response, Uri};
@@ -827,4 +828,68 @@ mod tests {
         let res = client.check("http://\"").await.unwrap();
         assert!(res.status().is_failure());
     }
+
+    #[tokio::test]
+    async fn test_max_redirects() {
+        let mock_server = wiremock::MockServer::start().await;
+
+        let ok_uri = format!("{}/ok", &mock_server.uri());
+        let redirect_uri = format!("{}/redirect", &mock_server.uri());
+
+        // Set up permanent redirect loop
+        let redirect = wiremock::ResponseTemplate::new(StatusCode::PERMANENT_REDIRECT)
+            .insert_header("Location", ok_uri.as_str());
+        wiremock::Mock::given(wiremock::matchers::method("GET"))
+            .and(path("/redirect"))
+            .respond_with(redirect)
+            .mount(&mock_server)
+            .await;
+
+        let ok = wiremock::ResponseTemplate::new(StatusCode::OK);
+        wiremock::Mock::given(wiremock::matchers::method("GET"))
+            .and(path("/ok"))
+            .respond_with(ok)
+            .mount(&mock_server)
+            .await;
+
+        let client = ClientBuilder::builder()
+            .max_redirects(1_usize)
+            .build()
+            .client()
+            .unwrap();
+
+        let res = client.check(redirect_uri.clone()).await.unwrap();
+        assert!(res.status().is_failure());
+
+        let client = ClientBuilder::builder()
+            .max_redirects(2_usize)
+            .build()
+            .client()
+            .unwrap();
+
+        let res = client.check(redirect_uri).await.unwrap();
+        assert!(res.status().is_success());
+    }
+
+    #[tokio::test]
+    async fn test_limit_max_redirects() {
+        let mock_server = wiremock::MockServer::start().await;
+
+        // Set up permanent redirect loop
+        let template = wiremock::ResponseTemplate::new(StatusCode::PERMANENT_REDIRECT)
+            .insert_header("Location", mock_server.uri().as_str());
+        wiremock::Mock::given(wiremock::matchers::method("GET"))
+            .respond_with(template)
+            .mount(&mock_server)
+            .await;
+
+        let client = ClientBuilder::builder()
+            .max_redirects(0_usize)
+            .build()
+            .client()
+            .unwrap();
+
+        let res = client.check(mock_server.uri()).await.unwrap();
+        assert!(res.status().is_failure());
+    }
 }

lychee-lib/src/types/error.rs

@@ -99,6 +99,9 @@ pub enum ErrorKind {
     /// Regex error
     #[error("Error when using regex engine: {0}")]
     Regex(#[from] regex::Error),
+    /// Too many redirects (HTTP 3xx) were encountered (configurable)
+    #[error("Too many redirects")]
+    TooManyRedirects(#[source] reqwest::Error),
 }
 
 impl ErrorKind {
@@ -210,6 +213,7 @@ impl Hash for ErrorKind {
                 std::mem::discriminant(self).hash(state);
             }
             Self::Regex(e) => e.to_string().hash(state),
+            Self::TooManyRedirects(e) => e.to_string().hash(state),
         }
     }
 }

lychee-lib/src/types/status.rs

@@ -242,6 +242,8 @@ impl From<reqwest::Error> for Status {
     fn from(e: reqwest::Error) -> Self {
         if e.is_timeout() {
             Self::Timeout(e.status())
+        } else if e.is_redirect() {
+            Self::Error(ErrorKind::TooManyRedirects(e))
         } else if e.is_builder() {
             Self::Unsupported(ErrorKind::BuildRequestClient(e))
         } else if e.is_body() || e.is_decode() {