Implement JS-esque StateBase.srcCharCodeAt - #190
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #190 +/- ##
==========================================
- Coverage 96.17% 96.11% -0.07%
==========================================
Files 61 61
Lines 3267 3267
==========================================
- Hits 3142 3140 -2
- Misses 125 127 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Seeing that this PR only removes 4 instances of The downsides for rejecting are diverging slightly from upstream logic, and the chance that there's still some place in the code where |
## Summary Fixes #415. Input that ends on a blockquote marker while a table is open inside that quote raises `IndexError: string index out of range`: ```python from markdown_it import MarkdownIt # html_block.py — raises IndexError MarkdownIt().enable("table").parse("> | a | b |\n> |---|---|\n>") # heading.py — same input, raises once html_block is out of the way MarkdownIt("commonmark", {"html": False}).enable("table").parse("> | a | b |\n> |---|---|\n>") ``` ## Root cause With `table` enabled, `html_block` and `heading` run as **terminator rules** on the empty final line that a trailing `>` produces. There, `pos = state.bMarks[startLine] + state.tShift[startLine]` equals `len(state.src)`, and both rules index `state.src[pos]` without guarding that boundary: - `html_block.py`: `if state.src[pos] != "<":` - `heading.py`: `ch = state.src[pos]` — its `pos >= maximum` check is on the *next* line, after the index In markdown-it (JS) the equivalent `state.src.charCodeAt(pos)` returns `NaN` out of range instead of raising — the port hazard tracked in #190. ## Fix Sibling terminator rules already defend against exactly this — `hr.py` and `blockquote.py` wrap the same index access in `try: ... except IndexError: return False` (added for #185 / #204). `html_block.py` and `heading.py` were missed. This applies the same established guard to both. `html_block` runs before `heading`, so it shadows it (disabling `html` moves the traceback to `heading` rather than fixing it) — hence both rules need the guard, and there is a regression test for each path. ## Tests Added two regression tests in `tests/test_fuzzer.py` (the existing home for crash-regression cases), one per crash path — asserting the input renders without raising. ## Note on local verification I verified the guard logic in isolation (the old expression raises `IndexError` when `pos == len(src)`; the guarded version returns `False`, i.e. the rule declines, with identical behavior for in-range characters — matching how `hr.py`/`blockquote.py` already behave). I was not able to run the full pytest suite locally (my environment's Python predates this package's minimum), so I'd appreciate CI confirming the two new tests pass and that the rendered output for that input is sensible. Happy to adjust the tests to assert exact rendered HTML if you'd prefer that over "does not raise". Co-authored-by: saket3395 <sakettulsan95@gmail.com> Co-authored-by: Chris Sewell <chrisj_sewell@hotmail.com>
|
Closing as unrebaseable: Generated by Claude Code |
2 participants
Replaces use of
StateBase.srcCharCodewith the more JS-esqueStateBase.srcCharCodeAtthat can returnNone.I didn't remove
StateBase.srcCharCodefor ease of migration because mdit-py-plugins use it. But did deprecate it.Happy to hear what you think @chrisjsewell . Not sure if this is the solution or migration strategy you want, but it's something 😄
I haven't measured performance yet. We may want to do that if this is something we want to proceed with otherwise
EDIT: This PR is in response to discussion in #186