mity / md4c

C Markdown parser. Fast. SAX-like interface. Compliant to CommonMark specification.
MIT License
759 stars 140 forks source link

Incorrectly detects an ordered list when there isn't one #95

Closed ec1oud closed 4 years ago

ec1oud commented 4 years ago

If the input is simply ".\n" (an actual newline there), the leave-block callback is invoked with MD_BLOCK_OL. Found by fuzzing: https://bugreports.qt.io/browse/QTBUG-78870

mity commented 4 years ago

Fixed. Thanks for reporting it.

mity commented 4 years ago

@ec1oud To prevent a misunderstanding, I fixed generating bad output as in:

$ printf '.\n' | ./md2html/md2html
<ol start="0">
<li></li>
</ol>

After the fix applied:

$ printf '.\n' | ./md2html/md2html
<p>.</p>

I am confused by the info in the https://bugreports.qt.io/browse/QTBUG-78870 that only the leave callback is called. Imho that cannot be true because md2html sees the enter event too as it outputs the opening tag. Can you please double check that?

ec1oud commented 4 years ago

Yes you're right, the enter callback is called with MD_BLOCK_OL. I just didn't push anything on my stack because there were no list items, so it's probably really a good idea to check whether it's empty before popping.

mity commented 4 years ago

But that is imho not a correct fix. Consider the empty list is nested in a parent list. The stack isn't then empty, yet you will happily pop it and end the parent list too early.

ec1oud commented 4 years ago

Should empty lists still happen in some cases?

mity commented 4 years ago

Yes. The list item mark not followed by text but just a new-line forms an empty list item with no text.

$ printf '*\n' | /src/cmark/build/src/cmark
<ul>
<li></li>
</ul>

And these can be nested in each other. E.g.

$ printf '*\n  *\n  *\n' | /src/cmark/build/src/cmark
<ul>
<li>
<ul>
<li></li>
<li></li>
</ul>
</li>
</ul>
ec1oud commented 4 years ago

OK. Will have to take care of that then. Thanks for your help; I'll get back to it in a day or two.

mity commented 4 years ago

Why you cannot push to the list stack directly from the cbEnterBlock() instead postponing it via the flag machinery? It would imho fix your issue and also simplify the code.