diff --git a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs index 8edac3cace..9316f4e3f5 100644 --- a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs +++ b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs @@ -245,6 +245,7 @@ impl HtmlHandlebars { } debug!("Emitting redirects"); + detect_redirect_loops(redirects)?; let redirects = combine_fragment_redirects(redirects); for (original, (dest, fragment_map)) in redirects { @@ -639,6 +640,69 @@ struct RenderChapterContext<'a> { chapter_titles: &'a HashMap, } +/// Returns the redirect source path used in `[output.html.redirect]` keys. +fn canonical_redirect_source(path: &str) -> String { + if path.starts_with('/') { + path.to_string() + } else { + format!("/{path}") + } +} + +fn redirect_destination_is_external(dest: &str) -> bool { + let dest = dest.trim(); + dest.starts_with("http://") || dest.starts_with("https://") || dest.starts_with("//") +} + +/// Detects cycles in `[output.html.redirect]` before emitting redirect pages. +fn detect_redirect_loops(redirects: &HashMap) -> Result<()> { + use std::collections::HashSet; + + let mut visited = HashSet::new(); + let mut sources: Vec<_> = redirects.keys().collect(); + sources.sort(); + for start in sources { + if visited.contains(start) { + continue; + } + let mut path: Vec<&str> = Vec::new(); + let mut current = start.as_str(); + loop { + if let Some(pos) = path.iter().position(|&p| p == current) { + let mut cycle = String::new(); + for source in &path[pos..] { + if !cycle.is_empty() { + cycle.push_str(" -> "); + } + cycle.push_str(source); + if let Some(dest) = redirects.get(*source) { + cycle.push_str(" -> "); + cycle.push_str(dest); + } + } + bail!("redirect loop detected: {cycle}"); + } + visited.insert(current.to_owned()); + path.push(current); + let Some(dest) = redirects.get(current) else { + break; + }; + if redirect_destination_is_external(dest) { + break; + } + let canonical = canonical_redirect_source(dest); + let Some((next, _)) = redirects + .get_key_value(&canonical) + .or_else(|| redirects.get_key_value(dest)) + else { + break; + }; + current = next.as_str(); + } + } + Ok(()) +} + /// Redirect mapping. /// /// The key is the source path (like `foo/bar.html`). The value is a tuple diff --git a/tests/testsuite/redirects.rs b/tests/testsuite/redirects.rs index 84c776832d..8e880ffa16 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 at build time. +#[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_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