Skip to content

Commit 4f9abe9

Browse files
committed
🐛 FIX: build-aborting crashes on hostile input, and docutils 0.23 compat
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
1 parent e610432 commit 4f9abe9

7 files changed

Lines changed: 125 additions & 6 deletions

File tree

.github/workflows/tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ jobs:
7474
strategy:
7575
fail-fast: false
7676
matrix:
77-
docutils-version: ["0.20", "0.21", "0.22"]
77+
docutils-version: ["0.20", "0.21", "0.22", "0.23"]
7878

7979
steps:
8080
- name: Checkout source

myst_parser/config/main.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,9 @@ def read_topmatter(text: str | Iterator[str]) -> dict[str, Any] | None:
609609
top_matter.append(line.rstrip() + "\n")
610610
try:
611611
metadata = yaml.safe_load("".join(top_matter))
612-
except (yaml.parser.ParserError, yaml.scanner.ScannerError) as err:
612+
# ValueError: PyYAML lets it escape for some invalid scalars,
613+
# e.g. out-of-range timestamps like `2021-99-99`
614+
except (yaml.YAMLError, ValueError) as err:
613615
raise TopmatterReadError("Malformed YAML") from err
614616
if not isinstance(metadata, dict):
615617
raise TopmatterReadError(f"YAML is not a dict: {type(metadata)}")

myst_parser/mdit_to_docutils/base.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,7 +1262,9 @@ def render_front_matter(self, token: SyntaxTreeNode) -> None:
12621262
if isinstance(token.content, str):
12631263
try:
12641264
data = yaml.safe_load(token.content)
1265-
except (yaml.parser.ParserError, yaml.scanner.ScannerError):
1265+
# ValueError: PyYAML lets it escape for some invalid scalars,
1266+
# e.g. out-of-range timestamps like `2021-99-99`
1267+
except (yaml.YAMLError, ValueError):
12661268
self.create_warning(
12671269
"Malformed YAML",
12681270
MystWarnings.MD_TOPMATTER,
@@ -1505,7 +1507,15 @@ def render_footnote_reference(self, token: SyntaxTreeNode) -> None:
15051507
"""Despite the name, this is actually a footnote definition, e.g. `[^a]: ...`"""
15061508
target = token.meta["label"]
15071509

1508-
if target in self.document.nameids:
1510+
# check against existing footnote labels only, not `document.nameids`,
1511+
# which holds every target name (headings, `(name)=` targets, ...) and
1512+
# so would wrongly drop a footnote that merely shares a heading's name
1513+
existing_labels = {
1514+
name
1515+
for footnote in self.document.autofootnotes + self.document.footnotes
1516+
for name in footnote["names"]
1517+
}
1518+
if target in existing_labels:
15091519
# note we chose to directly omit these footnotes in the parser,
15101520
# rather than let docutils/sphinx handle them, since otherwise you end up with a confusing warning:
15111521
# WARNING: Duplicate explicit target name: "x". [docutils]

myst_parser/mdit_to_docutils/html_to_nodes.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from docutils import nodes
99

10-
from myst_parser.parsers.parse_html import Data, tokenize_html
10+
from myst_parser.parsers.parse_html import Data, Element, tokenize_html
1111
from myst_parser.warnings_ import MystWarnings
1212

1313
if TYPE_CHECKING:
@@ -81,6 +81,22 @@ def html_to_nodes(
8181
):
8282
return default_html(text, renderer.document["source"], line_number)
8383

84+
try:
85+
return _render_nodes(root, line_number, renderer)
86+
except RecursionError:
87+
msg_node = renderer.create_warning(
88+
"HTML is too deeply nested to process",
89+
MystWarnings.HTML_PARSE,
90+
line=line_number,
91+
)
92+
return ([msg_node] if msg_node else []) + default_html(
93+
text, renderer.document["source"], line_number
94+
)
95+
96+
97+
def _render_nodes(
98+
root: Element, line_number: int, renderer: DocutilsRenderer
99+
) -> list[nodes.Element]:
84100
nodes_list = []
85101
for child in root:
86102
if child.name == "img":

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ keywords = [
3434
]
3535
requires-python = ">=3.11"
3636
dependencies = [
37-
"docutils>=0.20,<0.23",
37+
"docutils>=0.20,<0.24",
3838
"jinja2", # required for substitutions, but let sphinx choose version
3939
"markdown-it-py~=4.2",
4040
"mdit-py-plugins~=0.6,>=0.6.1",

tests/test_docutils.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import contextlib
22
import importlib.util
33
import io
4+
import sys
45
from dataclasses import dataclass, field, fields
56
from textwrap import dedent
67
from typing import Literal
@@ -210,6 +211,89 @@ def test_linkify_no_warning_when_available():
210211
assert "[myst.linkify]" not in stream.getvalue()
211212

212213

214+
def test_html_deep_nesting_warns():
215+
"""Deeply nested HTML degrades to raw output with a warning.
216+
217+
Regression: rendering the HTML AST recurses once per nesting level,
218+
and the resulting ``RecursionError`` escaped and aborted the build.
219+
"""
220+
depth = sys.getrecursionlimit()
221+
# tags on separate lines, to stay under docutils' line-length-limit
222+
source = (
223+
'<div class="admonition"><p class="title">Title</p>\n'
224+
+ "<div>\n" * depth
225+
+ "content\n"
226+
+ "</div>\n" * depth
227+
+ "</div>\n"
228+
)
229+
stream = io.StringIO()
230+
doctree = publish_doctree(
231+
source=source,
232+
parser=Parser(),
233+
settings_overrides={
234+
"myst_enable_extensions": ["html_admonition"],
235+
"warning_stream": stream,
236+
},
237+
)
238+
assert "HTML is too deeply nested" in stream.getvalue()
239+
assert "[myst.html]" in stream.getvalue()
240+
# the original text is preserved as a raw HTML node
241+
assert list(doctree.findall(nodes.raw))
242+
243+
244+
@pytest.mark.parametrize(
245+
"yaml_line",
246+
[
247+
"title: !UnknownTag value", # yaml.constructor.ConstructorError
248+
"date: 2021-99-99", # bare ValueError from an out-of-range timestamp
249+
],
250+
)
251+
def test_topmatter_hostile_yaml_warns(yaml_line):
252+
"""Front matter YAML errors beyond parser/scanner ones warn, not crash.
253+
254+
Regression: only ``ParserError``/``ScannerError`` were caught, so e.g. a
255+
``ConstructorError`` from an unknown tag aborted the whole build.
256+
"""
257+
stream = io.StringIO()
258+
doctree = publish_doctree(
259+
source=f"---\n{yaml_line}\n---\n\ncontent\n",
260+
parser=Parser(),
261+
settings_overrides={"warning_stream": stream},
262+
)
263+
assert "[myst.topmatter]" in stream.getvalue()
264+
assert "content" in doctree.pformat()
265+
266+
267+
def test_footnote_label_matching_heading_name():
268+
"""A footnote label sharing a heading's name is not a duplicate.
269+
270+
Regression: the duplicate check used ``document.nameids``, which holds
271+
every target name, so the footnote definition was silently dropped.
272+
"""
273+
stream = io.StringIO()
274+
doctree = publish_doctree(
275+
source="# Note\n\n[^note]\n\n[^note]: the definition\n",
276+
parser=Parser(),
277+
settings_overrides={"warning_stream": stream},
278+
)
279+
footnotes = list(doctree.findall(nodes.footnote))
280+
assert footnotes, "expected the footnote definition to be kept"
281+
assert "the definition" in footnotes[0].astext()
282+
assert "Duplicate footnote" not in stream.getvalue()
283+
284+
285+
def test_footnote_duplicate_definition_warns():
286+
"""A genuinely duplicated footnote definition still warns and is dropped."""
287+
stream = io.StringIO()
288+
doctree = publish_doctree(
289+
source="[^a]\n\n[^a]: first\n\n[^a]: second\n",
290+
parser=Parser(),
291+
settings_overrides={"warning_stream": stream},
292+
)
293+
assert "Duplicate footnote definition" in stream.getvalue()
294+
assert len(list(doctree.findall(nodes.footnote))) == 1
295+
296+
213297
def test_definition_list_orphan_definition():
214298
"""A definition with no preceding term errors, but keeps its content.
215299

tests/test_renderers/test_fixtures_docutils.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import pytest
1414
from docutils import __version_info__ as docutils_version
15+
from docutils import nodes
1516
from docutils.core import Publisher, publish_doctree
1617
from pytest_param_files import ParamTestData
1718

@@ -64,6 +65,12 @@ def test_link_resolution(file_params: ParamTestData, normalize_doctree_xml):
6465
parser=Parser(),
6566
settings_overrides=settings,
6667
)
68+
# docutils >=0.23 also inserts info-level (severity 1) system messages
69+
# into the doctree; they are still written to the report stream,
70+
# which is what the fixtures capture, so drop them from the tree
71+
for msg in list(doctree.findall(nodes.system_message)):
72+
if msg["level"] < 2:
73+
msg.parent.remove(msg)
6774
outcome = normalize_doctree_xml(doctree.pformat())
6875
if report_stream.getvalue().strip():
6976
outcome += "\n\n" + report_stream.getvalue().strip()

0 commit comments

Comments
 (0)