🐛 FIX: build-aborting crashes on hostile input, and docutils 0.23 compat#1157
Merged
Conversation
4013b0b to
341fdd1
Compare
Three defects where malformed or unlucky input aborted the whole build instead of degrading gracefully: - Deeply nested HTML: rendering the parsed HTML AST recurses once per nesting level, and the resulting RecursionError escaped the html_to_nodes try/except (which only covered tokenization), killing the build. Now caught, warning as myst.html_parse and falling back to the raw HTML. - Hostile YAML front matter: only ParserError/ScannerError were caught, so a ConstructorError (e.g. `title: !Ref X`), ComposerError, or the bare ValueError PyYAML leaks for out-of-range timestamps (`2021-99-99`) crashed the parse. Both read_topmatter and render_front_matter now catch yaml.YAMLError + ValueError and emit the intended myst.topmatter warning. - A footnote whose label matches any heading or target name was silently dropped: the duplicate-definition check tested against document.nameids, which holds every target name, not just footnote labels. It now checks against the names of previously rendered footnotes only. Also docutils 0.23 support (the only failure was test-side, docutils now attaches info-level system messages to the doctree when report_level allows): - normalize them out of the tree in test_link_resolution (they are still asserted via the captured report stream) - extend the pin to docutils<0.24 and add 0.23 to the myst-docutils CI test matrix
341fdd1 to
4f9abe9
Compare
- The footnote duplicate check now gates on `document.nametypes`:
a label colliding with any *explicit* target name (another footnote,
a `(name)=` target, a directive `:name:`, including ones created via
eval-rst) keeps the previous drop-with-warning behaviour — letting it
through would strip the name from both nodes and break previously
working references. Only collisions with *implicit* names (headings —
the actual bug) keep the footnote. This is also O(1) per definition,
where the set rebuild it replaces was O(n^2) per document
(+9.7s at 10k footnotes).
- The YAML catches move to a shared `YAML_LOAD_ERRORS` tuple that also
covers `RecursionError`: PyYAML's composer recurses per nesting
level, so deeply nested front matter (e.g. 500 nested flow sequences)
still aborted the build. Also applied to the directive-options
YAML path (only reachable with `validate_options=False`, e.g. myst-nb).
- Front matter fields are measured before being JSON-serialized for
display: YAML anchors/aliases can produce structures whose expanded
size is exponential in the source size ("billion laughs"), which
previously hung the build in `json.dumps`; self-referential
structures raised `ValueError`. Both now warn and skip the field.
- Regression tests for each, plus docstring/comment accuracy fixes.
The nesting must be a single-line flow sequence: spread over multiple lines it fails in PyYAML's (iterative) scanner — already caught before the fix — and never reaches the recursive composer. The recursion limit is pinned so the depth both overflows it and stays under docutils' line-length-limit on any runner.
This was referenced Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1152, fixes #1031.
Parser defects where bad (or merely unlucky) input aborted the whole build instead of degrading gracefully, plus docutils 0.23 support. Each fix has a regression test, and the branch has been through an extensive multi-angle review (adversarial input fuzzing, old-vs-new behavior comparison for previously-building documents, performance benchmarks, docutils 0.20→0.23 compatibility verification against real wheels) — the second commit contains the hardening that review produced.
Crash fixes
Deeply nested HTML →
RecursionError(html_to_nodes.py): rendering the parsed HTML AST recurses once per nesting level (including re-entrantly, via nested directive parses), and the existing try/except only covered tokenization — so a few hundred nested<div>s inside anhtml_admonitionkilled the build with an uncaughtRecursionError. The conversion is now guarded: it warns (myst.html) and falls back to passing through the raw HTML. Verified byte-identical output for all non-pathological HTML.Hostile YAML front matter (
config/main.py,base.py,directives.py):read_topmatterandrender_front_mattercaught onlyParserError/ScannerError. Three further classes escaped and aborted the build:ConstructorError/ComposerError(e.g.title: !Ref X), the bareValueErrorPyYAML leaks for out-of-range timestamps (date: 2021-99-99), andRecursionErrorfrom deeply nested YAML (PyYAML's composer recurses per nesting level). All sites now share aYAML_LOAD_ERRORStuple and emit the intendedmyst.topmatterwarning; the directive-options YAML path (only reachable withvalidate_options=False, e.g. myst-nb) gets the same treatment.YAML alias-expansion bomb (
base.py):yaml.safe_loadis immune to "billion laughs" (aliases stay shared), but serializing the loaded structure for the docinfo field list expanded it — ~20 lines of front matter hung the build indefinitely injson.dumps, and a self-referential alias raisedValueError. Field values are now measured first (identity-memoized, so linear time even for exponential graphs) and skipped with a warning when oversized or unserializable.Footnote label colliding with a heading name (
base.py): the duplicate-definition check useddocument.nameids, which holds every target name, so# Note+[^note]: ...silently discarded the footnote definition with a misleading warning and a dangling reference. The check now gates ondocument.nametypes: collisions with implicit names (headings — the bug) keep the footnote (explicit-over-implicit is standard docutils behaviour, INFO-level only; verified no change to anchors/{ref}/slug resolution), while collisions with explicit targets ((name)=, figure:name:, eval-rst-created footnotes) keep the existing drop-with-warning behaviour byte-for-byte — letting those through would strip the name from both nodes and break previously working references. Also O(1) per definition, where a naive per-definition scan measured O(n²) (+9.7s on a 10k-footnote document).docutils 0.23 (#1152)
As reported by @befeleme, docutils 0.23 also attaches info-level (severity 1) system messages to the doctree, where previously they only went to the report stream — breaking
test_link_resolution[explicit>implicit], which runs withreport_level=0. This was the only 0.23 failure, and it is test-side, so:system_messagenodes from the tree on docutils ≥0.23 before comparing (they are still asserted via the captured report stream)docutils>=0.20,<0.24(supersedes ⬆️ Update docutils requirement from <0.23,>=0.20 to >=0.20,<0.24 #1144, which dependabot should auto-close)0.23is added to thecheck-myst-docutilsCI matrix, so the combination is exercised for real (the pytest matrix is unaffected: Sphinx itself still pins docutils <0.23, so 0.23 currently only reaches docutils-standalone users; worth a sphinx-side matrix entry once Sphinx lifts its cap)Also included
auto_modein sphinx-autodoc2, closing auto_mode is never explained / missing? #1031. Please keep theCo-authored-bytrailer when squashing so authorship is preserved.Full test suite passes locally (1116 passed, 11 skipped) on docutils 0.22.4, and the docutils-only test files pass on a real docutils 0.23.