diff --git a/lychee-bin/src/commands/check.rs b/lychee-bin/src/commands/check.rs index fb2897a94a..40ab166b9c 100644 --- a/lychee-bin/src/commands/check.rs +++ b/lychee-bin/src/commands/check.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::pin::pin; use std::sync::Arc; use std::sync::Mutex; @@ -34,7 +34,7 @@ struct Recursion { enabled: bool, max_depth: Option, domains: Arc>, - crawled_urls: Arc>>, + crawled_urls: Arc>>, collector: Collector, extensions: FileExtensions, } @@ -50,7 +50,7 @@ impl Recursion { enabled: cfg.recursive(), max_depth: cfg.max_depth(), domains: Arc::new(input_domains), - crawled_urls: Arc::new(Mutex::new(HashSet::new())), + crawled_urls: Arc::new(Mutex::new(HashMap::new())), collector, extensions: cfg.extensions(), } @@ -68,7 +68,9 @@ impl Recursion { self.crawled_urls .lock() .unwrap() - .insert(canonical_url(source)); + .entry(canonical_url(source)) + .and_modify(|previous| *previous = (*previous).min(depth.saturating_sub(1))) + .or_insert(depth.saturating_sub(1)); } let url = response.redirects().map_or_else( @@ -89,8 +91,16 @@ impl Recursion { return Vec::new(); }; - if !self.crawled_urls.lock().unwrap().insert(url.clone()) { - return Vec::new(); + { + let mut crawled_urls = self.crawled_urls.lock().unwrap(); + if crawled_urls + .get(&url) + .is_some_and(|previous| self.max_depth.is_none() || *previous <= depth) + { + return Vec::new(); + } + // A shorter route leaves more depth available for this page's descendants. + crawled_urls.insert(url.clone(), depth); } collect_recursive_requests(self.collector.clone(), url, self.extensions.clone()) diff --git a/lychee-bin/tests/cli.rs b/lychee-bin/tests/cli.rs index 9ca2666423..0011f54766 100644 --- a/lychee-bin/tests/cli.rs +++ b/lychee-bin/tests/cli.rs @@ -2671,6 +2671,67 @@ The config file should contain every possible key for documentation purposes." .stdout(contains("0 Errors")); } + #[tokio::test] + async fn test_recursive_depth_uses_shorter_path_discovered_later() { + let server = wiremock::MockServer::start().await; + // The fast path reaches shared at depth 2, leaving too little budget + // for broken. The slower shortcut reaches it at depth 1 and must + // still allow checking the descendant within the configured limit. + for (route, body) in [ + ( + "/index.html", + r#"longshort"#, + ), + ("/a.html", r#"b"#), + ("/b.html", r#"shared"#), + ("/shortcut.html", r#"shared"#), + ("/shared.html", r#"child"#), + ("/child.html", r#"broken"#), + ] { + let mut response = ResponseTemplate::new(200).set_body_raw(body, "text/html"); + if route == "/shortcut.html" { + response = response.set_delay(Duration::from_secs(1)); + } + Mock::given(method("GET")) + .and(path(route)) + .respond_with(response) + .mount(&server) + .await; + } + Mock::given(method("GET")) + .and(path("/broken.html")) + .respond_with(ResponseTemplate::new(404)) + .mount(&server) + .await; + cargo_bin_cmd!() + .arg("--recursive") + .arg("--max-depth=3") + .arg("--max-concurrency=4") + .arg("--max-retries=0") + .arg("--verbose") + .arg(format!("{}/index.html", server.uri())) + .timeout(Duration::from_secs(20)) + .assert() + .code(2) + .stdout(contains("/broken.html")); + + // Verify that the delayed route really was extracted after the long + // route reached the depth boundary, rather than relying on timing alone. + let requests = server.received_requests().await.unwrap(); + let child = requests + .iter() + .position(|request| request.url.path() == "/child.html") + .unwrap(); + let shortcut_extraction = requests + .iter() + .enumerate() + .filter(|(_, request)| request.url.path() == "/shortcut.html") + .nth(1) + .map(|(index, _)| index) + .unwrap(); + assert!(child < shortcut_extraction); + } + async fn recursive_test_server() -> wiremock::MockServer { let mock_server = wiremock::MockServer::start().await;