Add support for relative links (#1489)

This commit introduces several improvements to the file checking process and URI handling: - Extract file checking logic into separate `Checker` structs (`FileChecker`, `WebsiteChecker`, `MailChecker`) - Improve handling of relative and absolute file paths - Enhance URI parsing and creation from file paths - Refactor `create_request` function for better clarity and error handling These changes provide better support for resolving relative links, handling different base URLs, and working with file paths. Fixes #1296 and #1480
lycheeverse · Oct 26, 2024 · 3094bbc · 3094bbc
1 parent 87d5b56
commit 3094bbc
Show file tree

Hide file tree

Showing 21 changed files with 1,134 additions and 523 deletions.
diff --git a/fixtures/resolve_paths/about/index.html b/fixtures/resolve_paths/about/index.html
@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <title>About</title>
+  </head>
+  <body>
+    <h1 id="fragment">About</h1>
+  </body>
+</html>
diff --git a/fixtures/resolve_paths/another page/index.html b/fixtures/resolve_paths/another page/index.html
diff --git a/fixtures/resolve_paths/index.html b/fixtures/resolve_paths/index.html
@@ -0,0 +1,21 @@
+<html>
+    <head>
+        <title>Index</title>
+    </head>
+    <body>
+        <h1>Index Title</h1>
+        <p>
+            <ul>
+                <li>
+                    <a href="/">home</a>
+                </li>
+                <li>
+                    <a href="/about">About</a>
+                </li>
+                <li>
+                    <a href="/another page">About</a>
+                </li>
+            </ul>
+        </p>
+    </body>
+</html>
diff --git a/lychee-bin/src/client.rs b/lychee-bin/src/client.rs
@@ -55,6 +55,7 @@ pub(crate) fn create(cfg: &Config, cookie_jar: Option<&Arc<CookieStoreMutex>>) -
 
     ClientBuilder::builder()
         .remaps(remaps)
+        .base(cfg.base.clone())
         .includes(includes)
         .excludes(excludes)
         .exclude_all_private(cfg.exclude_all_private)

diff --git a/lychee-bin/src/commands/check.rs b/lychee-bin/src/commands/check.rs
@@ -429,14 +429,12 @@ mod tests {
 
     #[tokio::test]
     async fn test_invalid_url() {
-        // Run a normal request with an invalid Url
         let client = ClientBuilder::builder().build().client().unwrap();
-        let request = Request::try_from("http://\"").unwrap();
-        let response = check_url(&client, request).await;
-        assert!(response.status().is_error());
+        let uri = Uri::try_from("http://\"").unwrap();
+        let response = client.check_website(&uri, None).await.unwrap();
         assert!(matches!(
-            response.status(),
-            Status::Error(ErrorKind::InvalidURI(_))
+            response,
+            Status::Unsupported(ErrorKind::BuildRequestClient(_))
         ));
     }
 

diff --git a/lychee-bin/src/formatters/response/color.rs b/lychee-bin/src/formatters/response/color.rs
@@ -65,12 +65,18 @@ mod tests {
         }
     }
 
+    #[cfg(test)]
+    /// Helper function to strip ANSI color codes for tests
+    fn strip_ansi_codes(s: &str) -> String {
+        console::strip_ansi_codes(s).to_string()
+    }
+
     #[test]
     fn test_format_response_with_ok_status() {
         let formatter = ColorFormatter;
         let body = mock_response_body(Status::Ok(StatusCode::OK), "https://example.com");
         assert_eq!(
-            formatter.format_response(&body),
+            strip_ansi_codes(&formatter.format_response(&body)),
             "     [200] https://example.com/"
         );
     }
@@ -83,7 +89,7 @@ mod tests {
             "https://example.com/404",
         );
         assert_eq!(
-            formatter.format_response(&body),
+            strip_ansi_codes(&formatter.format_response(&body)),
             "   [ERROR] https://example.com/404"
         );
     }

diff --git a/lychee-bin/tests/cli.rs b/lychee-bin/tests/cli.rs
@@ -267,17 +267,17 @@ mod cli {
     #[test]
     fn test_resolve_paths() {
         let mut cmd = main_command();
-        let offline_dir = fixtures_path().join("offline");
+        let dir = fixtures_path().join("resolve_paths");
 
         cmd.arg("--offline")
             .arg("--base")
-            .arg(&offline_dir)
-            .arg(offline_dir.join("index.html"))
+            .arg(&dir)
+            .arg(dir.join("index.html"))
             .env_clear()
             .assert()
             .success()
-            .stdout(contains("4 Total"))
-            .stdout(contains("4 OK"));
+            .stdout(contains("3 Total"))
+            .stdout(contains("3 OK"));
     }
 
     #[test]
@@ -944,13 +944,17 @@ mod cli {
 
         // check content of cache file
         let data = fs::read_to_string(&cache_file)?;
+
+        if data.is_empty() {
+            println!("Cache file is empty!");
+        }
+
         assert!(data.contains(&format!("{}/,200", mock_server_ok.uri())));
         assert!(!data.contains(&format!("{}/,204", mock_server_no_content.uri())));
         assert!(!data.contains(&format!("{}/,429", mock_server_too_many_requests.uri())));
 
         // clear the cache file
         fs::remove_file(&cache_file)?;
-
         Ok(())
     }
 
@@ -1216,8 +1220,9 @@ mod cli {
         Ok(())
     }
 
-    /// If base-dir is not set, don't throw an error in case we encounter
-    /// an absolute local link within a file (e.g. `/about`).
+    /// If `base-dir` is not set, don't throw an error in case we encounter
+    /// an absolute local link (e.g. `/about`) within a file.
+    /// Instead, simply ignore the link.
     #[test]
     fn test_ignore_absolute_local_links_without_base() -> Result<()> {
         let mut cmd = main_command();
@@ -1409,9 +1414,7 @@ mod cli {
             .arg("./NOT-A-REAL-TEST-FIXTURE.md")
             .assert()
             .failure()
-            .stderr(contains(
-                "Cannot find local file ./NOT-A-REAL-TEST-FIXTURE.md",
-            ));
+            .stderr(contains("Invalid file path: ./NOT-A-REAL-TEST-FIXTURE.md"));
 
         Ok(())
     }
@@ -1667,4 +1670,46 @@ mod cli {
             .success()
             .stdout(contains("0 Errors"));
     }
+
+    /// Test relative paths
+    ///
+    /// Imagine a web server hosting a site with the following structure:
+    /// root
+    /// └── test
+    ///     ├── index.html
+    ///     └── next.html
+    ///
+    /// where `root/test/index.html` contains `<a href="next.html">next</a>`
+    /// When checking the link in `root/test/index.html` we should be able to
+    /// resolve the relative path to `root/test/next.html`
+    ///
+    /// Note that the relative path is not resolved to the root of the server
+    /// but relative to the file that contains the link.
+    #[tokio::test]
+    async fn test_resolve_relative_paths_in_subfolder() -> Result<()> {
+        let mock_server = wiremock::MockServer::start().await;
+
+        let body = r#"<a href="next.html">next</a>"#;
+        wiremock::Mock::given(wiremock::matchers::method("GET"))
+            .and(wiremock::matchers::path("/test/index.html"))
+            .respond_with(wiremock::ResponseTemplate::new(200).set_body_string(body))
+            .mount(&mock_server)
+            .await;
+
+        wiremock::Mock::given(wiremock::matchers::method("GET"))
+            .and(wiremock::matchers::path("/test/next.html"))
+            .respond_with(wiremock::ResponseTemplate::new(200))
+            .mount(&mock_server)
+            .await;
+
+        let mut cmd = main_command();
+        cmd.arg("--verbose")
+            .arg(format!("{}/test/index.html", mock_server.uri()))
+            .assert()
+            .success()
+            .stdout(contains("1 Total"))
+            .stdout(contains("0 Errors"));
+
+        Ok(())
+    }
 }
diff --git a/lychee-lib/src/checker.rs b/lychee-lib/src/checker.rs