diff --git a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs index 8edac3cace..c07115a474 100644 --- a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs +++ b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs @@ -11,7 +11,7 @@ use mdbook_core::config::{BookConfig, Config, HtmlConfig}; use mdbook_core::utils::fs; use mdbook_renderer::{RenderContext, Renderer}; use serde_json::json; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Path, PathBuf}; use tracing::error; use tracing::{debug, info, trace, warn}; @@ -245,6 +245,7 @@ impl HtmlHandlebars { } debug!("Emitting redirects"); + validate_redirect_loops(redirects)?; let redirects = combine_fragment_redirects(redirects); for (original, (dest, fragment_map)) in redirects { @@ -639,6 +640,75 @@ struct RenderChapterContext<'a> { chapter_titles: &'a HashMap, } +/// Returns the canonical redirect map key (leading `/`, no leading `./`). +fn redirect_lookup_key(path: &str) -> String { + let path = path.trim_start_matches('/'); + format!("/{path}") +} + +fn is_external_redirect(url: &str) -> bool { + let url = url.trim(); + url.starts_with("http://") || url.starts_with("https://") || url.starts_with("//") +} + +fn find_redirect_cycle(start: &str, redirects: &HashMap) -> Option> { + let mut chain = vec![start.to_string()]; + let mut current = start.to_string(); + + loop { + let dest = redirects.get(¤t)?; + if is_external_redirect(dest) { + return None; + } + + let next = redirect_lookup_key(dest); + if let Some(loop_start) = chain.iter().position(|node| node == &next) { + let mut cycle = chain[loop_start..].to_vec(); + cycle.push(next); + return Some(cycle); + } + chain.push(next.clone()); + current = next; + } +} + +/// Rotates a redirect cycle so the lexicographically smallest node is first. +fn canonicalize_redirect_cycle(cycle: &[String]) -> Vec { + if cycle.len() <= 1 { + return cycle.to_vec(); + } + + let nodes = &cycle[..cycle.len() - 1]; + let min_idx = nodes + .iter() + .enumerate() + .min_by_key(|(_, node)| node.as_str()) + .map(|(idx, _)| idx) + .unwrap_or(0); + + let mut rotated = nodes[min_idx..].to_vec(); + rotated.extend_from_slice(&nodes[..min_idx]); + rotated.push(rotated[0].clone()); + rotated +} + +/// Detects cycles in `[output.html.redirect]` before emitting redirect pages. +fn validate_redirect_loops(redirects: &HashMap) -> Result<()> { + let mut reported = HashSet::new(); + + for start in redirects.keys() { + let Some(cycle) = find_redirect_cycle(start, redirects) else { + continue; + }; + let canonical = canonicalize_redirect_cycle(&cycle); + let signature: Vec<_> = canonical[..canonical.len() - 1].to_vec(); + if reported.insert(signature) { + bail!("redirect loop detected: {}", canonical.join(" → ")); + } + } + Ok(()) +} + /// Redirect mapping. /// /// The key is the source path (like `foo/bar.html`). The value is a tuple @@ -694,3 +764,49 @@ fn collect_redirects_for_path( .collect(); Ok(map) } + +#[cfg(test)] +mod redirect_loop_tests { + use super::*; + + #[test] + fn detects_fragment_redirect_loop() { + let redirects = HashMap::from([ + ( + "/chapter_1.html#a".to_string(), + "chapter_2.html#b".to_string(), + ), + ( + "/chapter_2.html#b".to_string(), + "chapter_1.html#a".to_string(), + ), + ]); + let err = validate_redirect_loops(&redirects).unwrap_err(); + assert!(err.to_string().contains("redirect loop detected")); + } + + #[test] + fn detects_page_redirect_loop() { + let redirects = HashMap::from([ + ("/a.html".to_string(), "b.html".to_string()), + ("/b.html".to_string(), "a.html".to_string()), + ]); + let err = validate_redirect_loops(&redirects).unwrap_err(); + assert!(err.to_string().contains("/a.html")); + } + + #[test] + fn allows_acyclic_redirect_chain() { + let redirects = HashMap::from([ + ("/a.html".to_string(), "b.html".to_string()), + ("/b.html".to_string(), "c.html".to_string()), + ]); + validate_redirect_loops(&redirects).unwrap(); + } + + #[test] + fn stops_at_external_redirect() { + let redirects = HashMap::from([("/a.html".to_string(), "https://example.com".to_string())]); + validate_redirect_loops(&redirects).unwrap(); + } +} diff --git a/tests/testsuite/redirects.rs b/tests/testsuite/redirects.rs index 84c776832d..a589280792 100644 --- a/tests/testsuite/redirects.rs +++ b/tests/testsuite/redirects.rs @@ -33,6 +33,21 @@ There must be an entry without the `#` fragment to determine the default destina }); } +// Redirect loops should fail the build. +#[test] +fn redirect_loop() { + BookTest::from_dir("redirects/redirect_loop").run("build", |cmd| { + cmd.expect_failure().expect_stderr(str![[r#" + INFO Book building has started + INFO Running the html backend +ERROR Rendering failed +[TAB]Caused by: Unable to emit redirects +[TAB]Caused by: redirect loop detected: /chapter_1.html#a → /chapter_2.html#b → /chapter_1.html#a + +"#]]); + }); +} + // Invalid redirect for an existing page. #[test] fn redirect_existing_page() { diff --git a/tests/testsuite/redirects/redirect_loop/book.toml b/tests/testsuite/redirects/redirect_loop/book.toml new file mode 100644 index 0000000000..86c15cfd81 --- /dev/null +++ b/tests/testsuite/redirects/redirect_loop/book.toml @@ -0,0 +1,6 @@ +[book] +title = "redirect_loop" + +[output.html.redirect] +"/chapter_1.html#a" = "chapter_2.html#b" +"/chapter_2.html#b" = "chapter_1.html#a" diff --git a/tests/testsuite/redirects/redirect_loop/src/SUMMARY.md b/tests/testsuite/redirects/redirect_loop/src/SUMMARY.md new file mode 100644 index 0000000000..fa67cf3e84 --- /dev/null +++ b/tests/testsuite/redirects/redirect_loop/src/SUMMARY.md @@ -0,0 +1,4 @@ +# Summary + +- [Chapter 1](./chapter_1.md) +- [Chapter 2](./chapter_2.md) diff --git a/tests/testsuite/redirects/redirect_loop/src/chapter_1.md b/tests/testsuite/redirects/redirect_loop/src/chapter_1.md new file mode 100644 index 0000000000..b743fda354 --- /dev/null +++ b/tests/testsuite/redirects/redirect_loop/src/chapter_1.md @@ -0,0 +1 @@ +# Chapter 1 diff --git a/tests/testsuite/redirects/redirect_loop/src/chapter_2.md b/tests/testsuite/redirects/redirect_loop/src/chapter_2.md new file mode 100644 index 0000000000..7ebb5961e3 --- /dev/null +++ b/tests/testsuite/redirects/redirect_loop/src/chapter_2.md @@ -0,0 +1 @@ +# Chapter 2