Fix quadratic-time backtracking when a reference link has no URL - #1631
Open
afonsojanu wants to merge 1 commit into
Open
Fix quadratic-time backtracking when a reference link has no URL#1631afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
A malformed reference definition line, one with a label but no URL
(just trailing whitespace after the colon), makes ReferenceProcessor's
regex backtrack badly. The pattern had two adjacent [ ]* groups around
an optional newline, both matching the same run of spaces, so for a
string of n spaces there were n+1 ways to split them before the engine
gave up and tried the next split. That turns markdown.markdown('[id]:'
+ ' ' * 50000) into an eight second call instead of a near-instant one,
and it gets worse fast as the input grows.
Rewrote the pattern so the leading run of spaces is consumed greedily
by a single group, with the optional newline plus more spaces folded
into one non-ambiguous alternative after it. Verified this produces
identical matches (and identical groups) as the old pattern on the
handful of valid reference-link shapes the tests already cover, and
added a dedicated regression test that fails on unmodified master and
passes with the fix.
Fixes Python-Markdown#798.
facelessuser
reviewed
Sep 7, 2026
| See https://github.com/Python-Markdown/markdown/issues/798 | ||
| """ | ||
| text = '[id]:' + (' ' * 50000) | ||
| start = time.time() |
Collaborator
There was a problem hiding this comment.
Timed tests should likely be avoided as some systems may be running some slow hardware.
facelessuser
requested changes
Sep 7, 2026
facelessuser
left a comment
facelessuser
left a comment
Collaborator
There was a problem hiding this comment.
Outside the timing tests, I think this looks okay. Please remove the timed test.
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
2 participants
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.
Fixes #798 (specifically the sub-case flagged by @stsewd in this comment, a regex in
ReferenceProcessor).markdown.markdown('[id]:' + ' ' * 50000)currently takes about 8 seconds. The reference-link regex has two adjacent[ ]*groups around an optional newline ([ ]*\n?[ ]*) sitting right before a mandatory non-whitespace group. When there's no newline, those two groups both match the same run of spaces, so for n trailing spaces there are n+1 ways to split them between the two groups before the engine gives up looking for the URL. That's quadratic in the length of the run.I rewrote it so the first run of spaces is consumed by a single greedy group, with the newline-plus-more-spaces case folded into one unambiguous alternative right after (
[ ]*(?:\n[ ]*)?). Checked this against the reference-link shapes the existing test suite covers (with/without title, title on its own line, id/url split across a line break, leading indent) and got byte-for-byte identical match groups on all of them.Added a new test file (
test_reference_links.py) with a few basic reference-link assertions plus the specific regression case, timed with a 2-second ceiling. Confirmed it fails on unmodifiedmaster(13.4s) and passes with the fix (well under a second for the whole file). Full existing suite (1096 tests) still passes, andflake8is clean on the touched files.No CLA or DCO step in this repo as far as I could find.