Avoid crashing when the end of an empty markdown list is detected

The markdown parser generates empty lists in some cases when a character
that can be used as a bullet is found on a line by itself.
cbEnterBlock() and cbLeaveBlock() are called symmetrically in such cases.
QStack::pop() on an empty stack triggers an assert, so push and pop need
to be done symmetrically too.  But it's difficult to actually create the
list as soon as the MD_BLOCK_UL or MD_BLOCK_OL callback occurs, without
breaking the case fixed in 7224d0e427 (and
probably other cases).  That's because QTextCursor::insertList() creates
a list item at the same time as it creates the list itself, and also
inherits block formatting from the previous block.  We now insert empty
lists with empty items whenever the need for that is detected though,
and there's a failsafe to prevent popping in case something still goes
wrong with that logic.  We aren't strict about reproducing the original
markdown when regenerating it via toMarkdown(), but it's getting closer.

Fixes: QTBUG-78870
Change-Id: Ided194ce7aec2710c60dbac42761ee4169ed9b78
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Robert Loehning <robert.loehning@qt.io>
bb10
Shawn Rutledge 2019-10-04 22:06:41 +02:00
parent 77455a9a8c
commit 524ab7b535
2 changed files with 81 additions and 6 deletions

View File

@ -216,6 +216,10 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det)
qCDebug(lcMD) << "LI";
} break;
case MD_BLOCK_UL: {
if (m_needsInsertList) // list nested in an empty list
m_listStack.push(m_cursor->insertList(m_listFormat));
else
m_needsInsertList = true;
MD_BLOCK_UL_DETAIL *detail = static_cast<MD_BLOCK_UL_DETAIL *>(det);
m_listFormat = QTextListFormat();
m_listFormat.setIndent(m_listStack.count() + 1);
@ -230,17 +234,19 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det)
m_listFormat.setStyle(QTextListFormat::ListDisc);
break;
}
qCDebug(lcMD, "UL %c level %d", detail->mark, m_listStack.count());
m_needsInsertList = true;
qCDebug(lcMD, "UL %c level %d", detail->mark, m_listStack.count() + 1);
} break;
case MD_BLOCK_OL: {
if (m_needsInsertList) // list nested in an empty list
m_listStack.push(m_cursor->insertList(m_listFormat));
else
m_needsInsertList = true;
MD_BLOCK_OL_DETAIL *detail = static_cast<MD_BLOCK_OL_DETAIL *>(det);
m_listFormat = QTextListFormat();
m_listFormat.setIndent(m_listStack.count() + 1);
m_listFormat.setNumberSuffix(QChar::fromLatin1(detail->mark_delimiter));
m_listFormat.setStyle(QTextListFormat::ListDecimal);
qCDebug(lcMD, "OL xx%d level %d", detail->mark_delimiter, m_listStack.count());
m_needsInsertList = true;
qCDebug(lcMD, "OL xx%d level %d", detail->mark_delimiter, m_listStack.count() + 1);
} break;
case MD_BLOCK_TD: {
MD_BLOCK_TD_DETAIL *detail = static_cast<MD_BLOCK_TD_DETAIL *>(det);
@ -306,8 +312,14 @@ int QTextMarkdownImporter::cbLeaveBlock(int blockType, void *detail)
break;
case MD_BLOCK_UL:
case MD_BLOCK_OL:
qCDebug(lcMD, "list at level %d ended", m_listStack.count());
m_listStack.pop();
if (Q_UNLIKELY(m_needsInsertList))
m_listStack.push(m_cursor->createList(m_listFormat));
if (Q_UNLIKELY(m_listStack.isEmpty())) {
qCWarning(lcMD, "list ended unexpectedly");
} else {
qCDebug(lcMD, "list at level %d ended", m_listStack.count());
m_listStack.pop();
}
break;
case MD_BLOCK_TR: {
// https://github.com/mity/md4c/issues/29

View File

@ -53,6 +53,8 @@ class tst_QTextMarkdownImporter : public QObject
private slots:
void headingBulletsContinuations();
void thematicBreaks();
void lists_data();
void lists();
};
void tst_QTextMarkdownImporter::headingBulletsContinuations()
@ -159,5 +161,66 @@ void tst_QTextMarkdownImporter::thematicBreaks()
#endif
}
void tst_QTextMarkdownImporter::lists_data()
{
QTest::addColumn<QString>("input");
QTest::addColumn<int>("expectedItemCount");
QTest::addColumn<bool>("expectedEmptyItems");
QTest::addColumn<QString>("rewrite");
// Some of these cases show odd behavior, which is subject to change
// as the importer and the writer are tweaked to fix bugs over time.
QTest::newRow("dot newline") << ".\n" << 0 << true << ".\n\n";
QTest::newRow("number dot newline") << "1.\n" << 1 << true << "";
QTest::newRow("star newline") << "*\n" << 1 << true << "";
QTest::newRow("hyphen newline") << "-\n" << 1 << true << "";
QTest::newRow("hyphen space newline") << "- \n" << 1 << true << "";
QTest::newRow("hyphen space letter newline") << "- a\n" << 1 << false << "- a\n";
QTest::newRow("hyphen nbsp newline") <<
QString::fromUtf8("-\u00A0\n") << 0 << true << "-\u00A0\n\n";
QTest::newRow("nested empty lists") << "*\n *\n *\n" << 1 << true << "";
QTest::newRow("list nested in empty list") << "-\n * a\n" << 2 << false << "- \n * a\n";
QTest::newRow("lists nested in empty lists")
<< "-\n * a\n * b\n- c\n *\n + d\n" << 5 << false
<< "- \n * a\n * b\n- c *\n + d\n";
QTest::newRow("numeric lists nested in empty lists")
<< "- \n 1. a\n 2. b\n- c\n 1.\n + d\n" << 4 << false
<< "- \n 1. a\n 2. b\n- c 1. + d\n";
}
void tst_QTextMarkdownImporter::lists()
{
QFETCH(QString, input);
QFETCH(int, expectedItemCount);
QFETCH(bool, expectedEmptyItems);
QFETCH(QString, rewrite);
QTextDocument doc;
doc.setMarkdown(input); // QTBUG-78870 : don't crash
QTextFrame::iterator iterator = doc.rootFrame()->begin();
QTextFrame *currentFrame = iterator.currentFrame();
int i = 0;
int itemCount = 0;
bool emptyItems = true;
while (!iterator.atEnd()) {
// There are no child frames
QCOMPARE(iterator.currentFrame(), currentFrame);
// Check whether the block is text or a horizontal rule
QTextBlock block = iterator.currentBlock();
if (block.textList()) {
++itemCount;
if (!block.text().isEmpty())
emptyItems = false;
}
qCDebug(lcTests, "%d %s%s", i,
(block.textList() ? "<li>" : "<p>"), qPrintable(block.text()));
++iterator;
++i;
}
QCOMPARE(itemCount, expectedItemCount);
QCOMPARE(emptyItems, expectedEmptyItems);
QCOMPARE(doc.toMarkdown(), rewrite);
}
QTEST_MAIN(tst_QTextMarkdownImporter)
#include "tst_qtextmarkdownimporter.moc"