From 3157efffe11916ec79fd568dbea15e77cf76e4a8 Mon Sep 17 00:00:00 2001 From: Paul Wicking Date: Fri, 5 May 2023 09:51:21 +0200 Subject: QDoc: Protect CMD_BRIEF from calls to getRestOfLine() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QDoc's \brief command takes an entire paragraph as its argument, and allows for the processing QDoc commands or macros that appear in the argument. Due to QDoc's internals, such commands may consume newline characters that the \brief command relies upon as its end condition. Thus the \brief command may erroneously include the following content as part of its argument. A previous patch addressed this issue specifically for `CMD_KEYWORD`. However, there are still multiple possible pathways in QDoc's code where the bug may be triggered. This patch seeks to add guards against this (mis)behavior in the following ways: Modify the `scopedenum` output test by adding a `\brief` to it, as that already makes use of `CMD_SINCELIST`. This modification exposes that the bug is also triggered in this case. To guard against the behavior, add a call to `leavePara()` in the parsing of `CMD_SINCELIST`. This resolves the issue, with no visual impact when generating the documentation for Qt. In parsing `CMD_IMAGE`, there's a call to `getRestOfLine()` when parsing the alternate text for an image. This patch adds test data for a page that features an image with an alternate text right at the top. This doesn't trigger the bug. The test data is kept as part of this patch as proof that the bug does not occur in this case, and to guard against possible future regressions. In parsing `CMD_OMITVALUE`, there's a call to getRestOfLine() while skipping potential description paragraphs. Modify the `scopedenum` test by adding a `\brief` to the `TestQDoc::Test::ScopedEnum` enum documentation, and rearrange the valuelist such that an `\omitvalue` command immediately follows the `\brief`. This triggers the bug. Add a call to `leavePara()` upon entering the case. This resolves the issue, with no visual impact when generating the documentation for Qt. In parsing of QDoc's line comments, `//!`, there's a call to `getRestOfLine()` while also adjusting the position. While adding a call to `leavePara()` in the parsing of line comments modifies the output of QDoc in what is arguably a correct way (that is, paragraphs are inserted where new paragraphs should be reasonably expected), that modification causes an unwanted side-effect in `CMD_BRIEF`: the consumption of the newline character by `getRestIfLine()` breaks `CMD_BRIEF` if one inserts a line comment within the arguments to the command and continue the argument on the following line. For example: \brief SANTA //! wait for it... MOZZARELLA! will, surprisingly, make the brief contain only SANTA, not SANTA MOZZARELLA! as one would expect. As the proposed workaround of inserting calls to `leavePara()` is found to have unwanted side-effects on `CMD_BRIEF` in the case of QDoc line comments, resolving this is left for a future change. Task-number: QTBUG-113118 Change-Id: Ic71e7e15c62a9beb834690f24f7538500346cb60 Reviewed-by: Topi Reiniö --- src/qdoc/qdoc/docparser.cpp | 2 ++ .../html/illformatteddocumentation.index | 1 + .../page-with-an-image-at-the-top.html | 19 +++++++++++++++++++ .../scopedenum-docbook/testqdoc-test.xml | 1 + .../expected_output/scopedenum/testqdoc-test.html | 1 + .../expected_output/scopedenum/whatsnew.html | 1 + .../illformatted_documentation/brief_adventures.qdoc | 12 ++++++++++++ .../testdata/scopedenum/scopedenum.qdoc | 6 +++--- .../auto/qdoc/generatedoutput/tst_generatedoutput.cpp | 1 + 9 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 tests/auto/qdoc/generatedoutput/expected_output/page-with-an-image-at-the-top.html diff --git a/src/qdoc/qdoc/docparser.cpp b/src/qdoc/qdoc/docparser.cpp index 0d4383f67..f0607945a 100644 --- a/src/qdoc/qdoc/docparser.cpp +++ b/src/qdoc/qdoc/docparser.cpp @@ -544,6 +544,7 @@ void DocParser::parse(const QString &source, DocPrivate *docPrivate, append(Atom::AnnotatedList, getArgument()); break; case CMD_SINCELIST: + leavePara(); append(Atom::SinceList, getRestOfLine().simplified()); break; case CMD_GENERATELIST: { @@ -750,6 +751,7 @@ void DocParser::parse(const QString &source, DocPrivate *docPrivate, getUntilEnd(cmd); break; case CMD_OMITVALUE: { + leavePara(); p1 = getArgument(); if (!m_private->m_enumItemList.contains(p1)) m_private->m_enumItemList.append(p1); diff --git a/tests/auto/qdoc/generatedoutput/expected_output/html/illformatteddocumentation.index b/tests/auto/qdoc/generatedoutput/expected_output/html/illformatteddocumentation.index index 8acd9baa2..96db721b1 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/html/illformatteddocumentation.index +++ b/tests/auto/qdoc/generatedoutput/expected_output/html/illformatteddocumentation.index @@ -11,6 +11,7 @@ + diff --git a/tests/auto/qdoc/generatedoutput/expected_output/page-with-an-image-at-the-top.html b/tests/auto/qdoc/generatedoutput/expected_output/page-with-an-image-at-the-top.html new file mode 100644 index 000000000..5675558f4 --- /dev/null +++ b/tests/auto/qdoc/generatedoutput/expected_output/page-with-an-image-at-the-top.html @@ -0,0 +1,19 @@ + + + + + + + This page starts with an image | IllformattedDocumentation + + +
+

This page starts with an image

+ +
+

This is the alternate text for the image

The image should render as expected, and the alternate text should be there, too. This text is contained in its own paragraph following the image.

+

This paragraph is a new paragraph, and doesn't contain an image.

+
+ + + diff --git a/tests/auto/qdoc/generatedoutput/expected_output/scopedenum-docbook/testqdoc-test.xml b/tests/auto/qdoc/generatedoutput/expected_output/scopedenum-docbook/testqdoc-test.xml index 4710a4cf5..81c003fe9 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/scopedenum-docbook/testqdoc-test.xml +++ b/tests/auto/qdoc/generatedoutput/expected_output/scopedenum-docbook/testqdoc-test.xml @@ -91,6 +91,7 @@ enum Test::ScopedEnum +This enum has a brief to trigger a bug in CMD_BRIEF. diff --git a/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/testqdoc-test.html b/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/testqdoc-test.html index c6f837f78..05a3dea20 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/testqdoc-test.html +++ b/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/testqdoc-test.html @@ -90,6 +90,7 @@ target_link_libraries(mytarget PRIVATE Qt6::QDocTest)

enum class Test::ScopedEnum

+

This enum has a brief to trigger a bug in CMD_BRIEF.

diff --git a/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/whatsnew.html b/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/whatsnew.html index f284b86f6..56e2a9783 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/whatsnew.html +++ b/tests/auto/qdoc/generatedoutput/expected_output/scopedenum/whatsnew.html @@ -3,6 +3,7 @@ + New Classes and Functions | TestCPP diff --git a/tests/auto/qdoc/generatedoutput/testdata/illformatted_documentation/brief_adventures.qdoc b/tests/auto/qdoc/generatedoutput/testdata/illformatted_documentation/brief_adventures.qdoc index b9e621f2b..908271519 100644 --- a/tests/auto/qdoc/generatedoutput/testdata/illformatted_documentation/brief_adventures.qdoc +++ b/tests/auto/qdoc/generatedoutput/testdata/illformatted_documentation/brief_adventures.qdoc @@ -22,3 +22,15 @@ It refers to a change that bypassed the issue by moving the \\keyword command, at \l https://codereview.qt-project.org/c/qt/qtdoc/+/242033/. */ + +/*! + \page page-with-an-image-at-the-top.html + \title This page starts with an image + \brief This page has an image right at the top. + \image leonardo-da-vinci.png This is the alternate text for the image + The image should render as expected, and the alternate text should be + there, too. This text is contained in its own paragraph following the + image. + + This paragraph is a new paragraph, and doesn't contain an image. +*/ diff --git a/tests/auto/qdoc/generatedoutput/testdata/scopedenum/scopedenum.qdoc b/tests/auto/qdoc/generatedoutput/testdata/scopedenum/scopedenum.qdoc index 775e5f81e..22ef9e3f3 100644 --- a/tests/auto/qdoc/generatedoutput/testdata/scopedenum/scopedenum.qdoc +++ b/tests/auto/qdoc/generatedoutput/testdata/scopedenum/scopedenum.qdoc @@ -12,7 +12,8 @@ /*! \enum TestQDoc::Test::ScopedEnum - + \brief This enum has a brief to trigger a bug in CMD_BRIEF. + \omitvalue UselessValue \value This Something \value That Something else \omitvalue OmittedValue \omit Unused - @@ -20,7 +21,6 @@ \value [since 2.0] All Everything \omitvalue VeryLastValue Nothing here - \omitvalue UselessValue A scoped enum. */ @@ -38,6 +38,6 @@ /*! \page whatsnew.html \title New Classes and Functions - + \brief New classes documentation \sincelist 2.0 */ diff --git a/tests/auto/qdoc/generatedoutput/tst_generatedoutput.cpp b/tests/auto/qdoc/generatedoutput/tst_generatedoutput.cpp index ea0c47e1a..f110b9e45 100644 --- a/tests/auto/qdoc/generatedoutput/tst_generatedoutput.cpp +++ b/tests/auto/qdoc/generatedoutput/tst_generatedoutput.cpp @@ -303,6 +303,7 @@ void tst_generatedOutput::illformated_documentation() "html/illformatted-examples.webxml " "html/illformatteddocumentation-someexample-example.webxml " "html/illformatteddocumentation.index " + "page-with-an-image-at-the-top.html " "brief-adventures.html"); } -- cgit v1.2.1
ConstantValueDescription
TestQDoc::Test::ScopedEnum::This0x01Something
TestQDoc::Test::ScopedEnum::That0x02Something else