diff options
Diffstat (limited to 'tests/test_messagebuilder.cpp')
| -rw-r--r-- | tests/test_messagebuilder.cpp | 466 |
1 files changed, 466 insertions, 0 deletions
diff --git a/tests/test_messagebuilder.cpp b/tests/test_messagebuilder.cpp new file mode 100644 index 0000000..73d388c --- /dev/null +++ b/tests/test_messagebuilder.cpp @@ -0,0 +1,466 @@ +/* + * 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 <QDir> +#include <QFile> +#include <QThread> +#include <QObject> +#include <QRegularExpression> +#include <QTemporaryDir> +#include <QTest> + +#include <atomic> +#include <memory> + +#include "config.h" +#include "messagebuilder.h" +#include "types.h" + +/// MessageBuilder's tests assert on the GENERATED BYTES, never by round-tripping +/// through MimeParser. A builder and a parser that agree can be wrong together: +/// both are ours, and a shared misunderstanding of a charset or a part order +/// would show as a green suite and as mojibake on the recipient's screen. +class TestMessageBuilder : public QObject +{ + Q_OBJECT + +private slots: + void initTestCase(); + + void plainOnlyWhenSendHtmlIsOff(); + void multipartAlternativeWhenSendHtmlIsOn(); + void thePlainPartCarriesTheMarkdownSourceUnmodified(); + void theHtmlPartIsRenderedFromTheSameSource(); + void anAccentedBodyIsUtf8QuotedPrintable(); + void anAccentedSubjectIsRfc2047Utf8(); + void inReplyToAndReferencesAreCarried(); + void bareMessageIdsAreBracketedRatherThanEmittedEmpty(); + void attachmentsProduceMultipartMixed(); + void aMissingAttachmentFailsTheBuild(); + void aDirectoryAttachmentFailsRatherThanHangingTheProcess(); + void anUnparseableRecipientFailsRatherThanVanishing(); + void everyMessageCarriesADateAndMessageId(); + void recipientsAppearInTheirOwnHeaders(); + void anAccountWithNoAddressFailsRatherThanBuildingHeaderlessMail(); + +private: + Account m_account; + + /// A message with the fixture account and one recipient, so each test can + /// change only the field it is about. + OutgoingMessage baseMessage() const + { + OutgoingMessage m; + m.accountKey = m_account.key; + m.to = QStringList{QStringLiteral("someone@example.org")}; + m.subject = QStringLiteral("A subject"); + m.markdownBody = QStringLiteral("Hello there."); + return m; + } +}; + +void TestMessageBuilder::initTestCase() +{ + m_account.key = QStringLiteral("work"); + m_account.name = QStringLiteral("Danilo M."); + m_account.address = QStringLiteral("user@example.org"); + m_account.maildir = QStringLiteral("work"); + m_account.sendCommand = QStringLiteral("/bin/true"); +} + +/// With the HTML toggle off the message must be a single text/plain part. +/// A multipart/alternative carrying one alternative is not merely wasteful: it +/// makes every message an attachment-bearing shape to some clients, and the +/// toggle exists precisely so a user can send mail nothing has to negotiate. +void TestMessageBuilder::plainOnlyWhenSendHtmlIsOff() +{ + OutgoingMessage m = baseMessage(); + m.sendHtml = false; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY(text.contains(QStringLiteral("Content-Type: text/plain"))); + QVERIFY(!text.contains(QStringLiteral("multipart/alternative"))); + QVERIFY(!text.contains(QStringLiteral("text/html"))); +} + +/// With the toggle on both parts must be present, and text/plain must come +/// FIRST. Order is load-bearing in multipart/alternative: a client renders the +/// LAST part it understands, so least-rich first. Reversed, every HTML-capable +/// client would show the markdown source and the rendered part would never be +/// seen by anyone. +void TestMessageBuilder::multipartAlternativeWhenSendHtmlIsOn() +{ + OutgoingMessage m = baseMessage(); + m.sendHtml = true; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY(text.contains(QStringLiteral("multipart/alternative"))); + + const int plain = text.indexOf(QStringLiteral("text/plain")); + const int html = text.indexOf(QStringLiteral("text/html")); + QVERIFY(plain >= 0); + QVERIFY(html >= 0); + QVERIFY2(plain < html, "text/plain must precede text/html in multipart/alternative"); +} + +/// The markdown SOURCE is the plain part, not a stripped-of-syntax rendering of +/// it. `**bold**` reads as emphasis to a human, and a plain-text renderer would +/// mean inventing a second renderer whose output could disagree with the HTML +/// one. The draft the user autosaves is this same text, which is the other +/// reason it must not be rewritten on the way out. +void TestMessageBuilder::thePlainPartCarriesTheMarkdownSourceUnmodified() +{ + OutgoingMessage m = baseMessage(); + m.sendHtml = true; + m.markdownBody = QStringLiteral("**bold** and - [ ] a task"); + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("**bold** and - [ ] a task")), + qPrintable(text)); +} + +/// The HTML part comes from the same source through MarkdownRenderer, so the +/// two parts can never describe different messages. +void TestMessageBuilder::theHtmlPartIsRenderedFromTheSameSource() +{ + OutgoingMessage m = baseMessage(); + m.sendHtml = true; + m.markdownBody = QStringLiteral("**bold**"); + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("<strong>bold</strong>")), qPrintable(text)); +} + +/// Measured 2026-08-20: g_mime_text_part_set_text() encodes with whatever +/// charset is set at the moment it is CALLED, so setting the charset afterwards +/// RELABELS the part without re-encoding it. That produces a part headed +/// charset=utf-8 whose bytes are latin-1 (`Perch=E9`), which looks correct in +/// every header and arrives as mojibake. Asserting on the label alone would +/// pass against exactly that bug, so this asserts on the BYTES too: =C3=A9 must +/// be there and =E9 must not. +void TestMessageBuilder::anAccentedBodyIsUtf8QuotedPrintable() +{ + OutgoingMessage m = baseMessage(); + m.markdownBody = QStringLiteral("perché è così"); + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("charset=utf-8"), Qt::CaseInsensitive), + qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("=C3=A9")), qPrintable(text)); + QVERIFY2(!text.contains(QStringLiteral("=E9\n")) && !text.contains(QStringLiteral("=E9 ")), + "latin-1 bytes under a utf-8 label"); +} + +/// Measured 2026-08-20: GMime encodes a header as iso-8859-1 unless told +/// otherwise, so g_mime_message_set_subject(msg, text, NULL) produced +/// =?iso-8859-1?B?...?=. The explicit "utf-8" argument is what makes an Italian +/// subject survive. +void TestMessageBuilder::anAccentedSubjectIsRfc2047Utf8() +{ + OutgoingMessage m = baseMessage(); + m.subject = QStringLiteral("Perché no"); + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("=?UTF-8?"), Qt::CaseInsensitive), qPrintable(text)); + QVERIFY2(!text.contains(QStringLiteral("=?iso-8859-1?"), Qt::CaseInsensitive), + qPrintable(text)); +} + +/// Not optional decoration. Without In-Reply-To and References a reply appears +/// as an orphan thread in the sender's own client, since the sent copy is +/// indexed by notmuch like any other message and notmuch threads on these +/// headers. +void TestMessageBuilder::inReplyToAndReferencesAreCarried() +{ + OutgoingMessage m = baseMessage(); + m.inReplyTo = QStringLiteral("<orig@example.org>"); + m.references = QStringList{QStringLiteral("<older@example.org>"), + QStringLiteral("<orig@example.org>")}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("In-Reply-To: <orig@example.org>")), qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("References:")), qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("<older@example.org>")), qPrintable(text)); +} + +/// **The brackets are syntax, and a bare id ships an EMPTY header rather than a +/// malformed one.** This is what every real caller supplies: GMime strips the +/// brackets when MimeParser reads Message-ID, and +/// ComposeContextBuilder::referencesForReply strips them from the References +/// chain so the two agree, so both values arrive here bare. +/// +/// Measured 2026-08-21: handed `orig@example.org`, GMime wrote `In-Reply-To:` +/// with no value at all and did not complain. Every reply would have arrived as +/// an orphan thread in the recipient's client, with nothing wrong to see +/// locally. Asserted on the FULL header line, since a test for the id alone +/// passes against an empty header that merely contains the name. +void TestMessageBuilder::bareMessageIdsAreBracketedRatherThanEmittedEmpty() +{ + OutgoingMessage m = baseMessage(); + m.inReplyTo = QStringLiteral("orig@example.org"); + m.references = QStringList{QStringLiteral("older@example.org"), + QStringLiteral("orig@example.org")}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("In-Reply-To: <orig@example.org>")), qPrintable(text)); + QVERIFY2(text.contains( + QStringLiteral("References: <older@example.org> <orig@example.org>")), + qPrintable(text)); + // The failure this exists for: the header present and empty. + QVERIFY2(!text.contains(QStringLiteral("In-Reply-To:\r\n")) + && !text.contains(QStringLiteral("In-Reply-To:\n")), + qPrintable(text)); +} + +/// The attachment wrapper must NEST the body, not sit beside it: multipart/mixed +/// outermost, with the multipart/alternative as its first part. Beside it, a +/// client would show the alternatives as attachments and the body would be +/// unreadable. Position in the byte stream is what distinguishes the two, so the +/// test asserts mixed appears BEFORE alternative. +void TestMessageBuilder::attachmentsProduceMultipartMixed() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("notes.txt")); + QFile f(path); + QVERIFY(f.open(QIODevice::WriteOnly)); + f.write("some attached bytes\n"); + f.close(); + + OutgoingMessage m = baseMessage(); + m.sendHtml = true; + m.attachments = QStringList{path}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + const int mixed = text.indexOf(QStringLiteral("multipart/mixed")); + const int alternative = text.indexOf(QStringLiteral("multipart/alternative")); + QVERIFY2(mixed >= 0, qPrintable(text)); + QVERIFY2(alternative >= 0, qPrintable(text)); + QVERIFY2(mixed < alternative, "multipart/mixed must wrap the body, not sit beside it"); + QVERIFY2(text.contains(QStringLiteral("notes.txt")), qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("Content-Disposition: attachment")), qPrintable(text)); +} + +/// A file can vanish between being attached and being sent, so existence is +/// checked at BUILD time. The build must produce NOTHING sendable: an empty +/// `bytes` is what stops a caller that only checks for content from shipping a +/// message missing the thing it was written to carry. +void TestMessageBuilder::aMissingAttachmentFailsTheBuild() +{ + OutgoingMessage m = baseMessage(); + m.attachments = QStringList{QStringLiteral("/nonexistent/path/to/report.pdf")}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY(!r.ok()); + QVERIFY(r.bytes.isEmpty()); + QVERIFY2(r.error.contains(QStringLiteral("report.pdf")), qPrintable(r.error)); +} + +/// A directory is not a file that can be attached, and accepting one does not +/// produce a bad message, it produces NO message ever: QFileInfo reports a +/// directory as existing and readable, opening one read-only is legal, and +/// GMime's base64 encoder then loops on a read() returning EISDIR without +/// advancing. Measured 2026-08-20 with strace at 2,169,821 failed reads in +/// twenty seconds and still going. build() runs synchronously from autosave on +/// the GUI thread, so this froze the whole application with the draft +/// unrecoverable. +/// +/// The TIMEOUT is deliberate and is the point of the test's shape. A regression +/// here hangs the binary rather than failing it, and CLAUDE.md already records +/// a hung test binary as a misleading failure mode that costs a session. The +/// build runs on a worker thread so this test can outlive it and report a +/// FAILURE instead of blocking ctest until its own timeout. +/// +/// Two details are what make that actually work, and the first draft of this +/// test had neither. It must NOT join the worker: a thread stuck in the defect +/// never returns, so a wait() after the timeout hangs exactly as the bug does +/// and the recorded failure is never printed. Verified by reverting the fix: +/// with the join the binary had to be killed at 150s with no verdict, without +/// it the run reports a FAIL and finishes. The worker is therefore deliberately +/// leaked on the failing path, which is correct for a test binary about to exit +/// and is the only way this reports rather than hangs. The result is read +/// through a shared_ptr for the same reason: a leaked thread must not write +/// into a stack frame that has returned. +void TestMessageBuilder::aDirectoryAttachmentFailsRatherThanHangingTheProcess() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString subdir = dir.filePath(QStringLiteral("a-folder")); + QVERIFY(QDir().mkpath(subdir)); + + // The guard this protects: a directory looks like a perfectly good + // attachment to the checks that were there before. + const QFileInfo info(subdir); + QVERIFY(info.exists()); + QVERIFY(info.isReadable()); + QVERIFY(!info.isFile()); + + OutgoingMessage m = baseMessage(); + m.attachments = QStringList{subdir}; + + // Shared with the worker rather than captured by reference, so a thread + // still spinning after this function returns cannot write into a dead + // frame. + struct Shared + { + std::atomic_bool finished{false}; + MessageBuilder::Result result; + }; + auto shared = std::make_shared<Shared>(); + const OutgoingMessage msg = m; + const Account account = m_account; + + QThread *worker = QThread::create([shared, msg, account] { + shared->result = MessageBuilder::build(msg, account); + shared->finished = true; + }); + worker->start(); + + // Five seconds against a defect measured at twenty seconds and unbounded. + // No join: see the note above, waiting on the stuck thread reproduces the + // hang instead of reporting it. + QTRY_VERIFY_WITH_TIMEOUT(shared->finished.load(), 5000); + if (!shared->finished.load()) + QFAIL("build() did not return for a directory attachment: it is looping on read()"); + + worker->wait(); + delete worker; + + QVERIFY(!shared->result.ok()); + QVERIFY(shared->result.bytes.isEmpty()); + QVERIFY2(shared->result.error.contains(QStringLiteral("a-folder")), + qPrintable(shared->result.error)); +} + +/// A recipient the builder cannot parse must STOP the send, never be dropped. +/// Measured 2026-08-20: internet_address_list_parse returns a ZERO-LENGTH list +/// rather than NULL for garbage, so a guard on the assembled list's length +/// built a message with no To: header at all and reported success. With +/// `msmtp -t` the recipients come FROM the headers, so that message reaches the +/// send command with nobody to deliver to, and the sent copy is filed in Sent +/// looking sent and having reached no one. +/// +/// Asserts on the error naming the offending entry, because with several +/// recipients the user cannot otherwise tell which one to fix. +void TestMessageBuilder::anUnparseableRecipientFailsRatherThanVanishing() +{ + OutgoingMessage m = baseMessage(); + m.to = QStringList{QStringLiteral("not an address at all ((("), + QStringLiteral("good@example.org")}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(!r.ok(), "an unparseable recipient must fail the build"); + QVERIFY(r.bytes.isEmpty()); + QVERIFY2(r.error.contains(QStringLiteral("not an address at all")), qPrintable(r.error)); + + // The other half of the same defect: with several recipients, the old code + // delivered the good ones and dropped the bad one without a word, so the + // user had no way to learn which recipient never received the message. A + // valid entry beside the bad one must not rescue the build. + QVERIFY2(!r.bytes.contains("good@example.org"), + "a valid recipient must not smuggle the message past a bad one"); +} + +/// Measured 2026-08-20: GMime generates neither header unless asked. A message +/// without a Message-ID cannot be threaded by anything that receives it, +/// including this application's own notmuch index once the sent copy lands. +void TestMessageBuilder::everyMessageCarriesADateAndMessageId() +{ + const MessageBuilder::Result r = MessageBuilder::build(baseMessage(), m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("Date: ")), qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("Message-Id: "), Qt::CaseInsensitive), qPrintable(text)); + QVERIFY(!r.messageId.isEmpty()); +} + +/// Bcc must be PRESENT in the bytes. The documented send command is `msmtp -t`, +/// which reads its recipients FROM the headers and strips Bcc itself before +/// transmission. Removing it here would mean blind recipients never receive the +/// message at all, silently. +/// +/// If a later change passes recipients as command arguments instead of relying +/// on -t, this test must change with it: under that scheme leaving Bcc in the +/// bytes discloses the blind recipients to everyone. +void TestMessageBuilder::recipientsAppearInTheirOwnHeaders() +{ + OutgoingMessage m = baseMessage(); + m.to = QStringList{QStringLiteral("to@example.org")}; + m.cc = QStringList{QStringLiteral("cc@example.org")}; + m.bcc = QStringList{QStringLiteral("bcc@example.org")}; + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + QVERIFY2(text.contains(QStringLiteral("From: ")), qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("user@example.org")), qPrintable(text)); + + const QRegularExpression to(QStringLiteral("^To:.*to@example\\.org"), + QRegularExpression::MultilineOption); + const QRegularExpression cc(QStringLiteral("^Cc:.*cc@example\\.org"), + QRegularExpression::MultilineOption); + const QRegularExpression bcc(QStringLiteral("^Bcc:.*bcc@example\\.org"), + QRegularExpression::MultilineOption); + QVERIFY2(to.match(text).hasMatch(), qPrintable(text)); + QVERIFY2(cc.match(text).hasMatch(), qPrintable(text)); + QVERIFY2(bcc.match(text).hasMatch(), qPrintable(text)); +} + +/// Config::account() returns a DEFAULT-CONSTRUCTED Account for an unknown key +/// rather than failing, so without this guard a bad key would build a message +/// with an empty From: silently malformed mail rather than a refusal, handed to +/// the send command as though it were fine. +void TestMessageBuilder::anAccountWithNoAddressFailsRatherThanBuildingHeaderlessMail() +{ + const Account empty; + const MessageBuilder::Result r = MessageBuilder::build(baseMessage(), empty); + QVERIFY(!r.ok()); + QVERIFY(r.bytes.isEmpty()); +} + +QTEST_MAIN(TestMessageBuilder) +#include "test_messagebuilder.moc" |
