fix: skip empty TightParagraph without panicking - #1128
Closed
teddytennant wants to merge 2 commits into
Closed
teddytennant wants to merge 2 commits into
teddytennant wants to merge 2 commits into
Conversation
When iterating events, empty TightParagraph nodes previously called tree.cur().unwrap() after push, which panics if the paragraph has no children. That showed up with ENABLE_TASKLISTS + ENABLE_STRIKETHROUGH on fuzz input (pulldown-cmark#1084) and related cases (pulldown-cmark#1095). Skip empty tight paragraphs by popping and advancing to the next sibling, then continue iteration, instead of unwrapping or returning None early (which would drop remaining events). Add regression tests for the pulldown-cmark#1084 and pulldown-cmark#1095 inputs. Fixes pulldown-cmark#1084
Nightly (and -D warnings) rejects `use core::... u8` because the std/core integer modules are deprecated. u8::MAX still resolves via the primitive type's associated const once the module is not imported.
notriddle
requested changes
Aug 9, 2026
| let parser = Parser::new_ext(s, Options::all()); | ||
| for _ in parser {} | ||
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
Hey, I copied these two tests over to the current main branch, and they don't fail.
Contributor
Author
|
You're right, sorry about that. I confirmed the panic against v0.13.3 and assumed it still applied to main, but the tests pass there. Looks like the NUL handling work fixed this one along the way, so there's nothing left to fix here. Closing this. I did find a separate panic that still reproduces on main, opened it as #1133 with the test in its own commit. |
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.
Problem
Parsing certain Markdown with
Options::ENABLE_TASKLISTSandOptions::ENABLE_STRIKETHROUGHcould panic while iterating the parser:Reproducer from #1084:
A related empty-
TightParagraphpanic was reported in #1095.Root cause
Tight list items use
ItemBody::TightParagraphnodes that intentionally emit neitherStartnorEnd. The iterator always didtree.push()thentree.cur().unwrap()to descend into the paragraph body. When the tight paragraph had no children,curwasNoneand the unwrap panicked.Fix
When encountering a
TightParagraph:push()into it.pop(), advance to the next sibling, and recurse intonext_event_rangeso remaining events are still emitted.This is intentionally different from returning
Noneon an empty tight paragraph, which would end iteration early and drop later siblings or ancestor ends.Note: a similar fix landed on
branch_0.13via #1096 / #1101, but not onmainafter theParserInnerrefactor. This ports the correct empty-paragraph handling to currentmain.Test plan
cargo test -p pulldown-cmark --lib(includes newissue_1084andissue_1095)cargo test -p pulldown-cmark(full package, 1241+ suite tests)v0.13.3and does not panic with this changeFixes #1084