Repository navigation
Stop list nesting short of the recursion limit - #1685
Open
Steve-20-ai wants to merge 3 commits into
Open
Steve-20-ai wants to merge 3 commits into
Steve-20-ai wants to merge 3 commits into
Conversation
A list nests one level per `parseBlocks` call, so a deeply nested list -- 500 markers on one line is about 1 KB of input -- reached the interpreter's limit and raised `RecursionError` out of `markdown.markdown()`. Anything rendering untrusted Markdown could be crashed by it. `BlockQuoteProcessor` has been limited this way since Python-Markdown#799: its `test()` stops nesting once `util.nearing_recursion_limit()` is true, and the remaining markers are kept as text. The list processors had no such guard, so `ListIndentProcessor.test()` and `OListProcessor.test()` now apply it, `UListProcessor` inheriting the latter. Content is not lost, the markers which no longer nest are rendered as text. The guard alone left the degraded path slow, because `get_items()` appended every continuation line with `'{}\n{}'.format(item, line)`, copying the whole item each time and making it quadratic in the number of lines. It now collects the lines of an item in a list and joins them once, which renders a 1200 level list in 3.8s instead of 20s. Output is unchanged on a corpus of 3000 random list documents. fixes Python-Markdown#1679
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.
Description
markdown.markdown()raises an uncaughtRecursionErroron a deeply nestedlist, so any service rendering untrusted Markdown can be crashed by a small
input. 500 list markers on one line is about 1 KB:
The same happens for an indented list of about 500 lines, and for a list inside
a blockquote. It is the class of problem
#1534covered, with a differenttrigger.
A list nests one level per
parseBlockscall, and the list processors had nolimit on that.
BlockQuoteProcessorhas had one since#799, which is why>>>>>>...renders instead of raising:This applies the same guard to the list processors.
ListIndentProcessor.test()and
OListProcessor.test()now returnFalseoncenearing_recursion_limit()is true, and
UListProcessorinherits the latter. The block then falls throughto the paragraph processor, so the markers which no longer nest are kept as
text, exactly as the blockquote guard keeps its
>markers. No content is lost:'- ' * 500 + 'x'(1 KB)RecursionError<ul>'1. ' * 500 + 'x'(1.5 KB)RecursionError'> ' + '- ' * 500 + 'x'(1 KB)RecursionError' ' * i + '- x'(354 KB)RecursionErrorThe guard alone is not enough to make that path reasonable, which is the part
of the report about rendering rather than crashing:
get_items()appended everycontinuation line with
'{}\n{}'.format(item, line), copying the whole item oneach line, which is quadratic in the number of lines of an item. A 1200 level
list, which is one huge item once nesting stops, took 20.0s with the guard and
3.8s once the lines of an item are collected in a list and joined once. That is
the second commit of this change, and it also speeds up any list item which
spans many lines.
Output is unchanged for input which nests below the limit: all 1192 tests pass,
and rendering a corpus of 3000 random list documents produces byte-identical
output before and after.
The remaining time for a very deep list is spent in the block processors
re-scanning the tail once per level, which this change does not address.
Two notes for review:
sys.getrecursionlimit(), as itdoes for blockquotes. The added test uses
recursionlimit()to exercise theguard at a low limit, in the style of
TestBlockquoteBlocks.test_nesting_limit().the report asks for both halves. Happy to split them if you would prefer to
review the guard on its own.
fixes #1679
AI Assistance Disclosure
AI tools were used and I have disclosed which ones, and have fully reviewed and verified their output.
AI tools were use to assist with the following:
The tool was the WorkBuddy coding agent. I reviewed the change and ran the
checks below myself, and I am the one responsible for it in review.
Checklist
Checks run locally:
python -m unittest discover tests(1192 tests, OK, 111skipped),
flake8 markdown tests(no new findings).