From 4f6d1ecb352ad8bba850ca5611ad4881d113bf52 Mon Sep 17 00:00:00 2001
From: "Danilo M." <danix@danix.xyz>
Date: Thu, 20 Aug 2026 18:15:29 +0200
Subject: feat(compose): render markdown bodies with cmark-gfm, item 123

The composer's body is markdown and the text/html part is generated from it.
cmark-gfm rather than plain cmark for autolink: under CommonMark a bare URL
in a mail body is not a link, and in mail it is expected to be clickable.

Three extensions are enabled and tables are deliberately not, since they
render badly across mail clients whoever generates them. Raw HTML in the
input is suppressed with CMARK_OPT_SAFE: the body is the user's own text,
but a body that can inject markup into its own generated HTML part is a
sharp edge with no upside.

The build needs TWO lookups. Only the core library ships a pkg-config file;
libcmark-gfm-extensions has none and is located with find_library, the way
notmuch already is. All three extensions live in that second library, so
finding only the first produces a build that compiles and silently renders
plain CommonMark.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
---
 tests/test_markdownrenderer.cpp | 121 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)
 create mode 100644 tests/test_markdownrenderer.cpp

(limited to 'tests/test_markdownrenderer.cpp')

diff --git a/tests/test_markdownrenderer.cpp b/tests/test_markdownrenderer.cpp
new file mode 100644
index 0000000..44f3ccb
--- /dev/null
+++ b/tests/test_markdownrenderer.cpp
@@ -0,0 +1,121 @@
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <QtTest>
+
+#include "markdownrenderer.h"
+
+/// The extension configuration cmark-gfm renders the composer's body with.
+///
+/// No QApplication is needed here: MarkdownRenderer is a pure function over
+/// strings, so QTEST_APPLESS_MAIN avoids pulling in a platform plugin for a
+/// test that has nothing to do with widgets.
+class TestMarkdownRenderer : public QObject
+{
+    Q_OBJECT
+private slots:
+    void commonMarkBasicsRender();
+    void autolinkTurnsABareUrlIntoALink();
+    void strikethroughRenders();
+    void tasklistRenders();
+    void tablesAreNotEnabled();
+    void rawHtmlIsSuppressed();
+    void accentedTextSurvivesAsUtf8();
+    void emptyInputProducesEmptyOutput();
+};
+
+void TestMarkdownRenderer::commonMarkBasicsRender()
+{
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("**bold** *italic* `code`"));
+    QVERIFY2(html.contains(QStringLiteral("<strong>")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("<em>")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("<code>")), qPrintable(html));
+}
+
+void TestMarkdownRenderer::autolinkTurnsABareUrlIntoALink()
+{
+    // The whole reason cmark-gfm was chosen over plain cmark. Under
+    // CommonMark a bare URL is text, and a bare URL in mail is expected to
+    // be clickable.
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("see https://example.org for details"));
+    QVERIFY2(html.contains(QStringLiteral("<a href=\"https://example.org\"")),
+             qPrintable(html));
+}
+
+void TestMarkdownRenderer::strikethroughRenders()
+{
+    const QString html = MarkdownRenderer::toHtml(QStringLiteral("~~gone~~"));
+    QVERIFY2(html.contains(QStringLiteral("<del>gone</del>")), qPrintable(html));
+}
+
+void TestMarkdownRenderer::tasklistRenders()
+{
+    // Known ceiling: many mail clients strip the checkbox, so those
+    // recipients see the item with no marker. The plain part still carries
+    // the literal "- [ ]", so nothing is lost, only the HTML rendering.
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("- [ ] todo\n- [x] done"));
+    QVERIFY2(html.contains(QStringLiteral("type=\"checkbox\"")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("checked")), qPrintable(html));
+}
+
+void TestMarkdownRenderer::tablesAreNotEnabled()
+{
+    // Deliberately off: tables render badly across mail clients regardless of
+    // who generates them. The extension EXISTS in the library, so this
+    // asserts a decision rather than a limitation, and would silently start
+    // passing the wrong way if someone attached it "for completeness".
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("| a | b |\n|---|---|\n| 1 | 2 |"));
+    QVERIFY2(!html.contains(QStringLiteral("<table")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("| a | b |")), qPrintable(html));
+}
+
+void TestMarkdownRenderer::rawHtmlIsSuppressed()
+{
+    // CMARK_OPT_SAFE. The body is the user's own text, but a body that can
+    // inject markup into its own generated HTML part is a sharp edge with no
+    // upside.
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("<script>alert(1)</script>\n\nafter"));
+    QVERIFY2(!html.contains(QStringLiteral("<script>")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("after")), qPrintable(html));
+}
+
+void TestMarkdownRenderer::accentedTextSurvivesAsUtf8()
+{
+    // This user writes Italian, so accented text is every message rather
+    // than an edge case, and a UTF-8 round trip through a C library is
+    // exactly where it would be lost.
+    const QString source = QString::fromUtf8("perch\xC3\xA9 \xC3\xA8 cos\xC3\xAC");
+    const QString html = MarkdownRenderer::toHtml(source);
+    QVERIFY2(html.contains(source), qPrintable(html));
+}
+
+void TestMarkdownRenderer::emptyInputProducesEmptyOutput()
+{
+    // reply_no_quote opens a composer with an empty body and it must not
+    // produce a stray paragraph or crash the renderer.
+    const QString html = MarkdownRenderer::toHtml(QString());
+    QVERIFY2(html.trimmed().isEmpty(), qPrintable(html));
+}
+
+QTEST_APPLESS_MAIN(TestMarkdownRenderer)
+#include "test_markdownrenderer.moc"
-- 
cgit v1.2.3


From 2baf2d4e2c1d8d059a0795bf46596c64c28e01e5 Mon Sep 17 00:00:00 2001
From: "Danilo M." <danix@danix.xyz>
Date: Thu, 20 Aug 2026 18:19:31 +0200
Subject: fix(compose): document what actually suppresses raw HTML, item 123

CMARK_OPT_SAFE has had no effect since cmark-gfm made safe mode the
default; the flag is retained for API compatibility and the real
protection is that CMARK_OPT_UNSAFE is never set. Measured against
0.29.0.gfm.13: rendering with OPT_DEFAULT alone, with OPT_SAFE, and with
OPT_UNSAFE shows the first two suppress a script element and a
javascript: link while the third leaks both.

The comment credited the flag, which would have sent the next reader to
the wrong place, and the test could not tell the two apart: it would
have passed just as well with the flag deleted. What it has to guard
against is OPT_UNSAFE being introduced, so it now also asserts that
unsafe links are stripped, which is a protection this gets for free and
previously asserted nothing about.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
---
 src/markdownrenderer.cpp        | 21 +++++++++++++++++----
 tests/test_markdownrenderer.cpp | 29 ++++++++++++++++++++++++++---
 2 files changed, 43 insertions(+), 7 deletions(-)

(limited to 'tests/test_markdownrenderer.cpp')

diff --git a/src/markdownrenderer.cpp b/src/markdownrenderer.cpp
index a9b8177..ccb3309 100644
--- a/src/markdownrenderer.cpp
+++ b/src/markdownrenderer.cpp
@@ -34,8 +34,8 @@ namespace {
 ///
 /// `table` is absent deliberately, not by oversight: tables render badly
 /// across mail clients regardless of who generates them. `tagfilter` is absent
-/// because CMARK_OPT_SAFE already suppresses raw HTML wholesale, which is the
-/// stronger measure.
+/// because safe mode (see below) already suppresses raw HTML wholesale, which
+/// is the stronger measure.
 const char *const kExtensions[] = { "autolink", "strikethrough", "tasklist" };
 
 }  // namespace
@@ -51,8 +51,21 @@ QString MarkdownRenderer::toHtml(const QString &markdown)
     // after the first call.
     cmark_gfm_core_extensions_ensure_registered();
 
-    // SAFE suppresses raw HTML in the INPUT. It does not escape the output,
-    // which is markup by definition.
+    // CMARK_OPT_DEFAULT is 0, and CMARK_OPT_SAFE is a NO-OP in cmark-gfm 0.29:
+    // safe mode has been the default since that release, and the flag is kept
+    // only for API compatibility with code written against older versions.
+    // The real requirement is that CMARK_OPT_UNSAFE must never be set. Under
+    // safe mode a raw <script> block is replaced with an HTML comment
+    // placeholder, and a link whose scheme is not in the allowed set
+    // (javascript:, vbscript:, file:, and data: except a few safe image
+    // types) is replaced with an empty href. Measured against
+    // cmark-gfm-0.29.0.gfm.13 on 2026-08-20: rendering the same script tag and
+    // a javascript: link under OPT_DEFAULT alone, under OPT_DEFAULT|OPT_SAFE,
+    // and under OPT_UNSAFE shows the first two behave identically and
+    // suppress both, while OPT_UNSAFE leaks both verbatim into the output.
+    // OPT_SAFE is kept anyway, both as a statement of intent and in case a
+    // future cmark-gfm release makes it meaningful again; do not read its
+    // presence as the mechanism actually doing the suppressing.
     const int options = CMARK_OPT_DEFAULT | CMARK_OPT_SAFE;
 
     cmark_parser *parser = cmark_parser_new(options);
diff --git a/tests/test_markdownrenderer.cpp b/tests/test_markdownrenderer.cpp
index 44f3ccb..0c35b6c 100644
--- a/tests/test_markdownrenderer.cpp
+++ b/tests/test_markdownrenderer.cpp
@@ -35,6 +35,7 @@ private slots:
     void tasklistRenders();
     void tablesAreNotEnabled();
     void rawHtmlIsSuppressed();
+    void unsafeLinksAreStripped();
     void accentedTextSurvivesAsUtf8();
     void emptyInputProducesEmptyOutput();
 };
@@ -90,15 +91,37 @@ void TestMarkdownRenderer::tablesAreNotEnabled()
 
 void TestMarkdownRenderer::rawHtmlIsSuppressed()
 {
-    // CMARK_OPT_SAFE. The body is the user's own text, but a body that can
-    // inject markup into its own generated HTML part is a sharp edge with no
-    // upside.
+    // Safe mode (the cmark-gfm 0.29 default, not CMARK_OPT_SAFE, which is a
+    // no-op in this version, see markdownrenderer.cpp). The body is the
+    // user's own text, but a body that can inject markup into its own
+    // generated HTML part is a sharp edge with no upside.
+    //
+    // Asserted on the actual placeholder rather than only "no <script>",
+    // because the weaker assertion would still pass with CMARK_OPT_UNSAFE
+    // set by mistake, as long as something ELSE in the string also matched
+    // "not <script>" and "contains after" (measured: it does not distinguish
+    // safe from unsafe mode on its own). "raw HTML omitted" is what safe mode
+    // actually emits in place of the tag.
     const QString html = MarkdownRenderer::toHtml(
         QStringLiteral("<script>alert(1)</script>\n\nafter"));
     QVERIFY2(!html.contains(QStringLiteral("<script>")), qPrintable(html));
+    QVERIFY2(html.contains(QStringLiteral("raw HTML omitted")), qPrintable(html));
     QVERIFY2(html.contains(QStringLiteral("after")), qPrintable(html));
 }
 
+void TestMarkdownRenderer::unsafeLinksAreStripped()
+{
+    // A protection this gets for free from safe mode, and previously
+    // asserted nothing about: a javascript: link is replaced with an empty
+    // href rather than passed through. The body is the user's own text, but
+    // it is rendered into an HTML part sent to other people, so a
+    // javascript: link surviving into that part would be a real defect, not
+    // a cosmetic one.
+    const QString html = MarkdownRenderer::toHtml(
+        QStringLiteral("[click](javascript:alert(1))"));
+    QVERIFY2(!html.contains(QStringLiteral("javascript:")), qPrintable(html));
+}
+
 void TestMarkdownRenderer::accentedTextSurvivesAsUtf8()
 {
     // This user writes Italian, so accented text is every message rather
-- 
cgit v1.2.3


From d492192b7a9f682dac4a530a5a68ed74f006ddc2 Mon Sep 17 00:00:00 2001
From: "Danilo M." <danix@danix.xyz>
Date: Thu, 20 Aug 2026 18:26:37 +0200
Subject: fix(compose): correct the header's attribution and harden three
 tests, item 123

The header still credited CMARK_OPT_SAFE after the .cpp comment and the test
were corrected, which left the wrong mechanism named in the file
MessageBuilder's author will actually read.

Three test weaknesses, each measured rather than assumed. The accented-text
test survived a SYMMETRIC latin-1 mutation, since the round trip cancels for
codepoints under U+0100, so it now carries a character latin-1 cannot
represent. The tasklist test asserted on the bare word "checked", which
ordinary prose would satisfy, and now asserts the attribute. And the
extension registration is wrapped in a function-local static: cmark-gfm's
registry has no once-guard, and this project has a worker thread, so the
first call racing itself would tear the registry rather than crash cleanly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
---
 src/markdownrenderer.cpp        | 15 ++++++++++-----
 src/markdownrenderer.h          |  4 +++-
 tests/CMakeLists.txt            |  2 +-
 tests/test_markdownrenderer.cpp | 11 +++++++++--
 4 files changed, 23 insertions(+), 9 deletions(-)

(limited to 'tests/test_markdownrenderer.cpp')

diff --git a/src/markdownrenderer.cpp b/src/markdownrenderer.cpp
index ccb3309..7158981 100644
--- a/src/markdownrenderer.cpp
+++ b/src/markdownrenderer.cpp
@@ -45,11 +45,16 @@ QString MarkdownRenderer::toHtml(const QString &markdown)
     if (markdown.isEmpty())
         return {};
 
-    // Idempotent and required before cmark_find_syntax_extension() can resolve
-    // any name. Calling it per render rather than once at startup keeps this
-    // function free of initialisation order concerns; it is a hash lookup
-    // after the first call.
-    cmark_gfm_core_extensions_ensure_registered();
+    // Idempotent, and a hash lookup after the first call. The function-local
+    // static makes the FIRST call thread-safe: cmark-gfm's registry carries no
+    // once-guard of its own, so two threads racing the first call would tear
+    // it. Today's only caller is on the UI thread; this costs nothing and
+    // removes the trap before a worker-thread caller finds it.
+    static const bool registered = [] {
+        cmark_gfm_core_extensions_ensure_registered();
+        return true;
+    }();
+    Q_UNUSED(registered)
 
     // CMARK_OPT_DEFAULT is 0, and CMARK_OPT_SAFE is a NO-OP in cmark-gfm 0.29:
     // safe mode has been the default since that release, and the flag is kept
diff --git a/src/markdownrenderer.h b/src/markdownrenderer.h
index 80c52bf..6373fd9 100644
--- a/src/markdownrenderer.h
+++ b/src/markdownrenderer.h
@@ -32,7 +32,9 @@ namespace MarkdownRenderer {
 ///
 /// Three extensions are enabled (autolink, strikethrough, tasklist) and
 /// tables are deliberately not. Raw HTML in the input is suppressed by
-/// CMARK_OPT_SAFE.
+/// cmark-gfm's safe mode, which is the DEFAULT in 0.29 and is not the
+/// CMARK_OPT_SAFE flag (a no-op); see markdownrenderer.cpp for the
+/// measurement. The requirement is that CMARK_OPT_UNSAFE is never set.
 QString toHtml(const QString &markdown);
 
 }  // namespace MarkdownRenderer
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 9f6e851..58d5659 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -68,10 +68,10 @@ add_qtmaildir_test(searchterm)
 add_qtmaildir_test(busyindicator)
 add_qtmaildir_test(tagstrip)
 add_qtmaildir_test(messagedetailsdialog)
+add_qtmaildir_test(markdownrenderer)
 add_qtmaildir_test(translations)
 # Asserts on the tracked .ts rather than the generated .qm: an untranslated
 # string is dropped by lrelease, so it is invisible in the .qm and shows up
 # only as English in a running Italian UI.
 target_compile_definitions(test_translations PRIVATE
     TRANSLATIONS_DIR="${CMAKE_SOURCE_DIR}/translations")
-add_qtmaildir_test(markdownrenderer)
diff --git a/tests/test_markdownrenderer.cpp b/tests/test_markdownrenderer.cpp
index 0c35b6c..697a28f 100644
--- a/tests/test_markdownrenderer.cpp
+++ b/tests/test_markdownrenderer.cpp
@@ -74,7 +74,10 @@ void TestMarkdownRenderer::tasklistRenders()
     const QString html = MarkdownRenderer::toHtml(
         QStringLiteral("- [ ] todo\n- [x] done"));
     QVERIFY2(html.contains(QStringLiteral("type=\"checkbox\"")), qPrintable(html));
-    QVERIFY2(html.contains(QStringLiteral("checked")), qPrintable(html));
+    // Not a bare "checked": that is a common English word ordinary prose
+    // would satisfy on its own. The attribute is what proves [x] differs
+    // from [ ].
+    QVERIFY2(html.contains(QStringLiteral("checked=\"\"")), qPrintable(html));
 }
 
 void TestMarkdownRenderer::tablesAreNotEnabled()
@@ -127,7 +130,11 @@ void TestMarkdownRenderer::accentedTextSurvivesAsUtf8()
     // This user writes Italian, so accented text is every message rather
     // than an edge case, and a UTF-8 round trip through a C library is
     // exactly where it would be lost.
-    const QString source = QString::fromUtf8("perch\xC3\xA9 \xC3\xA8 cos\xC3\xAC");
+    //
+    // Includes a character outside latin-1, so a symmetric toLatin1/fromLatin1
+    // substitution cannot round-trip it and cancel itself out. Measured: with
+    // accented latin-1 text alone, mutating both sides together passes.
+    const QString source = QString::fromUtf8("perch\xC3\xA9 \xC3\xA8 cos\xC3\xAC \xE2\x82\xAC");
     const QString html = MarkdownRenderer::toHtml(source);
     QVERIFY2(html.contains(source), qPrintable(html));
 }
-- 
cgit v1.2.3