diff --git a/docs/changelog.md b/docs/changelog.md index 5dc7b0f8..1cc874f5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -16,6 +16,8 @@ See the [Contributing Guide](contributing.md) for details. ### Fixed +* Fix quadratic-time id generation in the `toc` extension when many headers + share the same text. * Strip complete HTML tags with quoted `>` characters from `toc` labels and generated heading IDs. * Fix quadratic-time regex backtracking in `attr_list` when a block ends with an diff --git a/markdown/extensions/toc.py b/markdown/extensions/toc.py index e0c4c74e..ba65225c 100644 --- a/markdown/extensions/toc.py +++ b/markdown/extensions/toc.py @@ -54,14 +54,35 @@ def slugify_unicode(value: str, separator: str) -> str: IDCOUNT_RE = re.compile(r'^(.*)_([0-9]+)$') -def unique(id: str, ids: MutableSet[str]) -> str: - """ Ensure id is unique in set of ids. Append '_1', '_2'... if not """ - while id in ids or not id: +def unique(id: str, ids: MutableSet[str], counters: dict[str, int] | None = None) -> str: + """ Ensure id is unique in set of ids. Append '_1', '_2'... if not + + `counters` is an optional cache which maps a base id to the highest `n` for which + `base_1` through `base_n` are all known to be in `ids`. When the same cache is passed + along with the same `ids` on every call, a repeated id skips past the suffixes which + earlier calls already found to be taken rather than checking each of them again. + """ + if id in ids or not id: m = IDCOUNT_RE.match(id) if m: - id = '%s_%d' % (m.group(1), int(m.group(2))+1) + base, count = m.group(1), int(m.group(2)) else: - id = '%s_%d' % (id, 1) + base, count = id, 0 + skip = False + if counters is not None: + known = counters.get(base, 0) + # Only skip ahead when this id falls within the run of suffixes known to be + # taken, or is the next one after it, as any later suffix may still be free. + if count <= known or id == f'{base}_{known + 1}': + skip = True + count = max(count, known) + count += 1 + while f'{base}_{count}' in ids: + count += 1 + id = f'{base}_{count}' + if skip: + # Every suffix from `base_1` through the new id is now known to be taken. + counters[base] = count ids.add(id) return id @@ -369,6 +390,7 @@ def run(self, doc: etree.Element) -> None: for el in doc.iter(): if "id" in el.attrib: used_ids.add(el.attrib["id"]) + id_counters: dict[str, int] = {} toc_tokens = [] for el in doc.iter(): @@ -379,7 +401,7 @@ def run(self, doc: etree.Element) -> None: # Do not override pre-existing ids if "id" not in el.attrib: - el.attrib["id"] = unique(self.slugify(html.unescape(name), self.sep), used_ids) + el.attrib["id"] = unique(self.slugify(html.unescape(name), self.sep), used_ids, id_counters) data_toc_label = '' if 'data-toc-label' in el.attrib: diff --git a/tests/test_syntax/extensions/test_toc.py b/tests/test_syntax/extensions/test_toc.py index 07df442b..12a07d50 100644 --- a/tests/test_syntax/extensions/test_toc.py +++ b/tests/test_syntax/extensions/test_toc.py @@ -445,6 +445,44 @@ def testTOCUniqueFunc(self): self.assertEqual(unique('foo', ids), 'foo_1') self.assertEqual(ids, {'foo', 'foo_1'}) + def testTOCUniqueFuncCounters(self): + """ + With a cache of counters, `unique` must return the same ids it does + without one, including when ids with gaps or leading zeros exist. + """ + ids = {'foo', 'foo_1', 'foo_3'} + counters = {} + self.assertEqual(unique('foo', ids, counters), 'foo_2') + self.assertEqual(unique('foo', ids, counters), 'foo_4') + self.assertEqual(unique('foo_1', ids, counters), 'foo_5') + self.assertEqual(unique('foo_8', ids, counters), 'foo_8') + self.assertEqual(unique('foo_8', ids, counters), 'foo_9') + self.assertEqual(unique('foo', ids, counters), 'foo_6') + # `foo_07` is taken but `foo_7` is still free. + self.assertEqual(unique('foo_07', ids, counters), 'foo_07') + self.assertEqual(unique('foo_07', ids, counters), 'foo_10') + self.assertEqual(unique('foo', ids, counters), 'foo_7') + self.assertEqual(unique('foo', ids, counters), 'foo_11') + self.assertEqual(counters, {'foo': 11}) + self.assertEqual( + ids, + {'foo', 'foo_07'} | {f'foo_{i}' for i in range(1, 12)} + ) + + def testManyHeadersWithSameText(self): + """ + Each header with the same text as earlier headers gets the next free id + without checking every id already given to those headers again. + """ + count = 10000 + self.assertMarkdownRenders( + '# Header\n' * count, + '\n'.join( + ['