diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b4c2b3dd..d8ea2697 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -74,7 +74,7 @@ jobs: strategy: fail-fast: false matrix: - docutils-version: ["0.20", "0.21", "0.22"] + docutils-version: ["0.20", "0.21", "0.22", "0.23"] steps: - name: Checkout source diff --git a/docs/syntax/code_and_apis.md b/docs/syntax/code_and_apis.md index 771b843d..f24ccef2 100644 --- a/docs/syntax/code_and_apis.md +++ b/docs/syntax/code_and_apis.md @@ -170,7 +170,7 @@ Sphinx and MyST provide means to analyse source code and automatically generate As opposed to `sphinx.ext.autodoc`, `sphinx-autodoc2` performs static (rather than dynamic) analysis of the source code, integrates full package documenting, and also allows for docstrings to be written in both RestructureText and MyST. -The `auto_mode` will automatically generate the full API documentation, as shown . +[sphinx-autodoc2's `auto_mode` config option](https://sphinx-autodoc2.readthedocs.io/en/latest/config.html#confval-autodoc2_packages-auto_mode) (`True` by default) will automatically generate the full API documentation, as shown . Alternatively, the `autodoc2-object` directive can be used to generate documentation for a single object. To embed in a MyST document the MyST `render_plugin` should be specified, for example: diff --git a/myst_parser/config/main.py b/myst_parser/config/main.py index 2a380c49..9bf637dc 100644 --- a/myst_parser/config/main.py +++ b/myst_parser/config/main.py @@ -593,6 +593,8 @@ def read_topmatter(text: str | Iterator[str]) -> dict[str, Any] | None: """ import yaml + from myst_parser.parsers.directives import YAML_LOAD_ERRORS + if isinstance(text, str): if not text.startswith("---"): # skip creating the line list in memory return None @@ -609,7 +611,7 @@ def read_topmatter(text: str | Iterator[str]) -> dict[str, Any] | None: top_matter.append(line.rstrip() + "\n") try: metadata = yaml.safe_load("".join(top_matter)) - except (yaml.parser.ParserError, yaml.scanner.ScannerError) as err: + except YAML_LOAD_ERRORS as err: raise TopmatterReadError("Malformed YAML") from err if not isinstance(metadata, dict): raise TopmatterReadError(f"YAML is not a dict: {type(metadata)}") diff --git a/myst_parser/mdit_to_docutils/base.py b/myst_parser/mdit_to_docutils/base.py index 8ecfb70a..fe292635 100644 --- a/myst_parser/mdit_to_docutils/base.py +++ b/myst_parser/mdit_to_docutils/base.py @@ -46,7 +46,11 @@ MockState, MockStateMachine, ) -from myst_parser.parsers.directives import MarkupError, parse_directive_text +from myst_parser.parsers.directives import ( + YAML_LOAD_ERRORS, + MarkupError, + parse_directive_text, +) from myst_parser.warnings_ import MystWarnings, create_warning from .html_to_nodes import html_to_nodes @@ -1262,7 +1266,7 @@ def render_front_matter(self, token: SyntaxTreeNode) -> None: if isinstance(token.content, str): try: data = yaml.safe_load(token.content) - except (yaml.parser.ParserError, yaml.scanner.ScannerError): + except YAML_LOAD_ERRORS: self.create_warning( "Malformed YAML", MystWarnings.MD_TOPMATTER, @@ -1343,7 +1347,26 @@ def dict_to_fm_field_list( for key, value in data.items(): if not isinstance(value, str | int | float | date | datetime): - value = json.dumps(value) + # YAML anchors/aliases can produce structures whose expanded + # size is exponential in the source size (a "billion laughs" + # bomb), or even self-referential; measure before serializing + if _expanded_length(value) > _FM_FIELD_MAX_LENGTH: + self.create_warning( + f"Front matter field {key!r} is too large to render", + MystWarnings.MD_TOPMATTER, + line=line, + ) + continue + try: + value = json.dumps(value) + except (ValueError, RecursionError): + # e.g. a self-referential structure via a YAML alias + self.create_warning( + f"Front matter field {key!r} could not be serialized", + MystWarnings.MD_TOPMATTER, + line=line, + ) + continue value = str(value) body = nodes.paragraph() body.source, body.line = self.document["source"], line @@ -1505,10 +1528,16 @@ def render_footnote_reference(self, token: SyntaxTreeNode) -> None: """Despite the name, this is actually a footnote definition, e.g. `[^a]: ...`""" target = token.meta["label"] - if target in self.document.nameids: - # note we chose to directly omit these footnotes in the parser, - # rather than let docutils/sphinx handle them, since otherwise you end up with a confusing warning: - # WARNING: Duplicate explicit target name: "x". [docutils] + # A label sharing a name with an existing *explicit* target (another + # footnote, a `(name)=` target, a directive `:name:`, ...) is omitted + # here, rather than left to docutils/sphinx: registering it would + # emit a confusing warning (`Duplicate explicit target name`), + # strip the name from both nodes, and so break previously working + # references to the colliding target. + # A collision with an *implicit* target name (e.g. a section heading) + # keeps the footnote: explicit-over-implicit is standard docutils + # behaviour, reported at INFO level only. + if self.document.nametypes.get(target): # we use [ref.footnote] as the type/subtype, rather than a myst specific warning, # to make it more aligned with sphinx warnings for unreferenced footnotes self.create_warning( @@ -1945,6 +1974,33 @@ def render_substitution(self, token: SyntaxTreeNode, inline: bool) -> None: self.document.sub_references.difference_update(references) +_FM_FIELD_MAX_LENGTH = 100_000 +"""Maximum number of items a front matter field may expand to when rendered.""" + + +def _expanded_length(value: Any, _memo: dict[int, int] | None = None) -> int: + """Return the total number of items ``value`` expands to when serialized. + + Sub-structures are memoized by identity, so structures with shared + references (from YAML anchors/aliases), whose expanded size can be + exponential in the source size, are measured in linear time, + and self-referential structures terminate. + """ + if isinstance(value, dict): + children: Iterable[Any] = value.values() + elif isinstance(value, list | tuple): + children = value + else: + return 1 + if _memo is None: + _memo = {} + key = id(value) + if key not in _memo: + _memo[key] = 1 # occupied marker, in case value contains itself + _memo[key] = 1 + sum(_expanded_length(child, _memo) for child in children) + return _memo[key] + + def html_meta_to_nodes( data: dict[str, Any], document: nodes.document, line: int, reporter: Reporter ) -> list[nodes.meta | nodes.system_message]: diff --git a/myst_parser/mdit_to_docutils/html_to_nodes.py b/myst_parser/mdit_to_docutils/html_to_nodes.py index 713cc040..d8d07b09 100644 --- a/myst_parser/mdit_to_docutils/html_to_nodes.py +++ b/myst_parser/mdit_to_docutils/html_to_nodes.py @@ -7,7 +7,7 @@ from docutils import nodes -from myst_parser.parsers.parse_html import Data, tokenize_html +from myst_parser.parsers.parse_html import Data, Element, tokenize_html from myst_parser.warnings_ import MystWarnings if TYPE_CHECKING: @@ -81,6 +81,30 @@ def html_to_nodes( ): return default_html(text, renderer.document["source"], line_number) + try: + return _html_ast_to_nodes(root, line_number, renderer) + except RecursionError: + msg_node = renderer.create_warning( + "HTML is too deeply nested to process", + MystWarnings.HTML_PARSE, + line=line_number, + ) + return ([msg_node] if msg_node else []) + default_html( + text, renderer.document["source"], line_number + ) + + +def _html_ast_to_nodes( + root: Element, line_number: int, renderer: DocutilsRenderer +) -> list[nodes.Element]: + """Convert the parsed HTML AST to docutils nodes, + by running the equivalent ``image``/``admonition`` directives. + + Recursion depth scales with the HTML nesting depth + (via ``Element.render``/``Element.strip`` and the nested parse of + directive content, which can re-enter ``html_to_nodes``), + so callers must guard against ``RecursionError``. + """ nodes_list = [] for child in root: if child.name == "img": diff --git a/myst_parser/parsers/directives.py b/myst_parser/parsers/directives.py index 4f4f7ae3..1908d139 100644 --- a/myst_parser/parsers/directives.py +++ b/myst_parser/parsers/directives.py @@ -52,6 +52,13 @@ from .options import TokenizeError, options_to_items +YAML_LOAD_ERRORS = (yaml.YAMLError, ValueError, RecursionError) +"""Errors that ``yaml.safe_load`` can raise on invalid input: +``ValueError`` escapes PyYAML for some invalid scalars +(e.g. out-of-range timestamps like ``2021-99-99``), +and ``RecursionError`` for deeply nested data. +""" + @dataclass class ParseWarnings: @@ -207,7 +214,7 @@ def _parse_directive_options( yaml_errors: list[ParseWarnings] = [] try: yaml_options = yaml.safe_load(options_block or "") or {} - except (yaml.parser.ParserError, yaml.scanner.ScannerError): + except YAML_LOAD_ERRORS: yaml_options = {} yaml_errors.append( ParseWarnings( diff --git a/myst_parser/warnings_.py b/myst_parser/warnings_.py index 54b65991..99b9072f 100644 --- a/myst_parser/warnings_.py +++ b/myst_parser/warnings_.py @@ -61,7 +61,7 @@ class MystWarnings(Enum): STRIKETHROUGH = "strikethrough" """Strikethrough warning, since only implemented in HTML.""" HTML_PARSE = "html" - """HTML could not be parsed.""" + """Issue parsing or converting HTML.""" INVALID_ATTRIBUTE = "attribute" """Invalid attribute value.""" SUBSTITUTION = "substitution" diff --git a/pyproject.toml b/pyproject.toml index 3ff3bd2f..8c0d5d79 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ keywords = [ ] requires-python = ">=3.11" dependencies = [ - "docutils>=0.20,<0.23", + "docutils>=0.20,<0.24", "jinja2", # required for substitutions, but let sphinx choose version "markdown-it-py~=4.2", "mdit-py-plugins~=0.6,>=0.6.1", diff --git a/tests/test_docutils.py b/tests/test_docutils.py index bed66709..8a716d51 100644 --- a/tests/test_docutils.py +++ b/tests/test_docutils.py @@ -1,6 +1,7 @@ import contextlib import importlib.util import io +import sys from dataclasses import dataclass, field, fields from textwrap import dedent from typing import Literal @@ -210,6 +211,163 @@ def test_linkify_no_warning_when_available(): assert "[myst.linkify]" not in stream.getvalue() +def test_html_deep_nesting_warns(): + """Deeply nested HTML degrades to raw output with a warning. + + Regression: rendering the HTML AST recurses once per nesting level, + and the resulting ``RecursionError`` escaped and aborted the build. + """ + # rendering uses >=1 stack frame per nesting level, so this depth + # always overflows, whatever the currently available stack + depth = sys.getrecursionlimit() + # tags on separate lines, to stay under docutils' line-length-limit + source = ( + '

Title

\n' + + "
\n" * depth + + "content\n" + + "
\n" * depth + + "
\n" + ) + stream = io.StringIO() + doctree = publish_doctree( + source=source, + parser=Parser(), + settings_overrides={ + "myst_enable_extensions": ["html_admonition"], + "warning_stream": stream, + }, + ) + assert "HTML is too deeply nested" in stream.getvalue() + assert "[myst.html]" in stream.getvalue() + # the original text is preserved as a raw HTML node + assert list(doctree.findall(nodes.raw)) + + +@pytest.mark.parametrize( + "yaml_line", + [ + "title: !UnknownTag value", # yaml.constructor.ConstructorError + "date: 2021-99-99", # bare ValueError from an out-of-range timestamp + ], +) +def test_topmatter_hostile_yaml_warns(yaml_line): + """Front matter YAML errors beyond parser/scanner ones warn, not crash. + + Regression: only ``ParserError``/``ScannerError`` were caught, so e.g. a + ``ConstructorError`` from an unknown tag aborted the whole build. + """ + stream = io.StringIO() + doctree = publish_doctree( + source=f"---\n{yaml_line}\n---\n\ncontent\n", + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + assert "[myst.topmatter]" in stream.getvalue() + assert "content" in doctree.pformat() + + +def test_footnote_label_matching_heading_name(): + """A footnote label sharing a heading's name is not a duplicate. + + Regression: the duplicate check used ``document.nameids``, which holds + every target name, so the footnote definition was silently dropped. + """ + stream = io.StringIO() + doctree = publish_doctree( + source="# Note\n\n[^note]\n\n[^note]: the definition\n", + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + footnotes = list(doctree.findall(nodes.footnote)) + assert footnotes, "expected the footnote definition to be kept" + assert "the definition" in footnotes[0].astext() + assert "Duplicate footnote" not in stream.getvalue() + + +def test_footnote_label_matching_explicit_target(): + """A footnote label sharing an *explicit* target's name is dropped. + + Registering it would strip the name from both nodes and so break + previously working references to the target, with confusing docutils + warnings; the pre-existing drop-with-warning behaviour is kept. + """ + stream = io.StringIO() + doctree = publish_doctree( + source="(note)=\n\ntext\n\n[^note]: the definition\n", + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + assert "Duplicate footnote definition" in stream.getvalue() + assert "Duplicate explicit target name" not in stream.getvalue() + assert not list(doctree.findall(nodes.footnote)) + # the original target keeps its name, so references to it still resolve + assert "note" in doctree.nameids + assert doctree.nameids["note"] + + +def test_topmatter_deeply_nested_yaml_warns(): + """Deeply nested front matter YAML warns instead of crashing. + + Regression: PyYAML's composer recurses per nesting level, and the + resulting ``RecursionError`` is neither a ``YAMLError`` nor a + ``ValueError``, so it escaped and aborted the build. + + The nesting must be a single-line flow sequence: spread over lines it + fails in the (iterative) scanner instead, never reaching the composer. + """ + # pin the limit so the depth both overflows it and, as a single line, + # stays under docutils' line-length-limit (10000), whatever the runner + depth = 1000 + limit = sys.getrecursionlimit() + source = "---\nkey: " + "[" * depth + "]" * depth + "\n---\n\ncontent\n" + stream = io.StringIO() + try: + sys.setrecursionlimit(depth) + doctree = publish_doctree( + source=source, + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + finally: + sys.setrecursionlimit(limit) + assert "[myst.topmatter]" in stream.getvalue() + assert "content" in doctree.pformat() + + +def test_topmatter_alias_expansion_bomb_warns(): + """A YAML alias-expansion ("billion laughs") bomb warns, not hangs. + + ``yaml.safe_load`` keeps aliased structures shared, so loading is cheap, + but serializing the field for display would expand ``9**20`` items. + """ + lines = ["a0: &a0 [x, x, x, x, x, x, x, x, x]"] + for i in range(1, 20): + refs = ", ".join([f"*a{i - 1}"] * 9) + lines.append(f"a{i}: &a{i} [{refs}]") + source = "---\n" + "\n".join(lines) + "\n---\n\ncontent\n" + stream = io.StringIO() + doctree = publish_doctree( + source=source, + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + assert "too large to render" in stream.getvalue() + assert "[myst.topmatter]" in stream.getvalue() + assert "content" in doctree.pformat() + + +def test_footnote_duplicate_definition_warns(): + """A genuinely duplicated footnote definition still warns and is dropped.""" + stream = io.StringIO() + doctree = publish_doctree( + source="[^a]\n\n[^a]: first\n\n[^a]: second\n", + parser=Parser(), + settings_overrides={"warning_stream": stream}, + ) + assert "Duplicate footnote definition" in stream.getvalue() + assert len(list(doctree.findall(nodes.footnote))) == 1 + + def test_definition_list_orphan_definition(): """A definition with no preceding term errors, but keeps its content. diff --git a/tests/test_renderers/test_fixtures_docutils.py b/tests/test_renderers/test_fixtures_docutils.py index 4988f078..4e52bb6b 100644 --- a/tests/test_renderers/test_fixtures_docutils.py +++ b/tests/test_renderers/test_fixtures_docutils.py @@ -12,6 +12,7 @@ import pytest from docutils import __version_info__ as docutils_version +from docutils import nodes from docutils.core import Publisher, publish_doctree from pytest_param_files import ParamTestData @@ -64,6 +65,13 @@ def test_link_resolution(file_params: ParamTestData, normalize_doctree_xml): parser=Parser(), settings_overrides=settings, ) + # docutils >=0.23 also inserts info-level (severity 1) system messages + # into the doctree; they are still written to the report stream, + # which is what the fixtures capture, so drop them from the tree + if docutils_version >= (0, 23): + for msg in list(doctree.findall(nodes.system_message)): + if msg["level"] < 2: + msg.parent.remove(msg) outcome = normalize_doctree_xml(doctree.pformat()) if report_stream.getvalue().strip(): outcome += "\n\n" + report_stream.getvalue().strip()