aboutsummaryrefslogtreecommitdiffstats
path: root/tests/test_mainwindow.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-22 11:19:22 +0200
committerDanilo M. <danix@danix.xyz>2026-08-22 12:00:32 +0200
commita9d1cf73a91b5eef79a9722ec9921c97b9ec5c81 (patch)
tree91f45693edc9098dfd9c35d17eedd1ff8ab7066e /tests/test_mainwindow.cpp
parent141bc9b98213b9c4ba9f3bcba041c3e3108c12a3 (diff)
downloadqtmaildir-compose-and-send.tar.gz
qtmaildir-compose-and-send.zip
feat(compose): wire the composer into the main window, item 123compose-and-send
The reply family is disabled on mail that arrived at an account with no send_command, behind a ribbon in MessageView naming the account and the key to add. save_message is deliberately never disabled: it is the escape hatch for exactly that case. The ribbon is a WIDGET in the pane's layout, never markup inside the web view. Composing HTML from configuration into the one document that renders input from strangers is the wrong direction, and the header row is already a widget for the same reason. Compose itself is disabled only when NO account can send, and that state is not warned about at startup: an installation with no send_command anywhere is a valid read-only installation. Every reply resolves through messageScopeFor(), not threadFor(): a thread row means the one message its card shows. Replying to a thread is meaningless; a reply answers a message. The context is built from the DATABASE rather than the model, the rule Restore already follows, because a row whose state has not been re-queried carries stale values and a reply built from one would carry the wrong recipients. The mail root crosses from the worker as its own signal. There was no route for it at all: mailRootOf() is file-static in notmuchworker.cpp, and item 124 records that composing a destination from database.path writes into the Xapian tree under a split index. The test uses NotmuchFixture::splitIndex(), the only layout where the two accessors disagree. A thread row's path is RELATIVE to the mail root while a message row's is absolute, so the account lookup matched nothing and the reply family was dead on mail from an account that could send. Found by the positive guard test rather than the negative one, which passed throughout for the wrong reason. The quit path checks the failed-save case FIRST. In the ordinary case nothing is lost by saving; there, saving is what is already not working, so the dialog says plainly that quitting loses that text rather than offering a save that will fail again. Both dialogs name the composers, and the ordinary one asks once whatever the count, because three modals in a row is worse than a coarse answer. Its wording says drafts already saved stay in the folder, so Discard cannot read as 'delete my three messages'. The Save loop holds QPointers, not raw pointers. A deleteLater() posted while a nested exec() runs IS processed by that nested loop, measured in a standalone program: the guard nulls before the modal returns. Closing a composer while the quit dialog is up therefore freed a window the loop then called saveDraftNow() on, crashing at the exact moment the application promised to preserve that text. A compose request that matches nothing clears itself and says so. It was cleared only on a match, so a message deleted between selection and Reply left the request armed for the session: Reply did nothing, and the next ordinary click on that message opened a composer nobody asked for while the pane stayed blank. Forward carries the original's attachments, which the context has always had a field for and nothing ever filled, and seeds its HTML toggle from [compose] send_html. Only Reply seeds that from the original. save_message keeps its filename inside the chosen directory and no longer overwrites a file already there. The check was correct and untested: the test asserted through Attachment's helpers rather than through the function production calls, so deleting the containment check outright left it green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvwDptMWxjqhbCmjxwcSZ2
Diffstat (limited to 'tests/test_mainwindow.cpp')
-rw-r--r--tests/test_mainwindow.cpp955
1 files changed, 955 insertions, 0 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index cab6eae..ecaab2f 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -48,6 +48,7 @@
#include "keymap.h"
#include "mainwindow.h"
#include "messageview.h"
+#include "mimeparser.h"
#include "notmuchworker.h"
#include "carddelegate.h"
#include "composewindow.h"
@@ -103,6 +104,35 @@ public:
/// no trash key either. A caller that names an account and wants Delete to
/// work has to say where its trash is, which is the same requirement the
/// real config imposes.
+ /// One [account.<key>] section to write.
+ ///
+ /// `sendCommand` is what makes the account able to send, and its EMPTINESS
+ /// is what makes it receive-only: the capability is the key's presence,
+ /// not a separate flag, so a receive-only account is written by omitting
+ /// it exactly as the real config expresses it.
+ struct AccountSpec
+ {
+ QString key;
+ QString maildir;
+ QString trash;
+ QString sendCommand;
+ QString address;
+ };
+
+ /// Writes several accounts, for the compose cases.
+ ///
+ /// Beside build() rather than replacing it: every existing caller passes
+ /// at most one account and none of them needs a send command, so widening
+ /// the three-argument signature further would make ten call sites carry
+ /// two empty strings each for one test's benefit.
+ bool buildWithAccounts(const QList<AccountSpec> &accounts,
+ const QString &composeKey = QString())
+ {
+ m_accounts = accounts;
+ m_composeKey = composeKey;
+ return build();
+ }
+
bool build(const QString &accountKey = QString(),
const QString &accountMaildir = QString(),
const QString &accountTrash = QString())
@@ -149,6 +179,21 @@ public:
// folder that does not exist would CREATE it.
out << "inbox=inbox\n";
}
+ if (!m_composeKey.isEmpty())
+ out << "\n[compose]\n" << m_composeKey << "\n";
+ for (const AccountSpec &account : m_accounts) {
+ out << "\n[account." << account.key << "]\n"
+ << "maildir=" << account.maildir << "\n"
+ << "inbox=inbox\n";
+ if (!account.trash.isEmpty())
+ out << "trash=" << account.trash << "\n";
+ if (!account.address.isEmpty())
+ out << "address=" << account.address << "\n";
+ // Written only when non-empty. An account with no
+ // send_command is receive-only, which is the shape under test.
+ if (!account.sendCommand.isEmpty())
+ out << "send_command=" << account.sendCommand << "\n";
+ }
}
file.close();
@@ -169,6 +214,8 @@ private:
QTemporaryDir m_confDir;
Config m_config;
QString m_error;
+ QList<AccountSpec> m_accounts;
+ QString m_composeKey;
};
/// MainWindow is mostly wiring. Cases that need a real database opt into one
@@ -204,6 +251,24 @@ private slots:
void narrowingAnEmptyQueryBarIsAPlainSearch();
void aMalformedAccountIsReportedWithoutBlockingTheConstructor();
void aWorkerBackedWindowReturnsRealThreads();
+
+ // Compose and send, item 123 task 12.
+ void theMailRootComesFromTheConfigNotTheIndex();
+ void replyIsDisabledOnAReceiveOnlyAccountsMail();
+ void theReceiveOnlyRibbonNamesTheAccount();
+ void replyIsEnabledOnASendingAccountsMail();
+ void composeIsDisabledOnlyWhenNoAccountCanSend();
+ void quittingWithACleanComposerAsksNothing();
+ void quittingWithUnsavedEditsReportsEveryComposer();
+ void closingAComposerCompactsTheRegistry();
+ void savingAMessageRefusesToEscapeTheChosenDirectory();
+ void aHostileSubjectCannotEscapeTheSaveDirectory();
+ void savingTwiceDoesNotOverwriteTheFirstFile();
+ void savingAMessageWithAHostileSubjectStaysInTheDirectory();
+ void aStuckComposeRequestDoesNotHijackTheNextPaneLoad();
+ void theSaveLoopToleratesAComposerClosedUnderTheDialog();
+ void forwardingCarriesTheOriginalsAttachments();
+ void forwardSeedsHtmlFromTheConfigNotTheOriginal();
void aStartupAccountScopesTheStartupQuery();
void aStartupAccountAlsoScopesASavedStartupQuery();
void aGeneratedStartupQueryActuallyRuns();
@@ -8168,6 +8233,896 @@ void TestMainWindow::aWorkerBackedWindowReturnsRealThreads()
QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
}
+namespace {
+
+/// A worker-backed window with one message in one account's maildir.
+///
+/// The compose cases all need the same three things: a message on disk, an
+/// account owning the folder it landed in, and a selected row. Repeating that
+/// in six tests is how one of them ends up subtly different from the rest.
+struct WorkerComposeFixture
+{
+ WorkerBackedWindow backed;
+
+ /// Writes one message into <accountMaildir>/inbox and indexes it.
+ /// \p composeKey, when given, is written as one line under [compose].
+ bool seed(const QList<WorkerBackedWindow::AccountSpec> &accounts,
+ const QString &folder, const QString &composeKey = QString())
+ {
+ if (!backed.fixture().addMessage(
+ folder, QStringLiteral("compose1@example.org"),
+ QStringLiteral("A subject"),
+ QStringLiteral("sender@example.org"),
+ // Friday, verified with `date -d 2026-08-14 +%A`.
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text."))) {
+ return false;
+ }
+ return backed.buildWithAccounts(accounts, composeKey);
+ }
+
+ /// Runs a query and puts the current index on its one row.
+ ///
+ /// Waits on the MAIL ROOT as well as on the row. The reply family is gated
+ /// on which account owns the message, which needs the root, and that
+ /// arrives on its own queued signal: asserting on an action's enabled
+ /// state before it lands measures the startup race rather than the rule.
+ static bool selectTheMessage(MainWindow &window)
+ {
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<ThreadListView *>();
+ auto *queryEdit =
+ window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ if (!model || !view || !queryEdit)
+ return false;
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+
+ bool ready = false;
+ for (int attempt = 0; attempt < 150 && !ready; ++attempt) {
+ ready = model->rowCount(QModelIndex()) == 1
+ && !window.mailRootForTesting().isEmpty();
+ if (!ready)
+ QTest::qWait(100);
+ }
+ if (!ready)
+ return false;
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ return true;
+ }
+};
+
+} // namespace
+
+void TestMainWindow::theMailRootComesFromTheConfigNotTheIndex()
+{
+ // Item 124's rule, for the path the composer composes drafts and sent
+ // copies under. splitIndex() is what makes this test able to fail at all:
+ // in the ordinary layout notmuch_database_get_path() and
+ // NOTMUCH_CONFIG_MAIL_ROOT return the SAME string, so a test written
+ // against it passes whichever accessor the code uses.
+ WorkerComposeFixture fixture;
+ fixture.backed.fixture().splitIndex();
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+
+ // The MAIL root, not the index directory. Under the split layout these are
+ // different directories, and a draft composed under the index one is
+ // written into the Xapian tree.
+ QCOMPARE(window.mailRootForTesting(),
+ QDir(fixture.backed.fixture().maildirPath()).absolutePath());
+ QVERIFY2(window.mailRootForTesting()
+ != QDir(fixture.backed.fixture().indexPath()).absolutePath(),
+ "the window took the index directory for the mail root");
+}
+
+void TestMainWindow::replyIsDisabledOnAReceiveOnlyAccountsMail()
+{
+ // The capability IS the send_command's presence, so this account is
+ // written without one.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("listsonly"),
+ QStringLiteral("listsonly"), QString(),
+ /*sendCommand=*/QString(),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("listsonly/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QVERIFY(WorkerComposeFixture::selectTheMessage(window));
+
+ for (const QString &name : { QStringLiteral("reply"),
+ QStringLiteral("reply_all"),
+ QStringLiteral("reply_no_quote"),
+ QStringLiteral("forward") }) {
+ auto *action = window.findChild<QAction *>(name);
+ QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name)));
+ QVERIFY2(!action->isEnabled(),
+ qPrintable(QStringLiteral("%1 was live on receive-only mail")
+ .arg(name)));
+ }
+
+ // save_message is NEVER disabled, including here. It is the escape hatch
+ // for exactly this case: write the raw message out and attach it to a new
+ // message from an account that can send.
+ auto *save = window.findChild<QAction *>(QStringLiteral("save_message"));
+ QVERIFY(save);
+ QVERIFY2(save->isEnabled(),
+ "save_message was disabled, removing the escape hatch");
+}
+
+void TestMainWindow::replyIsEnabledOnASendingAccountsMail()
+{
+ // The guard for the test above. Without it, a bug disabling the reply
+ // family unconditionally would pass every assertion there while removing
+ // the feature entirely.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QVERIFY(WorkerComposeFixture::selectTheMessage(window));
+
+ for (const QString &name : { QStringLiteral("reply"),
+ QStringLiteral("reply_all"),
+ QStringLiteral("reply_no_quote"),
+ QStringLiteral("forward") }) {
+ auto *action = window.findChild<QAction *>(name);
+ QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name)));
+ QVERIFY2(action->isEnabled(),
+ qPrintable(QStringLiteral("%1 was disabled on mail from an "
+ "account that can send").arg(name)));
+ }
+
+ // And no ribbon: this account can send, so there is nothing to explain.
+ auto *ribbon =
+ window.findChild<QLabel *>(QStringLiteral("receiveOnlyRibbon"));
+ QVERIFY(ribbon);
+ QVERIFY2(ribbon->isHidden(),
+ "the receive-only ribbon showed on an account that can send");
+}
+
+void TestMainWindow::theReceiveOnlyRibbonNamesTheAccount()
+{
+ // The ribbon is a WIDGET in MessageView's layout, not markup inside the
+ // web view. Composing HTML from configuration into the one document that
+ // renders input from strangers is the wrong direction.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("listsonly"),
+ QStringLiteral("listsonly"), QString(),
+ QString(), QStringLiteral("you@example.org") } },
+ QStringLiteral("listsonly/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QVERIFY(WorkerComposeFixture::selectTheMessage(window));
+
+ auto *ribbon =
+ window.findChild<QLabel *>(QStringLiteral("receiveOnlyRibbon"));
+ QVERIFY2(ribbon, "no ribbon widget exists");
+
+ // isHidden() rather than isVisibleTo(): under the offscreen platform an
+ // unshown window's children report not visible whatever the code does, so
+ // isVisibleTo would fail against correct code. What is being asserted is
+ // that the ribbon was not left explicitly hidden.
+ QVERIFY2(!ribbon->isHidden(),
+ "the ribbon did not appear on receive-only mail");
+ QVERIFY2(ribbon->text().contains(QStringLiteral("listsonly")),
+ qPrintable(QStringLiteral("the ribbon does not name the account: %1")
+ .arg(ribbon->text())));
+
+ // PlainText, not AutoText. A QLabel guesses under AutoText, and this is
+ // the same protection MessageDetailsDialog states on every value.
+ QCOMPARE(ribbon->textFormat(), Qt::PlainText);
+}
+
+void TestMainWindow::composeIsDisabledOnlyWhenNoAccountCanSend()
+{
+ // An installation with no send_command anywhere is a valid read-only
+ // installation and is not warned about; compose is simply unavailable.
+ {
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("listsonly"),
+ QStringLiteral("listsonly"), QString(),
+ QString(), QStringLiteral("you@example.org") } },
+ QStringLiteral("listsonly/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ auto *compose = window.findChild<QAction *>(QStringLiteral("compose"));
+ QVERIFY(compose);
+ QVERIFY2(!compose->isEnabled(),
+ "compose was live with no account able to send");
+ }
+ {
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed(
+ { { QStringLiteral("listsonly"),
+ QStringLiteral("listsonly"), QString(), QString(),
+ QStringLiteral("you@example.org") },
+ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("work@example.org") } },
+ QStringLiteral("listsonly/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ auto *compose = window.findChild<QAction *>(QStringLiteral("compose"));
+ QVERIFY(compose);
+ QVERIFY2(compose->isEnabled(),
+ "compose was disabled although one account can send");
+ }
+}
+
+void TestMainWindow::quittingWithACleanComposerAsksNothing()
+{
+ // Case 1: every composer clean, quit directly, no dialog. A dialog here
+ // would be the "are you sure" this project deliberately does not do.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+
+ QVERIFY2(window.openComposerForTest(), "no composer opened");
+ QCOMPARE(window.openComposerCount(), 1);
+
+ QVERIFY2(window.composersBlockingQuit().isEmpty(),
+ "a clean composer was reported as blocking quit");
+
+ // Composers are parentless top-level windows and outlive this MainWindow,
+ // carrying a MessageSender and a running autosave timer into whatever test
+ // runs next. Closed here rather than left for the destructor, which never
+ // touches m_composers.
+ for (ComposeWindow *composer : window.openComposersForTest()) {
+ composer->show();
+ composer->close();
+ }
+}
+
+void TestMainWindow::quittingWithUnsavedEditsReportsEveryComposer()
+{
+ // Case 2: ONE dialog whatever the count, so the quit path has to see BOTH
+ // composers rather than stopping at the first dirty one.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+
+ QVERIFY(window.openComposerForTest());
+ QVERIFY(window.openComposerForTest());
+ QCOMPARE(window.openComposerCount(), 2);
+
+ // Clean until something is typed, which is the case-1 assertion holding
+ // here too and the guard that this test can distinguish the two states.
+ QVERIFY(window.composersBlockingQuit().isEmpty());
+
+ window.markComposersDirtyForTest();
+ QCOMPARE(window.composersBlockingQuit().size(), 2);
+
+ // Left open, these are parentless top-level windows with a live autosave
+ // timer, surviving into later tests. See the note in the clean-composer
+ // case above.
+ for (ComposeWindow *composer : window.openComposersForTest()) {
+ composer->show();
+ composer->close();
+ }
+}
+
+void TestMainWindow::closingAComposerCompactsTheRegistry()
+{
+ // The closed() signal's ONE job. The QPointer alone would keep
+ // composersBlockingQuit() correct, since it nulls on destruction, but the
+ // entry would stay in the list for the session's lifetime. This asserts
+ // the list is compacted, which only the signal can do.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+
+ ComposeWindow *composer = window.openComposerForTest();
+ QVERIFY(composer);
+ QCOMPARE(window.openComposerCount(), 1);
+
+ // A composer that was never shown returns early from close() WITHOUT
+ // reaching closeEvent(), so the signal would never fire and this test
+ // would assert nothing at all.
+ composer->show();
+ QVERIFY(composer->close());
+
+ // And the quit path must not see a destroyed window, which is the
+ // QPointer's job rather than the signal's.
+ QCOMPARE(window.openComposerCount(), 0);
+ QVERIFY(window.composersBlockingQuit().isEmpty());
+}
+
+void TestMainWindow::savingAMessageRefusesToEscapeTheChosenDirectory()
+{
+ // A subject is input from a stranger and is what the default filename is
+ // derived from, so it may carry separators and "..". Asserted through
+ // Attachment's own helpers, which is what saveDisplayedMessage() calls:
+ // a second implementation of the check here would prove nothing about the
+ // one that runs.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const QString directory = dir.path();
+
+ Attachment naming;
+ naming.filename = QStringLiteral("../../etc/passwd");
+ const QString target =
+ QDir(directory).absoluteFilePath(naming.safeFilename());
+
+ QVERIFY2(Attachment::isPathInsideDirectory(directory, target),
+ "a traversing subject escaped the chosen directory");
+ QVERIFY2(!target.contains(QStringLiteral("/etc/passwd")),
+ qPrintable(QStringLiteral("the traversal survived: %1").arg(target)));
+
+ // Compared as PATHS, never with startsWith(): a sibling directory whose
+ // name merely begins with the chosen one's is not inside it.
+ QVERIFY2(!Attachment::isPathInsideDirectory(
+ directory, directory + QStringLiteral("-evil/message.eml")),
+ "a sibling directory passed the containment check");
+}
+
+void TestMainWindow::aHostileSubjectCannotEscapeTheSaveDirectory()
+{
+ // Asserted through MainWindow::defaultMessageFilename(), which is what
+ // saveDisplayedMessage() actually calls. The previous version of this
+ // check built an Attachment by hand and called safeFilename() directly:
+ // that proves what Attachment does and nothing about whether save_message
+ // asks it anything, and three mutations to the real path left it green.
+ // CLAUDE.md: assert through the function the production path calls, not
+ // through the one it calls INTO.
+ const QString traversal =
+ MainWindow::defaultMessageFilename(QStringLiteral("../../etc/passwd"));
+
+ // No separator survives, so the name cannot address another directory.
+ QVERIFY2(!traversal.contains(QLatin1Char('/')),
+ qPrintable(QStringLiteral("a separator survived: %1").arg(traversal)));
+ // NOT asserting the absence of "..": with every separator replaced, a
+ // literal ".." inside a filename addresses nothing and is a legitimate
+ // part of a name. What matters is that the result is a single path
+ // COMPONENT, which is what makes traversal impossible.
+ QCOMPARE(QFileInfo(traversal).fileName(), traversal);
+ QVERIFY2(traversal != QStringLiteral("..")
+ && traversal != QStringLiteral("."),
+ qPrintable(QStringLiteral("the name is a directory reference: %1")
+ .arg(traversal)));
+
+ // And joining it onto a directory really does stay inside.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ Attachment naming;
+ naming.filename = traversal;
+ const QString target =
+ QDir(dir.path()).absoluteFilePath(naming.safeFilename());
+ QVERIFY2(Attachment::isPathInsideDirectory(dir.path(), target),
+ qPrintable(QStringLiteral("escaped the directory: %1").arg(target)));
+
+ // A backslash is a separator too, on a name written by Windows software.
+ const QString backslash = MainWindow::defaultMessageFilename(
+ QStringLiteral("..\\..\\Windows\\System32\\config"));
+ QVERIFY2(!backslash.contains(QLatin1Char('\\')),
+ qPrintable(QStringLiteral("a backslash survived: %1").arg(backslash)));
+
+ // A subject with nothing usable still yields a name rather than "" or a
+ // bare extension, which would make the write land on a dotfile.
+ const QString empty = MainWindow::defaultMessageFilename(QString());
+ QVERIFY2(empty.startsWith(QStringLiteral("message")),
+ qPrintable(QStringLiteral("empty subject gave: %1").arg(empty)));
+
+ // The extension survives truncation. Truncating AFTER appending it would
+ // cut ".eml" off a long subject and write an extensionless file.
+ const QString long_ = MainWindow::defaultMessageFilename(
+ QString(400, QLatin1Char('a')));
+ QVERIFY2(long_.endsWith(QStringLiteral(".eml")),
+ qPrintable(QStringLiteral("the extension was truncated away: %1")
+ .arg(long_.right(20))));
+}
+
+void TestMainWindow::savingTwiceDoesNotOverwriteTheFirstFile()
+{
+ // Two messages very often share a subject, and the filename is derived
+ // from it, so the second save must not destroy the first. Driven through
+ // saveDisplayedMessage() by way of the directory seam, which is the only
+ // way to reach the write guard at all: the file dialog is a modal the
+ // offscreen platform cannot click.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QVERIFY(WorkerComposeFixture::selectTheMessage(window));
+
+ QTemporaryDir out;
+ QVERIFY(out.isValid());
+
+ window.saveDisplayedMessageForTest(out.path());
+ window.saveDisplayedMessageForTest(out.path());
+
+ // Two files, not one overwritten. Asserted on the COUNT rather than on the
+ // second name, so the disambiguation scheme can change without the test
+ // caring what it is called.
+ const QStringList written =
+ QDir(out.path()).entryList(QDir::Files | QDir::NoDotAndDotDot);
+ QCOMPARE(written.size(), 2);
+
+ // And both are real copies rather than one empty placeholder.
+ for (const QString &name : written) {
+ QVERIFY2(QFileInfo(QDir(out.path()).absoluteFilePath(name)).size() > 0,
+ qPrintable(QStringLiteral("%1 is empty").arg(name)));
+ }
+}
+
+void TestMainWindow::savingAMessageWithAHostileSubjectStaysInTheDirectory()
+{
+ // Driven through saveDisplayedMessage() with a real hostile subject, which
+ // is the only shape that covers the production write path. An earlier
+ // version of this coverage built an Attachment by hand and called
+ // safeFilename() and isPathInsideDirectory() directly, which proves what
+ // Attachment does and nothing about whether save_message asks it anything.
+ //
+ // WHAT THIS CAN AND CANNOT CATCH, measured rather than assumed, because
+ // the numbers are surprising and the next person will otherwise redo the
+ // work. Three independent layers stand between a subject and the write:
+ // defaultMessageFilename() replaces separators, Attachment::safeFilename()
+ // reduces to a basename, and Attachment::isPathInsideDirectory() refuses
+ // the write. EACH ONE ALONE IS SUFFICIENT, so removing any single layer
+ // leaves this test green: measured, all three single-layer mutations pass.
+ // Removing all three fails it. That is real defence-in-depth rather than a
+ // probe pointed at the wrong object, and mimeparser.h:71-77 already says
+ // the same of isPathInsideDirectory, but it does mean this test is a guard
+ // against the DEFENCES COLLECTIVELY disappearing, not a guard on any one
+ // of them. aHostileSubjectCannotEscapeTheSaveDirectory() covers the first
+ // layer on its own, and a single-layer mutation there does fail.
+ //
+ // The subject is ABSOLUTE rather than "../..", and that matters.
+ // QDir::absoluteFilePath() does not resolve ".." (measured: it
+ // concatenates), but the collision loop below can rename a relative
+ // traversal by accident when the target happens to exist, which makes it
+ // the weaker probe. An absolute candidate replaces the directory outright.
+ WorkerComposeFixture fixture;
+ QVERIFY(fixture.backed.fixture().addMessage(
+ QStringLiteral("work/inbox"), QStringLiteral("hostile@example.org"),
+ // The subject is the attacker's input, and it is what the default
+ // filename is derived from.
+ // Absolute, not "../..". QDir::absoluteFilePath() does NOT resolve
+ // ".." (measured: it concatenates, giving "<dir>/../../x"), but an
+ // ABSOLUTE candidate replaces the directory outright, which is the
+ // escape that survives every accident. A relative traversal can be
+ // neutralised by the collision loop renaming it when the target
+ // happens to exist, so it is the weaker probe of the two.
+ QStringLiteral("/tmp/qtmaildir-pwned-probe"),
+ QStringLiteral("sender@example.org"),
+ // Friday, verified with `date -d 2026-08-14 +%A`.
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY2(fixture.backed.buildWithAccounts(
+ { { QStringLiteral("work"), QStringLiteral("work"), QString(),
+ QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } }),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QVERIFY(WorkerComposeFixture::selectTheMessage(window));
+
+ // A directory INSIDE another, so an escape has somewhere to land that the
+ // test can then look at. Escaping "out" writes into parent/, which is what
+ // the assertions below check is still empty.
+ QTemporaryDir parent;
+ QVERIFY(parent.isValid());
+ const QString out = parent.filePath(QStringLiteral("out"));
+ QVERIFY(QDir().mkpath(out));
+
+ window.saveDisplayedMessageForTest(out);
+
+ // The file landed inside the chosen directory.
+ // NOT QDir::Hidden. A file whose name begins with a dot is hidden on every
+ // Unix desktop, so the write would succeed while the user could not find
+ // what they saved. Listing without Hidden is what makes this assertion
+ // notice that, and it is how the leading-dot case was found: a traversing
+ // subject reduces to "..-..-etc-passwd" once its separators are replaced,
+ // which is a dotfile.
+ const QStringList inside =
+ QDir(out).entryList(QDir::Files | QDir::NoDotAndDotDot);
+ QCOMPARE(inside.size(), 1);
+ QVERIFY2(!inside.first().startsWith(QLatin1Char('.')),
+ qPrintable(QStringLiteral("the saved message is hidden: %1")
+ .arg(inside.first())));
+
+ // And nothing was written beside it, which is where a traversal would go.
+ const QStringList escaped =
+ QDir(parent.path()).entryList(QDir::Files | QDir::NoDotAndDotDot);
+ QVERIFY2(escaped.isEmpty(),
+ qPrintable(QStringLiteral("a file escaped the directory: %1")
+ .arg(escaped.join(QLatin1Char(' ')))));
+
+ // The written path really is contained, compared as PATHS rather than with
+ // startsWith(): a sibling directory whose name merely begins with the
+ // chosen one's is not inside it.
+ const QString written = QDir(out).absoluteFilePath(inside.first());
+ QVERIFY2(Attachment::isPathInsideDirectory(out, written),
+ qPrintable(QStringLiteral("escaped: %1").arg(written)));
+ QVERIFY2(QFileInfo(written).size() > 0, "the saved message is empty");
+}
+
+void TestMainWindow::aStuckComposeRequestDoesNotHijackTheNextPaneLoad()
+{
+ // A compose request for a message that is not in the index used to stay
+ // armed for ever, because it was cleared only on the branch that FOUND the
+ // id. The delayed symptom is the bad one: the pane's own loads are the
+ // traffic being matched against, so merely selecting that message later
+ // matched, opened a composer nobody asked for, and returned before
+ // renderMessages() leaving the pane blank on the row just clicked.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QVERIFY(WorkerComposeFixture::selectTheMessage(window));
+
+ // Arm a request for an id the database does not hold. loadMessage() emits
+ // an empty result for it, which is what must disarm the request.
+ window.requestMessageForComposeForTest(
+ QStringLiteral("nosuchmessage@example.org"),
+ ComposeContext::Kind::Reply, true);
+
+ // No composer, and the request stops being armed.
+ QTRY_VERIFY_WITH_TIMEOUT(!window.composeRequestPendingForTest(), 15000);
+ QCOMPARE(window.openComposerCount(), 0);
+
+ // Now the delayed half. Select the real message: the pane must render it,
+ // and no composer may appear. With the request still armed this failed
+ // only if the ids matched, so the request is re-armed for the REAL id to
+ // make the hijack reachable at all.
+ window.requestMessageForComposeForTest(
+ QStringLiteral("compose1@example.org"), ComposeContext::Kind::Reply,
+ true);
+ QTRY_VERIFY_WITH_TIMEOUT(!window.composeRequestPendingForTest(), 15000);
+
+ // That one DID match, so it opened a composer. Close it and clear the
+ // pane, then re-select and assert the pane renders rather than a second
+ // composer opening.
+ for (ComposeWindow *composer : window.openComposersForTest()) {
+ composer->show();
+ composer->close();
+ }
+ QCOMPARE(window.openComposerCount(), 0);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(model && view);
+ view->setCurrentIndex(QModelIndex());
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+
+ auto *pane = window.findChild<MessageView *>();
+ QVERIFY(pane);
+ QTRY_VERIFY_WITH_TIMEOUT(!pane->showingPlaceholder(), 15000);
+ QCOMPARE(window.openComposerCount(), 0);
+}
+
+void TestMainWindow::theSaveLoopToleratesAComposerClosedUnderTheDialog()
+{
+ // The regression for a measured use-after-free. composersBlockingQuit()
+ // used to return raw pointers, and the quit path held that list across
+ // QMessageBox::exec(). A nested event loop PROCESSES deleteLater(),
+ // verified in a standalone Qt program: a parentless WA_DeleteOnClose
+ // window closed while a modal is up is destroyed BEFORE exec() returns.
+ // The dialog is window-modal to the main window only, so a user really can
+ // close a composer from under it, and Save then ran on freed memory.
+ //
+ // The modal itself cannot be driven under the offscreen platform, so what
+ // is asserted is the property that makes the loop safe: the list holds
+ // QPointers, and an entry whose window is destroyed reads as null rather
+ // than as a dangling pointer. That is exactly what the null check in the
+ // Save loop consumes. Stated plainly because it is NOT full coverage of
+ // closeEvent(): see the report.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+
+ QVERIFY(window.openComposerForTest());
+ QVERIFY(window.openComposerForTest());
+ window.markComposersDirtyForTest();
+
+ QList<QPointer<ComposeWindow>> blocking = window.composersBlockingQuit();
+ QCOMPARE(blocking.size(), 2);
+
+ // Destroy one exactly as closing it under the dialog would, including the
+ // deleteLater() a nested exec() would process.
+ ComposeWindow *doomed = blocking.first().data();
+ QVERIFY(doomed);
+ doomed->show();
+ doomed->close();
+ QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
+
+ // The held list reports it as gone rather than handing back a dangling
+ // pointer. A raw QList<ComposeWindow *> could not express this at all.
+ QVERIFY2(blocking.first().isNull(),
+ "the held entry did not null when its window was destroyed");
+ QVERIFY2(!blocking.last().isNull(),
+ "the surviving composer was lost too");
+
+ // And the loop the quit path runs skips the null and still saves the
+ // survivor, which is the behaviour the crash destroyed: the remaining
+ // drafts were never written because the crash happened mid-loop.
+ int saved = 0;
+ for (const QPointer<ComposeWindow> &composer : blocking) {
+ if (composer) {
+ composer->saveDraftNow();
+ ++saved;
+ }
+ }
+ QCOMPARE(saved, 1);
+}
+
+namespace {
+
+/// Writes a multipart/mixed message with one named attachment part.
+///
+/// Hand-written rather than built with MessageBuilder: this is the INPUT to
+/// the forward path, and generating it with the same library that consumes it
+/// would let an encoding mistake agree with itself.
+bool writeMessageWithAttachment(const QString &path, const QString &attachName,
+ const QByteArray &attachBody)
+{
+ QFile file(path);
+ if (!file.open(QIODevice::WriteOnly))
+ return false;
+ QByteArray raw =
+ "From: sender@example.org\n"
+ "To: you@example.org\n"
+ "Subject: Quarterly report\n"
+ "Message-ID: <fwd-1@example.org>\n"
+ // Friday, verified with `date -d 2026-08-14 +%A`. Qt::RFC2822Date
+ // validates the weekday against the date.
+ "Date: Fri, 14 Aug 2026 10:00:00 +0200\n"
+ "MIME-Version: 1.0\n"
+ "Content-Type: multipart/mixed; boundary=\"MIX\"\n"
+ "\n"
+ "--MIX\n"
+ "Content-Type: text/plain; charset=utf-8\n"
+ "\n"
+ "See the attached document.\n"
+ "--MIX\n"
+ "Content-Type: application/octet-stream; name=\"" + attachName.toUtf8() + "\"\n"
+ "Content-Disposition: attachment; filename=\"" + attachName.toUtf8() + "\"\n"
+ "\n" + attachBody + "\n"
+ "--MIX--\n";
+ file.write(raw);
+ file.close();
+ return true;
+}
+
+/// Writes a multipart/alternative message that DOES carry a text/html part.
+bool writeHtmlMessage(const QString &path)
+{
+ QFile file(path);
+ if (!file.open(QIODevice::WriteOnly))
+ return false;
+ file.write(
+ "From: sender@example.org\n"
+ "To: you@example.org\n"
+ "Subject: Has HTML\n"
+ "Message-ID: <html-1@example.org>\n"
+ "Date: Fri, 14 Aug 2026 10:00:00 +0200\n"
+ "MIME-Version: 1.0\n"
+ "Content-Type: multipart/alternative; boundary=\"ALT\"\n"
+ "\n"
+ "--ALT\n"
+ "Content-Type: text/plain; charset=utf-8\n"
+ "\n"
+ "plain\n"
+ "--ALT\n"
+ "Content-Type: text/html; charset=utf-8\n"
+ "\n"
+ "<p>html</p>\n"
+ "--ALT--\n");
+ file.close();
+ return true;
+}
+
+} // namespace
+
+void TestMainWindow::forwardingCarriesTheOriginalsAttachments()
+{
+ // The spec requires Forward to carry attachments, twice. The context field
+ // existed and was never assigned, so a Forward opened with an empty
+ // attachment list: the composer looked entirely correct, and the recipient
+ // received a body quoting a document that was not attached, with nothing
+ // erroring anywhere.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const QString original = dir.filePath(QStringLiteral("original.eml"));
+ QVERIFY(writeMessageWithAttachment(original, QStringLiteral("report.pdf"),
+ QByteArray("PDFBYTES")));
+
+ QTemporaryDir confDir;
+ QVERIFY(confDir.isValid());
+ const QString confPath = confDir.filePath(QStringLiteral("qtmaildir.conf"));
+ {
+ QSettings settings(confPath, QSettings::IniFormat);
+ settings.beginGroup(QStringLiteral("account.work"));
+ settings.setValue(QStringLiteral("maildir"), QStringLiteral("work"));
+ settings.setValue(QStringLiteral("address"),
+ QStringLiteral("you@example.org"));
+ settings.setValue(QStringLiteral("send_command"),
+ QStringLiteral("/bin/true"));
+ settings.endGroup();
+ settings.sync();
+ }
+ Config config;
+ config.load(confPath);
+
+ ComposeContext context;
+ context.kind = ComposeContext::Kind::Forward;
+ context.accountKey = QStringLiteral("work");
+ context.originalPath = original;
+ context.subject = QStringLiteral("Fwd: Quarterly report");
+
+ ComposeWindow composer(context, config, dir.path());
+
+ // The attachment is present, and it is a REAL FILE on disk rather than a
+ // remembered name: MessageBuilder reads every attachment by path at build
+ // time and refuses a build naming one that does not exist.
+ const QStringList attached = composer.attachments();
+ QCOMPARE(attached.size(), 1);
+ QVERIFY2(QFileInfo::exists(attached.first()),
+ qPrintable(QStringLiteral("the extracted path does not exist: %1")
+ .arg(attached.first())));
+ QCOMPARE(QFileInfo(attached.first()).fileName(),
+ QStringLiteral("report.pdf"));
+
+ // And the bytes are the original's, not an empty placeholder.
+ QFile written(attached.first());
+ QVERIFY(written.open(QIODevice::ReadOnly));
+ QCOMPARE(written.readAll(), QByteArray("PDFBYTES"));
+ written.close();
+
+ // A Reply to the same message carries NOTHING. The spec says attachments
+ // are carried "for Forward, empty otherwise", and a reply that re-attached
+ // the original's documents would send them back to their own sender.
+ ComposeContext replyContext = context;
+ replyContext.kind = ComposeContext::Kind::Reply;
+ ComposeWindow replyComposer(replyContext, config, dir.path());
+ QVERIFY2(replyComposer.attachments().isEmpty(),
+ "a reply carried the original's attachments");
+}
+
+void TestMainWindow::forwardSeedsHtmlFromTheConfigNotTheOriginal()
+{
+ // MEASURED, and it revises what the spec review reported. Forward was
+ // NEVER seeding from the original: ComposeWindow::seedFields() already
+ // implements the split itself (composewindow.cpp, `isReply ?
+ // m_context.seedHtml : m_config.compose().sendHtml`), so the context's
+ // value is IGNORED for a forward and the config won regardless. The
+ // openComposerFor() line this test also covers was therefore cosmetic
+ // rather than a live defect: it stopped the context carrying a value that
+ // nothing read, which is worth doing but changed no behaviour.
+ //
+ // The consequence for this test: EITHER layer alone enforces the rule, so
+ // neither single-layer mutation fails it, and only mutating both does.
+ // Verified in both directions rather than assumed.
+ //
+ // The spec splits these: New and Forward seed from [compose] send_html,
+ // Reply and Reply-all from whether the original carried a text/html part.
+ // An HTML part in the original is a fact about the SENDER's software, so
+ // it is the right seed when answering them and says nothing about a
+ // forward, which is a new message to somebody else.
+ //
+ // Asserted on the CONTEXT the window is built from rather than through the
+ // checkbox, because what is under test is which source the value comes
+ // from. The two sources must DISAGREE or the test passes either way: the
+ // config says false while the original is plain text, so reading the
+ // original would give false as well. Hence send_html=true against a plain
+ // original: config true, original false.
+ // The two sources must DISAGREE or the test passes whichever one is read,
+ // and getting that wrong is why an earlier version of this survived every
+ // mutation: config send_html=FALSE against an original that DOES carry a
+ // text/html part. Reading the original gives true, reading the config
+ // gives false, so the assertion below can only be satisfied one way.
+ WorkerComposeFixture fixture;
+ QVERIFY2(fixture.seed({ { QStringLiteral("work"), QStringLiteral("work"),
+ QString(), QStringLiteral("/bin/true"),
+ QStringLiteral("you@example.org") } },
+ QStringLiteral("work/inbox"),
+ QStringLiteral("send_html=false")),
+ qPrintable(fixture.backed.error()));
+
+ MainWindow window(fixture.backed.config());
+ QTRY_VERIFY_WITH_TIMEOUT(!window.mailRootForTesting().isEmpty(), 15000);
+ QCOMPARE(fixture.backed.config().compose().sendHtml, false);
+
+ // The original lives inside the account's maildir so accountForReply()
+ // can resolve it; its CONTENT is what matters, not that notmuch indexed it.
+ const QString original =
+ QDir(window.mailRootForTesting())
+ .absoluteFilePath(QStringLiteral("work/inbox/cur/fwd-original"));
+ QVERIFY(writeHtmlMessage(original));
+
+ MimeParser parser;
+ const ParsedMessage parsed = parser.parse(original);
+ QVERIFY(parsed.ok);
+ QCOMPARE(parsed.hasHtml(), true);
+
+ // Through openComposerFor(), which is the production line that chooses
+ // the source. Building the context by hand here and asserting on the
+ // checkbox proved only that ComposeWindow honours what it is given: the
+ // mutation putting `original.hasHtml()` back stayed green, because the
+ // test was setting seedHtml itself.
+ MessageRef ref;
+ ref.messageId = QStringLiteral("html-1@example.org");
+ ref.filePath = original;
+ ref.matched = true;
+
+ window.openComposerForTest(ref, ComposeContext::Kind::Forward, true);
+
+ QList<ComposeWindow *> opened = window.openComposersForTest();
+ QCOMPARE(opened.size(), 1);
+ auto *sendHtml =
+ opened.first()->findChild<QCheckBox *>(QStringLiteral("sendHtml"));
+ QVERIFY(sendHtml);
+ QVERIFY2(!sendHtml->isChecked(),
+ "Forward seeded sendHtml from the original's HTML part rather "
+ "than from [compose] send_html");
+
+ // The counterpart, and it is what stops this asserting "always false":
+ // a REPLY to the same message seeds from the original, so it is checked
+ // where the forward is not. Without this half, disabling the checkbox
+ // outright would pass.
+ window.openComposerForTest(ref, ComposeContext::Kind::Reply, true);
+ const QList<ComposeWindow *> both = window.openComposersForTest();
+ QCOMPARE(both.size(), 2);
+ auto *replyHtml =
+ both.last()->findChild<QCheckBox *>(QStringLiteral("sendHtml"));
+ QVERIFY(replyHtml);
+ QVERIFY2(replyHtml->isChecked(),
+ "Reply did not seed sendHtml from the original's HTML part");
+
+ for (ComposeWindow *composer : both) {
+ composer->show();
+ composer->close();
+ }
+}
+
void TestMainWindow::aStartupAccountScopesTheStartupQuery()
{
// "Start me in Work - Inbox rather than All accounts - Inbox." The account