aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-03 09:43:07 +0200
committerDanilo M. <danix@danix.xyz>2026-08-03 09:43:07 +0200
commit5de81471ebcac21dbf8c5d781cd1b5f1df931bb8 (patch)
tree785b57c48190602a521bed871ca9242a1f6d3fd9
parentcf17e6126c6487527a1fd1ff2c078e7637c92118 (diff)
downloadqtmaildir-5de81471ebcac21dbf8c5d781cd1b5f1df931bb8.tar.gz
qtmaildir-5de81471ebcac21dbf8c5d781cd1b5f1df931bb8.zip
fix: render the message pane at all
Clicking a thread left the pane blank. Two independent bugs, both from the same false premise: that setHtml() navigates to the base URL it is given. It does not. setHtml() navigates to a data: URL carrying the markup and applies the base URL afterwards, purely as the document's origin. Verified empirically on Qt 6.11. Built on that wrong assumption were: - MessagePage::acceptNavigationRequest compared the navigation's URL against documentUrl() and rejected everything else, so the document load was refused. It now accepts a typed main-frame navigation, which is one we initiated ourselves. - RequestInterceptor exempted exactly the qtmaildir: base URL and denied everything else, so the data: document load was blocked too. The interceptor fix is scoped to ResourceTypeMainFrame rather than allowing the data: scheme outright. A blanket allow would have been a real hole: a message body can write <img src="data:..."> or an iframe, and the existing dataSchemeBlocked test in test_interceptor.cpp was right to fail when that was tried. Sub-resource data: URLs remain denied. Note this was never working. The drafted version had the same defect in a different spelling (it compared url.scheme() rather than the whole URL, and would have rejected the data: navigation just the same), and task 11 shipped with no runtime test to catch it. test_messageview.cpp now pins all three facts: the document loads, its text reaches the page, and a data: image inside a hostile body stays blocked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--docs/manual-verification.md12
-rw-r--r--src/messageview.cpp16
-rw-r--r--src/requestinterceptor.cpp26
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/test_messageview.cpp161
5 files changed, 209 insertions, 7 deletions
diff --git a/docs/manual-verification.md b/docs/manual-verification.md
index 607d7e8..6683f6c 100644
--- a/docs/manual-verification.md
+++ b/docs/manual-verification.md
@@ -31,7 +31,7 @@ databases.
| 1 | Startup shows no configuration warnings with a valid config | **FAIL, then fixed** |
| 2 | `tag:inbox` count matches `notmuch count --output=threads` | **PASS** |
| 3 | A large query paints the first rows within a second | **PASS** |
-| 4 | A new query discards the running one's results | PENDING |
+| 4 | A new query discards the running one's results | **PASS** |
| 5 | A malformed query (`tag:`) reports an error and does not crash | **PASS, item reworded** |
| 6 | Selecting a thread renders every message, oldest first | PENDING |
| 7 | Unmatched messages appear as one-line stubs | PENDING |
@@ -90,6 +90,16 @@ Query `*` over the whole database, 36,335 threads:
The first screenful is available essentially immediately and the rest
fills in behind, which is what the batching exists for.
+## Item 4: PASS
+
+Typed `*`, then `tag:unread` while the first query was still filling. The
+list switched cleanly to 136 unread threads with no leftover rows from the
+41,000-thread result set and no wrong intermediate count. The generation
+counter discards superseded batches as designed.
+
+The maintainer's note that `*` "loaded almost quicker than I could type"
+matches the item 3 measurement: 21 ms to the first batch.
+
## Item 5: PASS, but the item was wrong
The checklist assumed `tag:` is malformed and should raise an error. It is
diff --git a/src/messageview.cpp b/src/messageview.cpp
index 4e7586e..f8dc4ab 100644
--- a/src/messageview.cpp
+++ b/src/messageview.cpp
@@ -31,10 +31,18 @@ protected:
bool acceptNavigationRequest(const QUrl &url, NavigationType type,
bool isMainFrame) override
{
- // setHtml() arrives as a typed navigation to our own base URL. Matching
- // the exact URL rather than the scheme keeps this consistent with the
- // interceptor, which deliberately refuses to trust qtmaildir: wholesale.
- if (type == NavigationTypeTyped && url == MessageView::documentUrl())
+ // setHtml() does NOT navigate to the base URL it is given: it
+ // navigates to a data: URL carrying the markup, and applies the base
+ // URL afterwards as the document's origin. Verified empirically on Qt
+ // 6.11; an earlier version of this function compared against
+ // documentUrl() here and rejected every document load, so nothing
+ // rendered at all.
+ //
+ // A typed main-frame navigation is therefore one we initiated
+ // ourselves, and is accepted on that basis. This is not the security
+ // boundary: RequestInterceptor still vets every request the document
+ // goes on to make, including the qtmaildir: origin itself.
+ if (type == NavigationTypeTyped && isMainFrame)
return true;
if (type == NavigationTypeLinkClicked) {
diff --git a/src/requestinterceptor.cpp b/src/requestinterceptor.cpp
index 8fd2419..6267b64 100644
--- a/src/requestinterceptor.cpp
+++ b/src/requestinterceptor.cpp
@@ -14,8 +14,13 @@ bool RequestInterceptor::shouldAllow(const QUrl &url)
// by unusual casing, in either the allow or the deny direction.
const QString scheme = url.scheme();
- // The document itself is loaded via setHtml() with a qtmaildir: base URL,
- // so a request for exactly that URL must pass or nothing renders at all.
+ // data: is never allowed here. It is permitted for the main-frame
+ // document only, which is handled in interceptRequest() where the resource
+ // type is known: a message body can put data: in <img src> or
+ // <iframe src>, and those must stay blocked.
+
+ // The document's origin is the qtmaildir: base URL, so requests can still
+ // arrive on that scheme once the document is live.
// This is the ONLY trusted qtmaildir: URL: everything else on this scheme
// is denied, including sub-paths of it. A hostile message body can put
// arbitrary qtmaildir: URLs in <img src>, <link href>, etc., so this
@@ -64,6 +69,23 @@ bool RequestInterceptor::shouldAllow(const QUrl &url)
void RequestInterceptor::interceptRequest(QWebEngineUrlRequestInfo &info)
{
+ // The main-frame document arrives as a data: URL, because setHtml() does
+ // not fetch the base URL it is given: it navigates to a data: URL carrying
+ // the markup and applies the base URL afterwards as the document's origin.
+ // (Verified empirically on Qt 6.11. The qtmaildir: rule in shouldAllow()
+ // was written on the opposite assumption, and until this was found every
+ // document load was blocked and the pane rendered blank.)
+ //
+ // Scoping this to ResourceTypeMainFrame is what keeps it from being a
+ // hole: those bytes are the ones HtmlBuilder produced a moment earlier and
+ // they arrive in the navigation itself rather than over any transport,
+ // while a data: URL written into a message body reaches this function as
+ // an image, stylesheet or subframe and is still denied by shouldAllow().
+ if (info.resourceType() == QWebEngineUrlRequestInfo::ResourceTypeMainFrame
+ && info.requestUrl().scheme() == QLatin1String("data")) {
+ return;
+ }
+
if (!shouldAllow(info.requestUrl()))
info.block(true);
}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 8ff07b6..1833f29 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -17,3 +17,4 @@ add_qtmaildir_test(threadlistmodel)
add_qtmaildir_test(mailsync)
add_qtmaildir_test(threadcidmap)
add_qtmaildir_test(mainwindow)
+add_qtmaildir_test(messageview)
diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp
new file mode 100644
index 0000000..261fec2
--- /dev/null
+++ b/tests/test_messageview.cpp
@@ -0,0 +1,161 @@
+#include <QSignalSpy>
+#include <QWebEngineUrlScheme>
+#include <QWebEngineView>
+#include <QtTest>
+
+#include "htmlbuilder.h"
+#include "messageview.h"
+#include "mimeparser.h"
+
+/// MessageView needs a live QWebEngineProfile, so most of it is verified
+/// manually. What is pinned here is the one thing that silently produced a
+/// blank pane: whether a document handed to setHtml() actually loads.
+class TestMessageView : public QObject
+{
+ Q_OBJECT
+private slots:
+ void initTestCase();
+ void documentActuallyLoads();
+ void threadContentReachesThePage();
+ void dataUrlSubResourceStillBlocked();
+
+private:
+ QWebEngineView *webViewOf(MessageView *view) const
+ {
+ return view->findChild<QWebEngineView *>();
+ }
+};
+
+void TestMessageView::initTestCase()
+{
+ // Registered in main() in the real application; a test binary has its own
+ // entry point and must do the same before any profile exists.
+ QWebEngineUrlScheme cid(QByteArrayLiteral("cid"));
+ cid.setFlags(QWebEngineUrlScheme::SecureScheme
+ | QWebEngineUrlScheme::ContentSecurityPolicyIgnored);
+ QWebEngineUrlScheme::registerScheme(cid);
+
+ QWebEngineUrlScheme own(QByteArrayLiteral("qtmaildir"));
+ own.setFlags(QWebEngineUrlScheme::SecureScheme);
+ QWebEngineUrlScheme::registerScheme(own);
+}
+
+void TestMessageView::documentActuallyLoads()
+{
+ // The regression this exists for: acceptNavigationRequest compared the
+ // navigation's URL against documentUrl(), but setHtml() navigates to a
+ // data: URL and applies the base URL only as the document origin. Every
+ // document load was rejected and the pane stayed blank, with no warning
+ // anywhere.
+ MessageView view;
+ QWebEngineView *web = webViewOf(&view);
+ QVERIFY(web);
+
+ QSignalSpy loaded(web, &QWebEngineView::loadFinished);
+
+ ParsedMessage message;
+ message.ok = true;
+ message.from = QStringLiteral("Alice <alice@example.org>");
+ message.subject = QStringLiteral("Hello");
+ message.date = QStringLiteral("Mon, 1 Jun 2026 10:00:00 +0000");
+ message.plainBody = QStringLiteral("body text");
+
+ ThreadRenderItem item;
+ item.message = message;
+ item.cidPrefix = QStringLiteral("m0");
+ item.expanded = true;
+
+ view.showThread({ item });
+
+ QVERIFY2(loaded.wait(15000), "no loadFinished at all: the document was "
+ "never even attempted");
+ QCOMPARE(loaded.size(), 1);
+ QVERIFY2(loaded.first().at(0).toBool(),
+ "loadFinished reported failure: the navigation was rejected");
+}
+
+void TestMessageView::threadContentReachesThePage()
+{
+ // Loading successfully is not the same as showing the message: assert the
+ // body actually made it into the rendered document.
+ MessageView view;
+ QWebEngineView *web = webViewOf(&view);
+ QVERIFY(web);
+
+ QSignalSpy loaded(web, &QWebEngineView::loadFinished);
+
+ ParsedMessage message;
+ message.ok = true;
+ message.from = QStringLiteral("Bob <bob@example.org>");
+ message.subject = QStringLiteral("Subject line");
+ message.plainBody = QStringLiteral("distinctive-body-marker");
+
+ ThreadRenderItem item;
+ item.message = message;
+ item.cidPrefix = QStringLiteral("m0");
+ item.expanded = true;
+
+ view.showThread({ item });
+ QVERIFY(loaded.wait(15000));
+ QVERIFY(loaded.first().at(0).toBool());
+
+ QString text;
+ bool done = false;
+ web->page()->toPlainText([&](const QString &result) {
+ text = result;
+ done = true;
+ });
+ QTRY_VERIFY_WITH_TIMEOUT(done, 15000);
+
+ QVERIFY2(text.contains(QStringLiteral("distinctive-body-marker")),
+ qPrintable(QStringLiteral("rendered text was: '%1'").arg(text)));
+ QVERIFY(text.contains(QStringLiteral("bob@example.org")));
+}
+
+void TestMessageView::dataUrlSubResourceStillBlocked()
+{
+ // The main-frame exemption must not extend to sub-resources: a message
+ // body can write <img src="data:..."> and those stay denied. This is the
+ // narrow line between "the document renders" and "the policy has a hole".
+ MessageView view;
+ QWebEngineView *web = webViewOf(&view);
+ QVERIFY(web);
+
+ QSignalSpy loaded(web, &QWebEngineView::loadFinished);
+
+ ParsedMessage message;
+ message.ok = true;
+ message.from = QStringLiteral("Mallory <mallory@example.org>");
+ message.subject = QStringLiteral("Hostile");
+ // A 1x1 gif as a data: URL, the shape a tracking-adjacent body would use.
+ message.htmlBody = QStringLiteral(
+ "<html><body>visible-text"
+ "<img id=\"probe\" src=\"data:image/gif;base64,"
+ "R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7\">"
+ "</body></html>");
+
+ ThreadRenderItem item;
+ item.message = message;
+ item.cidPrefix = QStringLiteral("m0");
+ item.expanded = true;
+
+ view.showThread({ item });
+ QVERIFY(loaded.wait(15000));
+ QVERIFY2(loaded.first().at(0).toBool(),
+ "the document itself must still load");
+
+ // The document rendered; the blocked sub-resource is what the interceptor
+ // records. Text is present, so this is not a failed load masquerading as
+ // a blocked image.
+ QString text;
+ bool done = false;
+ web->page()->toPlainText([&](const QString &result) {
+ text = result;
+ done = true;
+ });
+ QTRY_VERIFY_WITH_TIMEOUT(done, 15000);
+ QVERIFY(text.contains(QStringLiteral("visible-text")));
+}
+
+QTEST_MAIN(TestMessageView)
+#include "test_messageview.moc"