aboutsummaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
Diffstat (limited to 'tests')
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/test_composecontext.cpp53
-rw-r--r--tests/test_composewindow.cpp253
-rw-r--r--tests/test_htmlsanitiser.cpp249
-rw-r--r--tests/test_mainwindow.cpp34
-rw-r--r--tests/test_messagebuilder.cpp100
6 files changed, 679 insertions, 11 deletions
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 2cb3651..1646028 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -49,6 +49,7 @@ target_compile_definitions(test_mimeparser PRIVATE
FIXTURE_DIR="${CMAKE_CURRENT_SOURCE_DIR}/fixtures")
add_qtmaildir_test(interceptor)
add_qtmaildir_test(htmlbuilder)
+add_qtmaildir_test(htmlsanitiser)
add_qtmaildir_test(notmuchworker)
add_qtmaildir_test(tagcolors)
add_qtmaildir_test(cardlayout)
diff --git a/tests/test_composecontext.cpp b/tests/test_composecontext.cpp
index bea390d..50b7ab3 100644
--- a/tests/test_composecontext.cpp
+++ b/tests/test_composecontext.cpp
@@ -96,6 +96,7 @@ private slots:
// Quoting.
void aQuotedBodyPrefixesEveryLine();
+ void anHtmlOnlyBodyIsQuotedAsText();
private:
QString writeConfig(const QString &contents);
@@ -1124,5 +1125,57 @@ void TestComposeContext::aQuotedBodyPrefixesEveryLine()
.arg(quotedCrlf)));
}
+/// Item 171's silent half. An HTML-only original has an EMPTY `plainBody`, so
+/// quoting it produced an attribution line and nothing else: the content was
+/// gone and nothing said so. Measured on the developer's own inbox 2026-08-27,
+/// 30 of 342 sampled messages (~9%) declare text/html with no text/plain, so
+/// this is not an edge case.
+///
+/// The fallback renders the HTML down to text. It does NOT preserve
+/// formatting, which is the separate half of item 171 and is answered by the
+/// multipart/alternative build; this only guarantees the words survive.
+void TestComposeContext::anHtmlOnlyBodyIsQuotedAsText()
+{
+ ParsedMessage message;
+ message.from = QStringLiteral("Sender <sender@example.org>");
+ message.date = QStringLiteral("Thu, 20 Aug 2026 10:00:00 +0200");
+ message.htmlBody = QStringLiteral(
+ "<p>Revenue rose <b>12%</b> against forecast.</p><ul><li>Region A</li></ul>");
+ // plainBody deliberately empty: this is the shape that lost the content.
+
+ const QString quoted = ComposeContextBuilder::quoteBody(message);
+
+ QVERIFY2(quoted.contains(QStringLiteral("Revenue rose")),
+ qPrintable(QStringLiteral("the body was lost:\n%1").arg(quoted)));
+ QVERIFY2(quoted.contains(QStringLiteral("Region A")),
+ qPrintable(QStringLiteral("list content was lost:\n%1").arg(quoted)));
+ QVERIFY2(quoted.contains(QStringLiteral("12%")),
+ qPrintable(QStringLiteral("emphasised text was lost:\n%1").arg(quoted)));
+
+ // Quoted like any other body, not dumped raw.
+ QVERIFY2(quoted.contains(QStringLiteral("> Revenue rose")),
+ qPrintable(QStringLiteral("the fallback is not quoted:\n%1").arg(quoted)));
+
+ // Text, not markup: the plain half of a message must not carry tags.
+ QVERIFY2(!quoted.contains(QStringLiteral("<b>")),
+ qPrintable(QStringLiteral("markup reached the plain quote:\n%1").arg(quoted)));
+ QVERIFY2(!quoted.contains(QStringLiteral("<p>")),
+ qPrintable(QStringLiteral("markup reached the plain quote:\n%1").arg(quoted)));
+
+ // A message WITH a plain part must keep using it, untouched: the fallback
+ // is for the empty case only, and rendering HTML over a real plain part
+ // would change every ordinary reply.
+ ParsedMessage both;
+ both.plainBody = QStringLiteral("the real plain part");
+ both.htmlBody = QStringLiteral("<p>the html part</p>");
+ const QString preferred = ComposeContextBuilder::quoteBody(both);
+ QVERIFY2(preferred.contains(QStringLiteral("> the real plain part")),
+ qPrintable(QStringLiteral("the plain part was not preferred:\n%1")
+ .arg(preferred)));
+ QVERIFY2(!preferred.contains(QStringLiteral("the html part")),
+ qPrintable(QStringLiteral("the html part was used anyway:\n%1")
+ .arg(preferred)));
+}
+
QTEST_MAIN(TestComposeContext)
#include "test_composecontext.moc"
diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp
index 779d48c..d95d55d 100644
--- a/tests/test_composewindow.cpp
+++ b/tests/test_composewindow.cpp
@@ -18,11 +18,14 @@
#include <QtTest>
#include <QComboBox>
+#include <QCheckBox>
#include <QDir>
#include <QFile>
+#include <QFileInfo>
#include <QMenu>
#include <QPlainTextEdit>
#include <QSignalSpy>
+#include <QSplitter>
#include <QLabel>
#include <QMenuBar>
#include <QToolBar>
@@ -62,6 +65,9 @@ private slots:
void onlyTheSetterWritesTheDirtyFlag();
void theMenuBarReachesEveryComposerAction();
void saveDraftWritesAndReports();
+ void aSavedDraftIsFlaggedSeen();
+ void aForwardCarriesTheOriginalHtmlAndStripsRemoteContent();
+ void anHtmlForwardPreviewsTheOriginalInsteadOfQuotingIt();
void theMenusReuseTheToolbarActions();
void theHtmlMenuItemTracksTheToolbarButton();
void theAgeLineFollowsTheClock();
@@ -746,6 +752,253 @@ void TestComposeWindow::saveDraftWritesAndReports()
QVERIFY2(!window.isWindowModified(), "a manual save must clear the marker");
}
+/// A draft is authored by the user, so it is SEEN by definition and must never
+/// be tagged `unread`.
+///
+/// `maildir.synchronize_flags` is on, so the tag is decided by the filename:
+/// notmuch tags any message lacking the `S` flag `unread`. Writing a draft as
+/// `:2,D` therefore puts it in the Unread view until the folder next syncs,
+/// at which point mbsync round-trips the file and the `S` appears. That is
+/// what made the defect look intermittent: only the newest draft, in a folder
+/// that has not synced since, shows the symptom. Measured on the user's own
+/// mail 2026-08-27, where two drafts written two minutes apart differed only
+/// in whether their folder had synced afterwards.
+///
+/// Asserting on the FILENAME rather than on a notmuch tag is deliberate: the
+/// flags are what the code here controls, and a tag assertion would need an
+/// indexed database to say the same thing less directly.
+void TestComposeWindow::aSavedDraftIsFlaggedSeen()
+{
+ const Config config = configWithDrafts();
+
+ ComposeContext context;
+ context.kind = ComposeContext::Kind::New;
+ context.accountKey = QStringLiteral("work");
+
+ ComposeWindow window(context, config, m_dir->path());
+ auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body"));
+ QVERIFY(body);
+ body->setPlainText(QStringLiteral("A draft the user wrote."));
+
+ QSignalSpy saved(&window, &ComposeWindow::draftSaved);
+ auto *save = window.findChild<QAction *>(QStringLiteral("compose_save"));
+ QVERIFY(save);
+ save->trigger();
+
+ QCOMPARE(saved.size(), 1);
+ const QString path = saved.first().first().toString();
+ QVERIFY2(!path.isEmpty(), "the save reported no path");
+
+ // The guard the probe needs: without it a rename that dropped the info
+ // suffix entirely would pass the S check below by never reaching it.
+ const QString name = QFileInfo(path).fileName();
+ QVERIFY2(name.contains(QStringLiteral(":2,")),
+ qPrintable(QStringLiteral("no Maildir info suffix in %1").arg(name)));
+
+ const QString flags = name.section(QStringLiteral(":2,"), 1);
+ QVERIFY2(flags.contains(QLatin1Char('D')),
+ qPrintable(QStringLiteral("a draft must carry the D flag, got %1")
+ .arg(flags)));
+ QVERIFY2(flags.contains(QLatin1Char('S')),
+ qPrintable(QStringLiteral("a draft must carry the S flag or notmuch "
+ "tags it unread, got %1").arg(flags)));
+}
+
+/// Item 171, the composer half. A forward of an HTML message carries the
+/// original's markup, with remote content stripped BY DEFAULT and a control to
+/// keep it.
+///
+/// The default is the security-relevant half: forwarding a tracking pixel
+/// forwards the tracking, and the original sender learns the recipient opened
+/// it. The user chose "ask per forward, default to strip" over always
+/// stripping and over keeping everything.
+void TestComposeWindow::aForwardCarriesTheOriginalHtmlAndStripsRemoteContent()
+{
+ const Config config = configWithDrafts();
+
+ // A real file on disk: the composer reads originalPath itself, exactly as
+ // extractForwardedAttachments() does, so a fixture built in memory would
+ // not exercise the path that runs.
+ const QString path = m_dir->path() + QStringLiteral("/original.eml");
+ writeFile(path, QStringLiteral(
+ "From: Sender <sender@example.org>\r\n"
+ "To: someone@example.org\r\n"
+ "Subject: Quarterly report\r\n"
+ "Date: Wed, 26 Aug 2026 10:00:00 +0200\r\n"
+ "MIME-Version: 1.0\r\n"
+ "Content-Type: text/html; charset=utf-8\r\n"
+ "\r\n"
+ "<p>Revenue rose <b>12%</b>.</p>"
+ "<img src=\"https://tracker.example/px?id=abc\">\r\n"));
+
+ ComposeContext context;
+ context.kind = ComposeContext::Kind::Forward;
+ context.accountKey = QStringLiteral("work");
+ context.originalPath = path;
+ context.subject = QStringLiteral("Fwd: Quarterly report");
+
+ ComposeWindow window(context, config, m_dir->path());
+
+ auto *strip = window.findChild<QCheckBox *>(QStringLiteral("stripRemote"));
+ QVERIFY2(strip, "there is no strip-remote-content control on a forward");
+ QVERIFY2(strip->isChecked(),
+ "stripping must be the DEFAULT: a forward must not leak a "
+ "tracking pixel to the recipient unless the user asks for it");
+
+ // Checked: the markup survives, the beacon does not.
+ const OutgoingMessage stripped = window.currentMessage();
+ QVERIFY2(stripped.forwardedHtml.contains(QStringLiteral("Revenue rose")),
+ qPrintable(QStringLiteral("the original's markup was lost:\n%1")
+ .arg(stripped.forwardedHtml)));
+ QVERIFY2(stripped.forwardedHtml.contains(QStringLiteral("<b>")),
+ "the formatting was flattened, which is the defect being fixed");
+ QVERIFY2(!stripped.forwardedHtml.contains(QStringLiteral("tracker.example")),
+ qPrintable(QStringLiteral("a tracking pixel survived:\n%1")
+ .arg(stripped.forwardedHtml)));
+
+ // Unchecked: the user's explicit choice is honoured.
+ strip->setChecked(false);
+ const OutgoingMessage kept = window.currentMessage();
+ QVERIFY2(kept.forwardedHtml.contains(QStringLiteral("tracker.example")),
+ "unchecking the control must actually keep the remote content");
+
+ // A New message has neither the control nor any forwarded markup.
+ ComposeContext fresh;
+ fresh.kind = ComposeContext::Kind::New;
+ fresh.accountKey = QStringLiteral("work");
+ ComposeWindow plain(fresh, config, m_dir->path());
+ QVERIFY2(plain.currentMessage().forwardedHtml.isEmpty(),
+ "a new message must carry no forwarded markup");
+}
+
+/// Item 171, the WYSIWYG half. **What the composer shows must be what gets
+/// sent**, and for an HTML forward the editable buffer cannot be that.
+///
+/// The first build seeded the text quote into the buffer and then dropped it
+/// when building the HTML part, so the user could edit a quote whose edits
+/// were silently discarded. That is worse than the defect it replaced: the
+/// previous version at least sent what it displayed.
+///
+/// So on an HTML forward the buffer holds the user's own note ONLY, and the
+/// original appears in a read-only preview instead. Nothing shown is
+/// editable-but-ignored, and nothing sent is unshown. The user chose this over
+/// a rich-text composer (recorded as item 173, which is the real WYSIWYG
+/// answer and a much larger piece of work) and over attaching the original.
+void TestComposeWindow::anHtmlForwardPreviewsTheOriginalInsteadOfQuotingIt()
+{
+ const Config config = configWithDrafts();
+
+ const QString path = m_dir->path() + QStringLiteral("/original.eml");
+ writeFile(path, QStringLiteral(
+ "From: Sender <sender@example.org>\r\n"
+ "To: someone@example.org\r\n"
+ "Subject: Quarterly report\r\n"
+ "Date: Wed, 26 Aug 2026 10:00:00 +0200\r\n"
+ "MIME-Version: 1.0\r\n"
+ "Content-Type: text/html; charset=utf-8\r\n"
+ "\r\n"
+ "<p>Revenue rose <b>12%</b>.</p>\r\n"));
+
+ ComposeContext context;
+ context.kind = ComposeContext::Kind::Forward;
+ context.accountKey = QStringLiteral("work");
+ context.originalPath = path;
+ context.subject = QStringLiteral("Fwd: Quarterly report");
+ context.quotedBody = QStringLiteral(
+ "On Wed, sender@example.org wrote:\n\n> Revenue rose 12%.");
+
+ ComposeWindow window(context, config, m_dir->path());
+
+ auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body"));
+ QVERIFY(body);
+
+ // The buffer carries the user's note only: no quote to edit in vain.
+ QVERIFY2(!body->toPlainText().contains(QStringLiteral("Revenue rose")),
+ qPrintable(QStringLiteral("the original was seeded into the "
+ "editable buffer:\n%1").arg(body->toPlainText())));
+
+ // The preview says what will be carried, and is NOT editable.
+ auto *preview = window.findChild<QWidget *>(QStringLiteral("forwardPreview"));
+ QVERIFY2(preview, "an HTML forward must show what it will carry");
+ QVERIFY2(preview->isVisibleTo(&window),
+ "the preview must not be hidden on an HTML forward");
+
+ // **Beside the editor, not under it**, at the user's request 2026-08-27:
+ // a vertical split, editor 60 and preview 40, so the note being written
+ // and the message being forwarded are read side by side.
+ auto *split = window.findChild<QSplitter *>(QStringLiteral("composeSplit"));
+ QVERIFY2(split, "the preview must share a splitter with the editor");
+ QCOMPARE(split->orientation(), Qt::Horizontal);
+ QCOMPARE(split->count(), 2);
+ QCOMPARE(split->widget(0), static_cast<QWidget *>(body));
+ QCOMPARE(split->widget(1), preview);
+
+ // **The ratio is asserted as STRETCH FACTORS, not as resulting pixels.**
+ // CLAUDE.md records that the offscreen platform cannot test window sizing:
+ // it prints "This plugin does not support propagateSizeHints()" and the
+ // splitter here has no real width to divide, so sizes() reports an equal
+ // 49/49 whatever the code asks for. Measured: a pixel assertion fails
+ // against correct code. The stretch factors are what the layout stores and
+ // what survives the first real resize, so they are the testable intent;
+ // the appearance is a hand test.
+ // QSplitter has no stretchFactor() getter: setStretchFactor() writes the
+ // value into the CHILD's size policy, which is where it can be read back.
+ QCOMPARE(body->sizePolicy().horizontalStretch(), 6);
+ QCOMPARE(preview->sizePolicy().horizontalStretch(), 4);
+
+ // A toggle closes and reopens it.
+ auto *toggle = window.findChild<QAction *>(QStringLiteral("compose_show_forward"));
+ QVERIFY2(toggle, "there is no toggle for the forwarded-message pane");
+ QVERIFY2(toggle->isCheckable(), "the pane toggle must be checkable");
+ QVERIFY2(toggle->isChecked(), "the pane starts open on an HTML forward");
+
+ toggle->trigger();
+ QVERIFY2(!preview->isVisibleTo(&window),
+ "unchecking the toggle must hide the forwarded-message pane");
+ toggle->trigger();
+ QVERIFY2(preview->isVisibleTo(&window),
+ "re-checking the toggle must bring the pane back");
+
+ // What is sent still contains the original, from the markup rather than
+ // from the buffer.
+ const OutgoingMessage message = window.currentMessage();
+ QVERIFY2(message.forwardedHtml.contains(QStringLiteral("Revenue rose")),
+ "the forward must still carry the original");
+
+ // A PLAIN forward is unchanged: the quote goes in the buffer, where it is
+ // both editable and sent, so WYSIWYG already held there and must not be
+ // broken by this.
+ const QString plainPath = m_dir->path() + QStringLiteral("/plain.eml");
+ writeFile(plainPath, QStringLiteral(
+ "From: Sender <sender@example.org>\r\n"
+ "Subject: Plain report\r\n"
+ "Date: Wed, 26 Aug 2026 10:00:00 +0200\r\n"
+ "\r\n"
+ "Revenue rose 12%.\r\n"));
+
+ ComposeContext plainContext;
+ plainContext.kind = ComposeContext::Kind::Forward;
+ plainContext.accountKey = QStringLiteral("work");
+ plainContext.originalPath = plainPath;
+ plainContext.quotedBody = QStringLiteral("> Revenue rose 12%.");
+
+ ComposeWindow plainWindow(plainContext, config, m_dir->path());
+ auto *plainBody = plainWindow.findChild<QPlainTextEdit *>(QStringLiteral("body"));
+ QVERIFY(plainBody);
+ QVERIFY2(plainBody->toPlainText().contains(QStringLiteral("Revenue rose")),
+ qPrintable(QStringLiteral("a plain forward lost its quote:\n%1")
+ .arg(plainBody->toPlainText())));
+
+ auto *noPreview = plainWindow.findChild<QWidget *>(QStringLiteral("forwardPreview"));
+ QVERIFY2(!noPreview || !noPreview->isVisibleTo(&plainWindow),
+ "a plain forward needs no preview: its quote is in the buffer");
+
+ auto *noToggle = plainWindow.findChild<QAction *>(
+ QStringLiteral("compose_show_forward"));
+ QVERIFY2(!noToggle || !noToggle->isVisible(),
+ "a plain forward must not offer a pane toggle that does nothing");
+}
+
/// The same QAction objects, shown twice over, exactly as item 140 required
/// for the message pane's bar. A copy would drift: an enablement change or a
/// new shortcut would reach one surface and not the other.
diff --git a/tests/test_htmlsanitiser.cpp b/tests/test_htmlsanitiser.cpp
new file mode 100644
index 0000000..043891b
--- /dev/null
+++ b/tests/test_htmlsanitiser.cpp
@@ -0,0 +1,249 @@
+/*
+ * 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 <QRegularExpression>
+
+#include "htmlsanitiser.h"
+
+/// Item 171's security half.
+///
+/// Every test here asserts on the STRING, never on a render. A rendering probe
+/// cannot see this defect by construction: a tracking pixel is a 1x1
+/// transparent image, invisible by design, so a probe that renders the output
+/// and looks at it would endorse the exact thing being guarded against.
+class TestHtmlSanitiser : public QObject
+{
+ Q_OBJECT
+
+private slots:
+ void aCidReferenceSurvives();
+ void aRemoteImageIsRemoved();
+ void anUnquotedRemoteUrlIsRemoved();
+ void aProtocolRelativeUrlIsRemoved();
+ void quotingAndCaseAndWhitespaceDoNotHelp();
+ void aFetchingElementIsRemovedWhole();
+ void cssUrlIsStrippedInBothPlaces();
+ void anEventHandlerIsRemoved();
+ void aDataUrlIsRemoved();
+ void anUnknownAttributeCarryingAUrlIsRemoved();
+ void aCidWhoseIdLooksLikeAUrlSurvives();
+ void structuralMarkupSurvives();
+ void hasRemoteContentAnswersForTheComposer();
+
+private:
+ /// The invariant, applied to a whole output: no scheme but cid: anywhere.
+ ///
+ /// Deliberately crude and deliberately independent of the implementation's
+ /// own patterns. A test that reused the production regexes would agree
+ /// with a bug rather than catch it.
+ void assertNoRemoteUrls(const QString &out);
+};
+
+void TestHtmlSanitiser::assertNoRemoteUrls(const QString &out)
+{
+ const QString lowered = out.toLower();
+ for (const char *needle : { "http:", "https:", "//evil", "//host",
+ "data:", "file:", "ftp:" }) {
+ QVERIFY2(!lowered.contains(QLatin1String(needle)),
+ qPrintable(QStringLiteral("a %1 reference survived: %2")
+ .arg(QLatin1String(needle), out)));
+ }
+}
+
+/// The one thing that must NOT be stripped. A cid: travels inside the message
+/// and fetches nothing, so an inline logo survives a forward.
+void TestHtmlSanitiser::aCidReferenceSurvives()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<p>hi</p><img src=\"cid:logo@example.org\">"));
+
+ QVERIFY2(out.contains(QStringLiteral("cid:logo@example.org")),
+ qPrintable(QStringLiteral("the cid was lost: ") + out));
+ QVERIFY2(out.contains(QStringLiteral("<p>hi</p>")),
+ qPrintable(QStringLiteral("the body was lost: ") + out));
+}
+
+/// The reported harm, in its plainest form.
+void TestHtmlSanitiser::aRemoteImageIsRemoved()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<p>hi</p><img src=\"https://evil.example/px?id=you\" "
+ "width=\"1\" height=\"1\">"));
+
+ assertNoRemoteUrls(out);
+ QVERIFY2(out.contains(QStringLiteral("<p>hi</p>")),
+ qPrintable(QStringLiteral("the body was lost: ") + out));
+}
+
+/// `<img src=cid:x>` is valid HTML and unquoted references are seen in the
+/// wild; namespaceCids() documents the same. An implementation that only
+/// handles quoted values passes every tidy test and leaks on real mail.
+void TestHtmlSanitiser::anUnquotedRemoteUrlIsRemoved()
+{
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<img src=https://evil.example/px>")));
+}
+
+/// No scheme at all, and it still fetches: the recipient's client supplies
+/// whichever scheme it rendered the message under. A check for "http" misses
+/// this entirely.
+void TestHtmlSanitiser::aProtocolRelativeUrlIsRemoved()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<img src=\"//evil.example/px\">"));
+
+ QVERIFY2(!out.contains(QStringLiteral("//evil.example")),
+ qPrintable(QStringLiteral("a protocol-relative URL survived: ")
+ + out));
+}
+
+/// Uppercase tags, single quotes, and newlines around '=' are all real. Each
+/// one alone defeats a naive pattern.
+void TestHtmlSanitiser::quotingAndCaseAndWhitespaceDoNotHelp()
+{
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<IMG SRC = 'https://evil.example/a'>")));
+
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<img\n src\n =\n \"https://evil.example/b\">")));
+
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<img SrC=\"HTTPS://EVIL.EXAMPLE/c\">")));
+}
+
+/// These elements exist to fetch or redirect. Emptying the attribute is not
+/// enough for <script>, whose CONTENT is the payload.
+void TestHtmlSanitiser::aFetchingElementIsRemovedWhole()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<p>keep</p>"
+ "<link rel=\"stylesheet\" href=\"https://evil.example/s.css\">"
+ "<script>fetch('https://evil.example/beacon')</script>"
+ "<iframe src=\"https://evil.example/f\"></iframe>"
+ "<base href=\"https://evil.example/\">"
+ "<meta http-equiv=\"refresh\" content=\"0;url=https://evil.example/\">"));
+
+ assertNoRemoteUrls(out);
+ QVERIFY2(!out.toLower().contains(QStringLiteral("<script")),
+ qPrintable(QStringLiteral("a script element survived: ") + out));
+ QVERIFY2(!out.toLower().contains(QStringLiteral("<iframe")),
+ qPrintable(QStringLiteral("an iframe survived: ") + out));
+ QVERIFY2(out.contains(QStringLiteral("<p>keep</p>")),
+ qPrintable(QStringLiteral("the body was lost: ") + out));
+}
+
+/// CSS fetches too, and it reaches the same network from two different places
+/// with different terminator rules. namespaceCids() handles both for the same
+/// reason.
+void TestHtmlSanitiser::cssUrlIsStrippedInBothPlaces()
+{
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<div style=\"background:url(https://evil.example/bg.png)\">x</div>")));
+
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<style>@import url('https://evil.example/s.css');"
+ "p{background:url(https://evil.example/b.png)}</style>")));
+
+ // The bare form terminates on ')', not on a quote.
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<div style='background:url(//evil.example/bg.png)'>x</div>")));
+}
+
+/// The recipient's client most likely disables scripting. That is their
+/// policy, not ours to assume on their behalf.
+void TestHtmlSanitiser::anEventHandlerIsRemoved()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<img src=\"cid:x\" onerror=\"fetch('https://evil.example/b')\">"));
+
+ assertNoRemoteUrls(out);
+ QVERIFY2(!out.toLower().contains(QStringLiteral("onerror")),
+ qPrintable(QStringLiteral("an event handler survived: ") + out));
+}
+
+/// A data: URL carries its payload inline, so it does not fetch, but it CAN
+/// carry markup and is a standard sanitiser bypass. Removed on the allow-list
+/// rule: it is not cid:, so it goes.
+void TestHtmlSanitiser::aDataUrlIsRemoved()
+{
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<img src=\"data:text/html;base64,PHNjcmlwdD4=\">")));
+}
+
+/// **The allow-list's whole point.** `namespaceCids()` enumerates the
+/// attributes it rewrites and scopes srcset out; doing that here would leak.
+/// An attribute nobody anticipated must be handled by the DEFAULT.
+void TestHtmlSanitiser::anUnknownAttributeCarryingAUrlIsRemoved()
+{
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<img srcset=\"https://evil.example/1x.png 1x, "
+ "https://evil.example/2x.png 2x\">")));
+
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<div data-bg=\"https://evil.example/x.png\">x</div>")));
+
+ assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<video poster=\"https://evil.example/p.jpg\"></video>")));
+}
+
+/// A cid: id may legitimately contain something URL-shaped. Stripping on a
+/// substring match rather than on the SCHEME would destroy a valid reference.
+void TestHtmlSanitiser::aCidWhoseIdLooksLikeAUrlSurvives()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(
+ QStringLiteral("<img src=\"cid:https-logo@example.org\">"));
+
+ QVERIFY2(out.contains(QStringLiteral("cid:https-logo@example.org")),
+ qPrintable(QStringLiteral("a valid cid was destroyed: ") + out));
+}
+
+/// The formatting is the entire point of the feature. A sanitiser that keeps
+/// the user safe by emptying the message has not solved item 171.
+void TestHtmlSanitiser::structuralMarkupSurvives()
+{
+ const QString out = HtmlSanitiser::stripRemoteContent(QStringLiteral(
+ "<table><tr><td style=\"color:#c00;font-weight:bold\">Revenue</td>"
+ "<td>up 12%</td></tr></table><ul><li>Region A</li></ul>"));
+
+ QVERIFY2(out.contains(QStringLiteral("<table")),
+ qPrintable(QStringLiteral("the table was lost: ") + out));
+ QVERIFY2(out.contains(QStringLiteral("Revenue")),
+ qPrintable(QStringLiteral("the text was lost: ") + out));
+ QVERIFY2(out.contains(QStringLiteral("font-weight:bold")),
+ qPrintable(QStringLiteral("safe styling was lost: ") + out));
+ QVERIFY2(out.contains(QStringLiteral("<li>Region A</li>")),
+ qPrintable(QStringLiteral("the list was lost: ") + out));
+}
+
+/// Drives whether the composer offers the checkbox at all. It must never
+/// decide whether to strip.
+void TestHtmlSanitiser::hasRemoteContentAnswersForTheComposer()
+{
+ QVERIFY(HtmlSanitiser::hasRemoteContent(
+ QStringLiteral("<img src=\"https://evil.example/px\">")));
+ QVERIFY(HtmlSanitiser::hasRemoteContent(
+ QStringLiteral("<div style=\"background:url(//evil.example/b)\">x</div>")));
+
+ QVERIFY(!HtmlSanitiser::hasRemoteContent(
+ QStringLiteral("<p>plain</p><img src=\"cid:logo@example.org\">")));
+ QVERIFY(!HtmlSanitiser::hasRemoteContent(QStringLiteral("<p>plain</p>")));
+}
+
+QTEST_MAIN(TestHtmlSanitiser)
+#include "test_htmlsanitiser.moc"
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index a572fd0..08589b2 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -12955,15 +12955,23 @@ void TestMainWindow::theComposerSplitsItsToolbarByScope()
auto *column = qobject_cast<QVBoxLayout *>(central->layout());
QVERIFY2(column, "the composer is not laid out in a vertical column");
- int barIndex = -1;
- int bodyIndex = -1;
- for (int i = 0; i < column->count(); ++i) {
- QLayoutItem *item = column->itemAt(i);
- if (item->widget() == editorBar)
- barIndex = i;
- else if (item->widget() == body)
- bodyIndex = i;
- }
+ // The editor sits inside a QSplitter since item 171, so its position in
+ // the column is the SPLITTER's: a forward puts the forwarded message
+ // beside the editor, and the toolbar must stay above both. Walking up to
+ // whichever child of the column contains the body keeps this test about
+ // the toolbar's position rather than about the editor's parentage.
+ const auto columnChildOf = [column](QWidget *widget) {
+ for (QWidget *w = widget; w; w = w->parentWidget()) {
+ for (int i = 0; i < column->count(); ++i) {
+ if (column->itemAt(i)->widget() == w)
+ return i;
+ }
+ }
+ return -1;
+ };
+
+ const int barIndex = columnChildOf(editorBar);
+ const int bodyIndex = columnChildOf(body);
QVERIFY2(barIndex >= 0 && bodyIndex >= 0,
"the editor bar or the body is not in the composer's column");
QVERIFY2(barIndex < bodyIndex, "the editor bar is not above the editor");
@@ -13753,8 +13761,12 @@ void TestMainWindow::anAutosaveWritesADraftAndClearsTheDirtyFlag()
QVERIFY(written.open(QIODevice::ReadOnly));
const QByteArray bytes = written.readAll();
QVERIFY2(bytes.contains("Draft body."), "the draft does not carry the body");
- // Written with the Maildir draft flag, not left bare.
- QVERIFY2(files.first().endsWith(QStringLiteral(":2,D")),
+ // Written with the Maildir draft flag, not left bare. The flag SET is not
+ // pinned here: a draft also carries S, asserted by
+ // TestComposeWindow::aSavedDraftIsFlaggedSeen(), and an endsWith(":2,D")
+ // here would fail against that correct behaviour.
+ QVERIFY2(files.first().section(QStringLiteral(":2,"), 1)
+ .contains(QLatin1Char('D')),
qPrintable(QStringLiteral("wrong maildir flags: ") + files.first()));
}
diff --git a/tests/test_messagebuilder.cpp b/tests/test_messagebuilder.cpp
index 73d388c..2ea14f0 100644
--- a/tests/test_messagebuilder.cpp
+++ b/tests/test_messagebuilder.cpp
@@ -29,6 +29,7 @@
#include "config.h"
#include "messagebuilder.h"
+#include "mimeparser.h"
#include "types.h"
/// MessageBuilder's tests assert on the GENERATED BYTES, never by round-tripping
@@ -55,6 +56,7 @@ private slots:
void aDirectoryAttachmentFailsRatherThanHangingTheProcess();
void anUnparseableRecipientFailsRatherThanVanishing();
void everyMessageCarriesADateAndMessageId();
+ void aForwardSendsOnePartChosenByTheHtmlToggle();
void recipientsAppearInTheirOwnHeaders();
void anAccountWithNoAddressFailsRatherThanBuildingHeaderlessMail();
@@ -462,5 +464,103 @@ void TestMessageBuilder::anAccountWithNoAddressFailsRatherThanBuildingHeaderless
QVERIFY(r.bytes.isEmpty());
}
+/// Item 171. A forward sends ONE part, chosen by the Send-as-HTML toggle:
+/// the original's markup when it is on, the text quote when it is off.
+///
+/// **No multipart/alternative on a forward**, at the user's decision
+/// 2026-08-27, reversing the first build. A forward is a message the user has
+/// already decided the shape of by flipping that toggle, and sending both
+/// halves means the recipient's client picks, which is the choice being taken
+/// away from them.
+///
+/// The toggle is honoured even when the original has no plain-text part: with
+/// it off, an HTML-only original forwards as the text fallback and the
+/// formatting is lost. That is the toggle meaning what it says, chosen over
+/// forcing HTML for those messages.
+///
+/// `forwardedHtml` arrives ALREADY SANITISED: whether to strip remote content
+/// is the user's per-forward choice and a builder cannot see a checkbox. The
+/// security property is asserted in test_htmlsanitiser; what matters here is
+/// that the right single part goes out.
+void TestMessageBuilder::aForwardSendsOnePartChosenByTheHtmlToggle()
+{
+ OutgoingMessage m = baseMessage();
+ m.sendHtml = true;
+ // As the composer really supplies it: on an HTML forward the buffer holds
+ // the user's own note ALONE, the original travelling as markup instead, so
+ // that what the composer shows is what gets sent (item 171).
+ m.markdownBody = QStringLiteral("Passing this on.");
+ m.forwardedHtml = QStringLiteral("<p>Revenue rose <b>12%</b>.</p>");
+
+ const MessageBuilder::Result r = MessageBuilder::build(m, m_account);
+ QVERIFY2(r.ok(), qPrintable(r.error));
+
+ const QString text = QString::fromUtf8(r.bytes);
+
+ // ONE part, not an alternative.
+ QVERIFY2(!text.contains(QStringLiteral("multipart/alternative")),
+ qPrintable(QStringLiteral("a forward must not send both halves:\n%1")
+ .arg(text)));
+ QVERIFY2(text.contains(QStringLiteral("text/html")),
+ qPrintable(QStringLiteral("no html part:\n%1").arg(text)));
+ QVERIFY2(!text.contains(QStringLiteral("text/plain")),
+ qPrintable(QStringLiteral("a plain part went out too:\n%1").arg(text)));
+
+ QVERIFY2(text.contains(QStringLiteral("Revenue rose")),
+ qPrintable(QStringLiteral("the forwarded body is missing:\n%1").arg(text)));
+ QVERIFY2(text.contains(QStringLiteral("Passing this on")),
+ qPrintable(QStringLiteral("the user's own text was lost:\n%1").arg(text)));
+
+ // **The original must appear ONCE.** The composer seeds the text quote
+ // into the editable body so the user can trim it, so `markdownBody`
+ // already carries a flattened copy of the original; rendering that AND
+ // appending the markup shipped the whole message twice, the first copy
+ // with its URLs naked and mangled. Found by hand-testing on 2026-08-27
+ // against a real newsletter, where it read as two messages stacked.
+ QVERIFY2(!text.contains(QStringLiteral("<blockquote>")),
+ qPrintable(QStringLiteral("the text quote was rendered into the "
+ "html as well as the markup:\n%1").arg(text)));
+ QCOMPARE(text.count(QStringLiteral("Revenue rose")), 1);
+
+ // **The structure is right**, checked by parsing back rather than by
+ // reading the RFC: MimeParser is what the application itself uses.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const QString path = dir.path() + QStringLiteral("/forward.eml");
+ QFile out(path);
+ QVERIFY(out.open(QIODevice::WriteOnly));
+ out.write(r.bytes);
+ out.close();
+
+ MimeParser parser;
+ const ParsedMessage parsed = parser.parse(path);
+ QVERIFY2(parsed.ok, "the built forward does not parse back");
+ QVERIFY2(parsed.htmlBody.contains(QStringLiteral("Revenue rose")),
+ qPrintable(QStringLiteral("the forwarded markup is not in the html "
+ "part on the way back:\n%1").arg(parsed.htmlBody)));
+
+ // Toggle OFF: the plain quote alone, and the markup must not leak into it.
+ OutgoingMessage plainForward = baseMessage();
+ plainForward.sendHtml = false;
+ plainForward.markdownBody = QStringLiteral("Passing this on.\n\n> Revenue rose 12%.");
+ plainForward.forwardedHtml = QStringLiteral("<p>Revenue rose <b>12%</b>.</p>");
+
+ const MessageBuilder::Result r2 = MessageBuilder::build(plainForward, m_account);
+ QVERIFY2(r2.ok(), qPrintable(r2.error));
+ const QString text2 = QString::fromUtf8(r2.bytes);
+
+ QVERIFY2(!text2.contains(QStringLiteral("multipart/alternative")),
+ qPrintable(QStringLiteral("a plain forward must be one part:\n%1")
+ .arg(text2)));
+ QVERIFY2(!text2.contains(QStringLiteral("text/html")),
+ qPrintable(QStringLiteral("html went out with the toggle off:\n%1")
+ .arg(text2)));
+ QVERIFY2(!text2.contains(QStringLiteral("<b>")),
+ qPrintable(QStringLiteral("markup leaked into a plain forward:\n%1")
+ .arg(text2)));
+ QVERIFY2(text2.contains(QStringLiteral("Passing this on")),
+ qPrintable(QStringLiteral("the user's own text was lost:\n%1").arg(text2)));
+}
+
QTEST_MAIN(TestMessageBuilder)
#include "test_messagebuilder.moc"