Procházet zdrojové kódy

Merge branch 'imap-uid-addressing': fix IMAP UID vs sequence-number confusion

Verified against the live mailbox after merging: search returned UID 15 and
the fetch succeeded (154058 bytes, subject 'teszt ocr'), where the same path
previously failed with 'Remote file not found'. Execution completed clean and
the full node fixture suite is 42/42.
fszontagh před 1 měsícem
rodič
revize
592ed5cb66
1 změnil soubory, kde provedl 30 přidání a 12 odebrání
  1. 30 12
      src/runner/imap/imap_client.cpp

+ 30 - 12
src/runner/imap/imap_client.cpp

@@ -96,9 +96,14 @@ common::Result<SearchResult> ImapClient::search(
     LOG_INFO("IMAP search - mailbox: '{}', criteria: '{}', limit: {}",
              mailbox, criteria, limit);
 
-    // For IMAP SEARCH, we use CUSTOMREQUEST
+    // For IMAP SEARCH, we use CUSTOMREQUEST.
+    // "UID SEARCH", not plain "SEARCH": a plain SEARCH returns message sequence
+    // numbers, which renumber on every expunge and are meaningless between
+    // sessions. Every other operation here addresses messages by UID (the
+    // ";UID=" URL form, "UID STORE", "UID COPY"), so search must return UIDs
+    // as well or the numbers do not refer to the same messages.
     std::string url_path = escapeMailbox(mailbox);
-    std::string search_cmd = "SEARCH " + criteria;
+    std::string search_cmd = "UID SEARCH " + criteria;
 
     auto response = performCommand(url_path, search_cmd);
 
@@ -107,7 +112,10 @@ common::Result<SearchResult> ImapClient::search(
             "IMAP search failed: " + response.error);
     }
 
-    // Parse search results (format: "* SEARCH 1 2 3 4 5")
+    // Parse search results (format: "* SEARCH 1 2 3 4 5").
+    // UID SEARCH replies with the same untagged "* SEARCH" line as a plain
+    // SEARCH - only the numbers it carries differ - so this parser is correct
+    // for both and must not be "fixed" to look for "* UID SEARCH".
     SearchResult result;
     std::istringstream stream(response.data);
     std::string line;
@@ -149,7 +157,7 @@ common::Result<EmailContent> ImapClient::fetch(
     // If peek mode, mark the message as unread to undo the \Seen flag
     if (peek && response.error.empty()) {
         std::string store_path = escapeMailbox(mailbox);
-        std::string store_cmd = "STORE " + uid + " -FLAGS (\\Seen)";
+        std::string store_cmd = "UID STORE " + uid + " -FLAGS (\\Seen)";
         auto store_response = performCommand(store_path, store_cmd);
         if (!store_response.error.empty()) {
             LOG_WARN("Failed to remove \\Seen flag after peek fetch: {}", store_response.error);
@@ -302,19 +310,24 @@ common::Result<ModifyResult> ImapClient::modify(
     LOG_INFO("IMAP modify - mailbox: '{}', uid: '{}', action: '{}'",
              mailbox, uid, action);
 
-    std::string url_path = escapeMailbox(mailbox) + ";UID=" + uid;
+    // The URL selects the mailbox only. It must not carry ";UID=", which would
+    // ask curl to fetch the message as well as run the command below - and a
+    // fetch sets \Seen, which would defeat "markUnread" outright.
+    std::string url_path = escapeMailbox(mailbox);
     std::string command;
 
+    // "UID STORE", not "STORE": the caller passes a UID, and a plain STORE
+    // would read it as a sequence number and flag an unrelated message.
     if (action == "markRead") {
-        command = "STORE " + uid + " +FLAGS (\\Seen)";
+        command = "UID STORE " + uid + " +FLAGS (\\Seen)";
     } else if (action == "markUnread") {
-        command = "STORE " + uid + " -FLAGS (\\Seen)";
+        command = "UID STORE " + uid + " -FLAGS (\\Seen)";
     } else if (action == "flag") {
-        command = "STORE " + uid + " +FLAGS (\\Flagged)";
+        command = "UID STORE " + uid + " +FLAGS (\\Flagged)";
     } else if (action == "unflag") {
-        command = "STORE " + uid + " -FLAGS (\\Flagged)";
+        command = "UID STORE " + uid + " -FLAGS (\\Flagged)";
     } else if (action == "delete") {
-        command = "STORE " + uid + " +FLAGS (\\Deleted)";
+        command = "UID STORE " + uid + " +FLAGS (\\Deleted)";
     } else {
         return common::Error(common::ErrorCode::InvalidArgument,
             "Unknown action: " + action);
@@ -341,8 +354,13 @@ common::Result<ModifyResult> ImapClient::move(
     LOG_INFO("IMAP move - from: '{}', uid: '{}', to: '{}'",
              mailbox, uid, target_mailbox);
 
-    std::string url_path = escapeMailbox(mailbox) + ";UID=" + uid;
-    std::string command = "COPY " + uid + " " + target_mailbox;
+    // Mailbox-only URL and a UID-addressed COPY, for the same reasons as
+    // modify() above. This one is the most damaging to get wrong: a plain COPY
+    // copies whichever message currently sits at that sequence position, and
+    // the delete below then removes a message chosen the same way - so a move
+    // could copy one email and delete another.
+    std::string url_path = escapeMailbox(mailbox);
+    std::string command = "UID COPY " + uid + " " + target_mailbox;
 
     auto response = performCommand(url_path, command);