diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/composecontext.cpp | 23 | ||||
| -rw-r--r-- | src/maildirname.cpp | 61 | ||||
| -rw-r--r-- | src/maildirname.h | 24 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 24 |
4 files changed, 126 insertions, 6 deletions
diff --git a/src/composecontext.cpp b/src/composecontext.cpp index d0406fc..7233330 100644 --- a/src/composecontext.cpp +++ b/src/composecontext.cpp @@ -24,6 +24,7 @@ #include "composecontext.h" #include "config.h" +#include "maildirname.h" #include "mimeparser.h" #include <QDir> @@ -491,16 +492,32 @@ ComposeContext ComposeContextBuilder::forDraft(const Config &config, { ComposeContext context; + // Item 163. The caller's path comes from the model, captured when the + // query ran, and mbsync renames an uploaded draft to add its `,U=<uid>` + // infix. Resolving first is what stops a rename from refusing the reopen: + // the refusal happens BEFORE any composer exists, so the user composes + // again into a FRESH window whose autosave has no previous path to unlink, + // and the draft is silently forked into two files with two Message-IDs, + // both of which reach the server. + // + // Returns the path unchanged when nothing was renamed, and empty when the + // file is genuinely gone, which still fails below exactly as before. + const QString resolved = MaildirName::resolveRenamed(path); + MimeParser parser; - const ParsedMessage draft = parser.parse(path); + const ParsedMessage draft = parser.parse(resolved); if (!draft.ok) return context; // Kind::New and empty: the caller reports the failure. context.kind = ComposeContext::Kind::Draft; - context.originalPath = path; + context.originalPath = resolved; // The file this composer OWNS. Without it the first autosave writes a // second draft and leaves this one behind, so one message becomes two. - context.draftPath = path; + // + // The RESOLVED path, never the caller's: seeding the stale one would let + // the reopen succeed and the unlink still miss, which is the same fork + // arriving one step later. + context.draftPath = resolved; const auto addresses = [](const QString &header) { QStringList out; diff --git a/src/maildirname.cpp b/src/maildirname.cpp index 6263aec..9f827fc 100644 --- a/src/maildirname.cpp +++ b/src/maildirname.cpp @@ -20,6 +20,8 @@ #include <QCoreApplication> #include <QDateTime> +#include <QDir> +#include <QFileInfo> #include <QHostInfo> namespace MaildirName { @@ -77,4 +79,63 @@ QString fresh(const QString &oldName) .arg(info); } +QString resolveRenamed(const QString &path) +{ + if (path.isEmpty()) + return QString(); + + // The ordinary case, and the overwhelmingly common one: nothing was + // renamed. One stat, then out. + if (QFileInfo::exists(path)) + return path; + + const QFileInfo info(path); + const QString name = info.fileName(); + + // The unique part mbsync preserves. `<stem>:2,D` becomes + // `<stem>,U=5:2,D`, so the stem ends at whichever of `,` or `:` comes + // first. A name carrying neither is all stem. + int cut = name.size(); + for (const QChar separator : { QLatin1Char(','), QLatin1Char(':') }) { + const int at = name.indexOf(separator); + if (at >= 0 && at < cut) + cut = at; + } + const QString stem = name.left(cut); + if (stem.isEmpty()) + return QString(); + + // One directory, never a recursive walk: a rename keeps the file where it + // was, and a file that changed FOLDERS is a different question that only + // the message id can answer (see NotmuchWorker::moveMessages(), item 162). + const QDir dir(info.absolutePath()); + if (!dir.exists()) + return QString(); + + QString found; + const QFileInfoList entries = + dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot); + for (const QFileInfo &entry : entries) { + const QString candidate = entry.fileName(); + // Anchored on the stem AND on what follows it, so `...Q2` cannot match + // `...Q23`: the next character must begin the infix or the flags. + if (!candidate.startsWith(stem)) + continue; + const QString rest = candidate.mid(stem.size()); + if (!rest.isEmpty() && !rest.startsWith(QLatin1Char(',')) + && !rest.startsWith(QLatin1Char(':'))) { + continue; + } + + // Two files sharing a stem cannot happen in a correct Maildir. Refuse + // rather than guess: the caller reports "gone", which is honest, where + // a guess could open, move or delete the wrong message. + if (!found.isEmpty()) + return QString(); + found = entry.absoluteFilePath(); + } + + return found; +} + } // namespace MaildirName diff --git a/src/maildirname.h b/src/maildirname.h index f24bc71..255517d 100644 --- a/src/maildirname.h +++ b/src/maildirname.h @@ -38,4 +38,28 @@ namespace MaildirName { /// what a newly composed draft is. QString fresh(const QString &oldName); +/// The file \p path names, or the renamed file that replaced it. +/// +/// Item 163. mbsync renames an uploaded file to add its `,U=<uid>` infix, and +/// anything holding the previous name (the model's `MessageRef::filePath`, a +/// draft's `ComposeContext::draftPath`) then points at a path that no longer +/// exists. Returns \p path unchanged when it is still there, so the ordinary +/// case costs one stat and nothing else. +/// +/// Matched on the UNIQUE STEM, the part before the first `,` or `:`, which +/// mbsync preserves: `<stem>:2,D` becomes `<stem>,U=5:2,D`. That is what makes +/// this safe to do by filename at all. The search is confined to the file's +/// own directory and never recurses, and an ambiguous match (more than one +/// candidate, which a correct Maildir cannot produce) yields nothing rather +/// than guessing. +/// +/// Empty when there is no such file, which every caller must treat as the +/// genuine "it is gone" it is: recovering silently from a real deletion would +/// turn a reportable defect into a wrong answer. +/// +/// This resolves a RENAME, not a MOVE. A file that changed folders is a +/// different question and belongs to whoever knows the message id; +/// `NotmuchWorker::moveMessages()` re-resolves that way for item 162. +QString resolveRenamed(const QString &path); + } // namespace MaildirName diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index af3b817..5845922 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -18,6 +18,8 @@ #include "mainwindow.h" +#include "maildirname.h" + #include <QAction> #include <QApplication> #include <QCloseEvent> @@ -1043,8 +1045,13 @@ void MainWindow::openComposerFor(const MessageRef &ref, return; } + // Item 163, the same stale path the pane and the draft reopen hit. Here it + // refuses a Reply or a Forward outright, so the user cannot answer a + // message that is sitting on disk and readable. + const QString originalPath = MaildirName::resolveRenamed(ref.filePath); + MimeParser parser; - const ParsedMessage original = parser.parse(ref.filePath); + const ParsedMessage original = parser.parse(originalPath); if (!original.ok) { showTransientStatus(tr("That message could not be read")); return; @@ -1052,7 +1059,7 @@ void MainWindow::openComposerFor(const MessageRef &ref, ComposeContext context; context.kind = kind; - context.originalPath = ref.filePath; + context.originalPath = originalPath; const bool replyAll = kind == ComposeContext::Kind::ReplyAll; const bool forwarding = kind == ComposeContext::Kind::Forward; @@ -3849,11 +3856,22 @@ void MainWindow::renderMessages(const QVector<MessageRef> &messages) const MessageRef &ref = messages.at(i); ThreadRenderItem item; - item.message = parser.parse(ref.filePath); + // Item 163. The model's path was captured when the query ran, and + // mbsync renames an uploaded file to add its `,U=<uid>` infix, so a row + // loaded before that sync names a file that no longer exists. The pane + // then reported the message unreadable while nothing was wrong with it. + // Unchanged when nothing was renamed; empty when the file is genuinely + // gone, which still reports below. + const QString path = MaildirName::resolveRenamed(ref.filePath); + item.message = parser.parse(path); if (!item.message.ok) { // One unreadable message must not lose the rest of the thread, so // it becomes an inline note rather than replacing the whole pane. + // + // Named by the path the model HOLDS, not by the resolved one: when + // resolution failed there is no resolved path, and the stale name + // is what the user can act on. item.message = {}; item.message.ok = true; item.message.from = tr("(unreadable message)"); |
