From f1731329e143d792e3e28c3736cabb27bdf7d095 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Wed, 17 Sep 2025 14:36:16 -0700
Subject: [PATCH] Fix ID collisions when the numeric suffix gets used

This fixes a collision with the ID generation where it a previous entry
could generate a unique ID like "foo-1", but then a header with the text
"Foo 1" would collide with it. This fixes it so that when generating the
ID for "Foo 1", it will loop unit it finds an ID that doesn't collide
(in this case, `foo-1-1`).
---
 crates/mdbook-html/src/html/print.rs          | 11 +++----
 crates/mdbook-html/src/html/tree.rs           |  2 +-
 crates/mdbook-html/src/utils.rs               | 30 +++++++++++--------
 .../header_links/expected/header_links.html   |  2 +-
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/crates/mdbook-html/src/html/print.rs b/crates/mdbook-html/src/html/print.rs
index 5996ef2b..3688a979 100644
--- a/crates/mdbook-html/src/html/print.rs
+++ b/crates/mdbook-html/src/html/print.rs
@@ -8,7 +8,7 @@ use super::Node;
 use crate::html::{ChapterTree, Element, serialize};
 use crate::utils::{ToUrlPath, id_from_content, normalize_path, unique_id};
 use mdbook_core::static_regex;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::path::{Component, PathBuf};
 
 /// Takes all the chapter trees, modifies them to be suitable to render for
@@ -42,12 +42,9 @@ pub(crate) fn render_print_page(mut chapter_trees: Vec<ChapterTree<'_>>) -> Stri
 /// been seen. This is used to generate unique IDs.
 fn make_ids_unique(
     chapter_trees: &mut [ChapterTree<'_>],
-) -> (
-    HashMap<PathBuf, HashMap<String, String>>,
-    HashMap<String, u32>,
-) {
+) -> (HashMap<PathBuf, HashMap<String, String>>, HashSet<String>) {
     let mut id_remap = HashMap::new();
-    let mut id_counter = HashMap::new();
+    let mut id_counter = HashSet::new();
     for ChapterTree {
         html_path, tree, ..
     } in chapter_trees
@@ -76,7 +73,7 @@ fn make_ids_unique(
 /// print output has something to link to.
 fn make_root_id_map(
     chapter_trees: &mut [ChapterTree<'_>],
-    id_counter: &mut HashMap<String, u32>,
+    id_counter: &mut HashSet<String>,
 ) -> HashMap<PathBuf, String> {
     let mut path_to_root_id = HashMap::new();
     for ChapterTree {
diff --git a/crates/mdbook-html/src/html/tree.rs b/crates/mdbook-html/src/html/tree.rs
index 802a215e..cd7a570b 100644
--- a/crates/mdbook-html/src/html/tree.rs
+++ b/crates/mdbook-html/src/html/tree.rs
@@ -820,7 +820,7 @@ where
     /// to all header elements, and to also add an `<a>` tag so that clicking
     /// the header will set the current URL to that header's fragment.
     fn add_header_links(&mut self) {
-        let mut id_counter = HashMap::new();
+        let mut id_counter = HashSet::new();
         let headings =
             self.node_ids_for_tag(&|name| matches!(name, "h1" | "h2" | "h3" | "h4" | "h5" | "h6"));
         for heading in headings {
diff --git a/crates/mdbook-html/src/utils.rs b/crates/mdbook-html/src/utils.rs
index 0fb54613..6c17b8d5 100644
--- a/crates/mdbook-html/src/utils.rs
+++ b/crates/mdbook-html/src/utils.rs
@@ -1,7 +1,6 @@
 //! Utilities for processing HTML.
 
-use std::collections::HashMap;
-use std::fmt::Write;
+use std::collections::HashSet;
 use std::path::{Component, Path, PathBuf};
 
 /// Utility function to normalize path elements like `..`.
@@ -54,18 +53,23 @@ impl ToUrlPath for Path {
 
 /// Make sure an HTML id is unique.
 ///
-/// The `id_counter` map is used to ensure the ID is globally unique. If the
-/// same id appears more than once, then it will have a number added to make
-/// it unique.
-pub(crate) fn unique_id(id: &str, id_counter: &mut HashMap<String, u32>) -> String {
-    let mut id = id.to_string();
-    let id_count = id_counter.entry(id.to_string()).or_insert(0);
-    if *id_count != 0 {
-        // FIXME: This should be a loop to ensure that the new ID is also unique.
-        write!(id, "-{id_count}").unwrap();
+/// Keeps a set of all previously returned IDs; if the requested id is already
+/// used, numeric suffixes (-1, -2, ...) are tried until an unused one is found.
+pub(crate) fn unique_id(id: &str, used: &mut HashSet<String>) -> String {
+    if used.insert(id.to_string()) {
+        return id.to_string();
+    }
+
+    // This ID is already in use. Generate one that is not by appending a
+    // numeric suffix.
+    let mut counter: u32 = 1;
+    loop {
+        let candidate = format!("{id}-{counter}");
+        if used.insert(candidate.clone()) {
+            return candidate;
+        }
+        counter += 1;
     }
-    *id_count += 1;
-    id
 }
 
 /// Generates an HTML id from the given text.
diff --git a/tests/testsuite/rendering/header_links/expected/header_links.html b/tests/testsuite/rendering/header_links/expected/header_links.html
index ae823201..29c35a53 100644
--- a/tests/testsuite/rendering/header_links/expected/header_links.html
+++ b/tests/testsuite/rendering/header_links/expected/header_links.html
@@ -6,5 +6,5 @@
 <h2 id="repeat"><a class="header" href="#repeat">Repeat</a></h2>
 <h2 id="repeat-1"><a class="header" href="#repeat-1">Repeat</a></h2>
 <h2 id="repeat-2"><a class="header" href="#repeat-2">Repeat</a></h2>
-<h2 id="repeat-1"><a class="header" href="#repeat-1">Repeat 1</a></h2>
+<h2 id="repeat-1-1"><a class="header" href="#repeat-1-1">Repeat 1</a></h2>
 <h2 id="with-emphasis-bold-bold_emphasis-code-escaped-html-link-httpsexamplecom"><a class="header" href="#with-emphasis-bold-bold_emphasis-code-escaped-html-link-httpsexamplecom"><!--comment--> With <em>emphasis</em> <strong>bold</strong> <strong><em>bold_emphasis</em></strong> <code>code</code> &lt;escaped&gt; <span>html</span> <a href="https://example.com/link">link</a> <a href="https://example.com/">https://example.com/</a></a></h2>
\ No newline at end of file