Apply code review fixes: tab stops, depth guard, demote guard, docs

Author: thibaudcolas

draftjs_exporter/contentstate_filter/__init__.py

@@ -160,6 +160,12 @@ def _run_actions(value: Any, actions: list[FilterAction], kind: str) -> Any:
                 current = None
                 continue
             if action == "demote":
+                if not isinstance(current, dict) or current.get("type") not in (
+                    HEADER_DEMOTION
+                ):
+                    raise ConfigException(
+                        "Filter callback must return a demotable header block"
+                    )
                 current = {**current, "type": HEADER_DEMOTION[current["type"]]}
                 continue
             current = action(current)

draftjs_exporter/markdown_importer/__init__.py

@@ -44,7 +44,7 @@ def __init__(self, config: ImporterConfig | None = None) -> None:
             config = {}
         parser_class = import_string(config.get("parser", DEFAULT_PARSER))
         self.parser: MarkdownParser = parser_class(config.get("parser_config"))
-        self.filter = ContentStateFilter(config.get("filter_rules"))
+        self.filter: ContentStateFilter = ContentStateFilter(config.get("filter_rules"))
 
     def import_markdown(self, markdown: str) -> ContentState:
         """Parse Markdown and apply filter rules.

draftjs_exporter/markdown_parser/__init__.py

@@ -21,19 +21,46 @@ class ParserConfig(TypedDict, total=False):
     """Options controlling which Markdown constructs are recognized."""
 
     headings: bool
+    """Parse ATX headings (default: True)."""
+
     blockquote: bool
+    """Parse ``>`` blockquotes (default: True)."""
+
     code_fenced: bool
+    """Parse fenced code blocks (default: True)."""
+
     thematic_break: bool
+    """Parse thematic breaks (default: True)."""
+
     unordered_list: bool
+    """Parse unordered lists (default: True)."""
+
     ordered_list: bool
+    """Parse ordered lists (default: True)."""
+
     emphasis: bool
+    """Parse bold and italic delimiters (default: True)."""
+
     code_inline: bool
+    """Parse backtick code spans (default: True)."""
+
     links: bool
+    """Parse ``[label](url)`` links (default: True)."""
+
     images: bool
+    """Parse ``![alt](url)`` images (default: True)."""
+
     line_breaks: bool
+    """Strip two-space hard break markers (default: True)."""
+
     link_resolvers: list[EntityResolver]
+    """Resolver chain for link URLs (default: empty, uses ``LINK`` with the URL)."""
+
     image_resolvers: list[EntityResolver]
+    """Resolver chain for image URLs (default: empty, uses ``IMAGE`` with ``src``/``alt``)."""
+
     inline_html_styles: dict[str, str]
+    """Whitelist of HTML tags mapped to inline styles, e.g. ``{"sup": "SUPERSCRIPT"}``."""
 
 
 class MarkdownParser:
@@ -47,6 +74,8 @@ class MarkdownParser:
 
     __slots__ = ("config",)
 
+    config: ParserConfig
+
     def __init__(self, config: ParserConfig | None = None) -> None:
         """Initialize the parser with the given configuration.
 

draftjs_exporter/markdown_parser/blocks.py

@@ -41,6 +41,28 @@
 """Heading block types indexed by ATX level minus one."""
 
 
+def _indent_width(text: str, tab_size: int = 4) -> int:
+    """Compute the column width of indentation, expanding tabs to tab stops.
+
+    Per CommonMark, a tab advances the column position to the next
+    multiple of 4 rather than counting as a fixed number of spaces.
+
+    Parameters:
+        text: The leading whitespace of a line.
+        tab_size: The tab stop interval.
+
+    Returns:
+        The column position after the indentation.
+    """
+    col = 0
+    for ch in text:
+        if ch == "\t":
+            col += tab_size - (col % tab_size)
+        else:
+            col += 1
+    return col
+
+
 class BlockParser:
     """Parse Markdown line by line into Draft.js blocks.
 
@@ -309,7 +331,7 @@ def _parse_list(self, lines: list[str], i: int) -> int:
             match = unordered or ordered
             if match is None:
                 break
-            indent = len(match.group(1).replace("\t", "    "))
+            indent = _indent_width(match.group(1))
             while stack and indent < stack[-1]:
                 stack.pop()
             if not stack or indent > stack[-1]:

draftjs_exporter/markdown_parser/inline.py

@@ -29,6 +29,9 @@
 TAG_RE = re.compile(r"<([a-zA-Z][a-zA-Z0-9]*)>")
 """Matches an HTML opening tag without attributes."""
 
+MAX_INLINE_DEPTH = 100
+"""Maximum nesting depth for inline constructs before rejecting input."""
+
 
 class InlineParser:
     """Parse inline Markdown constructs into text with style/entity ranges.
@@ -137,6 +140,7 @@ def _resolve_entity(
         try:
             resolution: EntityResolution = resolve(resolvers, url, label, default)
         except MarkdownParseError:
+            # Propagate without wrapping; unexpected exceptions are wrapped below.
             raise
         except Exception as err:
             raise MarkdownParseError(
@@ -156,8 +160,18 @@ def _resolve_entity(
             entity_type, data, resolution.get("mutability", "MUTABLE")
         )
 
-    def _parse(self, text: str) -> tuple[str, list[Span]]:
-        """Scan text, returning output characters and annotation spans."""
+    def _parse(self, text: str, depth: int = 0) -> tuple[str, list[Span]]:
+        """Scan text, returning output characters and annotation spans.
+
+        Parameters:
+            text: The text to scan.
+            depth: Current recursion depth for nested constructs.
+
+        Raises:
+            MarkdownParseError: If nesting exceeds ``MAX_INLINE_DEPTH``.
+        """
+        if depth > MAX_INLINE_DEPTH:
+            raise MarkdownParseError("Inline nesting too deep")
         out: list[str] = []
         spans: list[Span] = []
         i = 0
@@ -207,7 +221,7 @@ def _parse(self, text: str) -> tuple[str, list[Span]]:
                 result = self._link_target(text, i)
                 if result is not None:
                     label_src, url, end = result
-                    label_plain, label_spans = self._parse(label_src)
+                    label_plain, label_spans = self._parse(label_src, depth + 1)
                     start = len(out)
                     out.extend(label_plain)
                     spans.extend(
@@ -223,14 +237,14 @@ def _parse(self, text: str) -> tuple[str, list[Span]]:
 
             # Emphasis: * _ ** __ *** ___
             if self.emphasis and ch in "*_":
-                consumed = self._parse_emphasis(text, i, out, spans)
+                consumed = self._parse_emphasis(text, i, out, spans, depth)
                 if consumed is not None:
                     i = consumed
                     continue
 
             # Whitelisted inline HTML tags.
             if ch == "<" and self.inline_html_styles:
-                consumed = self._parse_inline_html(text, i, out, spans)
+                consumed = self._parse_inline_html(text, i, out, spans, depth)
                 if consumed is not None:
                     i = consumed
                     continue
@@ -275,7 +289,7 @@ def _link_target(text: str, i: int) -> tuple[str, str, int] | None:
         return text[i + 1 : close], text[close + 2 : paren], paren + 1
 
     def _parse_emphasis(
-        self, text: str, i: int, out: list[str], spans: list[Span]
+        self, text: str, i: int, out: list[str], spans: list[Span], depth: int
     ) -> int | None:
         """Parse an emphasis delimiter run at index i.
 
@@ -287,6 +301,7 @@ def _parse_emphasis(
             i: Index of the first delimiter character.
             out: Output characters accumulated so far.
             spans: Spans accumulated so far.
+            depth: Current recursion depth for nested constructs.
 
         Returns:
             The index after the closing delimiter, or None when the run
@@ -305,7 +320,7 @@ def _parse_emphasis(
         end = self._find_closing(text, i + run, marker)
         if end == -1:
             return None
-        inner_plain, inner_spans = self._parse(text[i + run : end])
+        inner_plain, inner_spans = self._parse(text[i + run : end], depth + 1)
         start = len(out)
         out.extend(inner_plain)
         spans.extend(
@@ -349,7 +364,7 @@ def _find_closing(text: str, start: int, marker: str) -> int:
             return end
 
     def _parse_inline_html(
-        self, text: str, i: int, out: list[str], spans: list[Span]
+        self, text: str, i: int, out: list[str], spans: list[Span], depth: int
     ) -> int | None:
         """Parse a whitelisted inline HTML tag at index i.
 
@@ -362,6 +377,7 @@ def _parse_inline_html(
             i: Index of the ``<`` character.
             out: Output characters accumulated so far.
             spans: Spans accumulated so far.
+            depth: Current recursion depth for nested constructs.
 
         Returns:
             The index after the closing tag, or None when the tag does
@@ -378,7 +394,7 @@ def _parse_inline_html(
         end = text.find(closing, match.end())
         if end == -1:
             return None
-        inner_plain, inner_spans = self._parse(text[match.end() : end])
+        inner_plain, inner_spans = self._parse(text[match.end() : end], depth + 1)
         start = len(out)
         out.extend(inner_plain)
         spans.extend(

tests/contentstate_filter/test_filter.py

@@ -228,3 +228,33 @@ def test_entity_callback_without_type_rejected(self):
             ContentStateFilter(
                 [{"type": "entity", "match": "LINK", "action": lambda e: {}}]
             ).apply(cs)
+
+
+class TestDemoteChainGuards(unittest.TestCase):
+    def test_demote_after_callback_returning_bad_block_rejected(self):
+        from draftjs_exporter.error import ConfigException
+
+        cs = cs_with_blocks(make_block("header-one"))
+        with self.assertRaises(ConfigException):
+            ContentStateFilter(
+                [
+                    {"type": "block", "match": "header-one", "action": lambda b: {}},
+                    {"type": "block", "match": "header-one", "action": "demote"},
+                ]
+            ).apply(cs)
+
+    def test_demote_after_callback_changing_type_rejected(self):
+        from draftjs_exporter.error import ConfigException
+
+        cs = cs_with_blocks(make_block("header-one"))
+        with self.assertRaises(ConfigException):
+            ContentStateFilter(
+                [
+                    {
+                        "type": "block",
+                        "match": "header-one",
+                        "action": lambda b: {**b, "type": "unstyled"},
+                    },
+                    {"type": "block", "match": "header-one", "action": "demote"},
+                ]
+            ).apply(cs)

tests/markdown_parser/test_blocks.py

@@ -279,3 +279,15 @@ def bad(url, label):
             parser.parse("[a](/b)")
         self.assertEqual(ctx.exception.line, 99)
         self.assertEqual(ctx.exception.message, "custom failure")
+
+
+class TestTabIndentation(unittest.TestCase):
+    def test_tab_advances_to_next_tab_stop(self):
+        # "  \t" is 4 columns per CommonMark (tab from column 2 advances
+        # to column 4), the same indent as four spaces.
+        cs = parse("- a\n    - b\n  \t- c")
+        self.assertEqual([b["depth"] for b in cs["blocks"]], [0, 1, 1])
+
+    def test_lone_tab_is_one_tab_stop(self):
+        cs = parse("- a\n\t- b")
+        self.assertEqual([b["depth"] for b in cs["blocks"]], [0, 1])

tests/markdown_parser/test_inline_html.py

@@ -44,3 +44,23 @@ def test_empty_whitelist_means_literal(self):
         text, styles, _ = make_parser().parse("<sup>2</sup>")
         self.assertEqual(text, "<sup>2</sup>")
         self.assertEqual(styles, [])
+
+
+class TestNestingDepth(unittest.TestCase):
+    def test_depth_guard_rejects_excessive_recursion(self):
+        from draftjs_exporter.error import MarkdownParseError
+        from draftjs_exporter.markdown_parser.inline import MAX_INLINE_DEPTH
+
+        # The parser's find-first-closer semantics make deep nesting
+        # unreachable from real input (mis-paired constructs fall back to
+        # literal text), so the guard is exercised directly.
+        parser = make_parser(inline_html_styles=SUP_SUB)
+        with self.assertRaises(MarkdownParseError):
+            parser._parse("x", depth=MAX_INLINE_DEPTH + 1)
+
+    def test_moderate_nesting_still_parses(self):
+        text, styles, _ = make_parser(inline_html_styles=SUP_SUB).parse(
+            "<sup><sub>x</sub></sup>"
+        )
+        self.assertEqual(text, "x")
+        self.assertEqual(len(styles), 2)