Bug 1914662 - Correctly extract or generate a message ID for delayed sending. r=mkmelin

Differential Revision: https://phabricator.services.mozilla.com/D220297

--HG--
extra : amend_source : 2b8372d71c3484a280f76e4915896e2eaac15d1a
extra : absorb_source : af7b000b38c88dcbfdda999fa33ec31f90f9da13

mailnews/compose/src/nsMsgSendLater.cpp

@@ -4,6 +4,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsMsgSendLater.h"
+#include "nsCOMPtr.h"
+#include "nsComponentManagerUtils.h"
+#include "nsDebug.h"
+#include "nsIMsgCompUtils.h"
 #include "nsIMsgMailNewsUrl.h"
 #include "nsMsgCompFields.h"
 #include "nsMsgCopy.h"
@@ -19,6 +23,7 @@
 #include "nsISmtpUrl.h"
 #include "nsIChannel.h"
 #include "nsNetUtil.h"
+#include "nsString.h"
 #include "prlog.h"
 #include "prmem.h"
 #include "nsComposeStrings.h"
@@ -56,6 +61,7 @@ nsMsgSendLater::nsMsgSendLater() {
   m_to = nullptr;
   m_bcc = nullptr;
   m_fcc = nullptr;
+  m_messageId = nullptr;
   m_newsgroups = nullptr;
   m_newshost = nullptr;
   m_headers = nullptr;
@@ -507,6 +513,25 @@ nsresult nsMsgSendLater::CompleteMailFileSend() {
     fields->SetFcc(m_fcc);
   }
 
+  char* messageId = m_messageId;
+
+  if (!messageId) {
+    // If the message headers don't include a message ID, generate one.
+    // Otherwise, the message won't be able to send with some protocols (e.g.
+    // SMTP).
+    nsCOMPtr<nsIMsgCompUtils> compUtils =
+        do_CreateInstance("@mozilla.org/messengercompose/computils;1", &rv);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCString newMessageId;
+    rv = compUtils->MsgGenerateMessageId(identity, ""_ns, newMessageId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    messageId = ToNewCString(newMessageId);
+  }
+
+  fields->SetMessageId(messageId);
+
   if (m_newsgroups) fields->SetNewsgroups(m_newsgroups);
 
   // Extract the returnReceipt, receiptHeaderType and DSN from the draft info.
@@ -900,6 +925,11 @@ nsresult nsMsgSendLater::BuildHeaders() {
       case 'l':
         if (!PL_strncasecmp("Lines", buf, end - buf)) prune_p = true;
         break;
+      case 'M':
+      case 'm':
+        if (!PL_strncasecmp("Message-ID", buf, end - buf))
+          header = &m_messageId;
+        break;
       case 'N':
       case 'n':
         if (!PL_strncasecmp("Newsgroups", buf, end - buf))

mailnews/compose/src/nsMsgSendLater.h

@@ -119,6 +119,7 @@ class nsMsgSendLater : public nsIMsgSendLater,
   char* m_to;
   char* m_bcc;
   char* m_fcc;
+  char* m_messageId;
   char* m_newsgroups;
   char* m_newshost;
   char* m_headers;

mailnews/compose/test/unit/data/sendlater_dsn.eml

@@ -0,0 +1,7 @@
+X-Mozilla-Draft-Info: internal/draft; vcard=0; receipt=0; DSN=1; uuencode=0;
+ attachmentreminder=0; deliveryformat=0
+From: Invalid User <from_A@foo.invalid>
+To: =?UTF-8?B?RnLDqcOpZGxlLCBUZXN0?= <to_A@foo.invalid>
+Subject: Test DSN
+
+Hello world!

mailnews/compose/test/unit/data/sendlater_dsn_with_message_id.eml

@@ -0,0 +1,8 @@
+X-Mozilla-Draft-Info: internal/draft; vcard=0; receipt=0; DSN=1; uuencode=0;
+ attachmentreminder=0; deliveryformat=0
+From: Invalid User <from_A@foo.invalid>
+To: =?UTF-8?B?RnLDqcOpZGxlLCBUZXN0?= <to_A@foo.invalid>
+Subject: Test DSN
+Message-ID: <f30c39e6-b14b-405a-8bf7-2ccc81fd1f6f@foo.invalid>
+
+Hello world!

mailnews/compose/test/unit/test_sendMessageLater_dsn_message_id.js

@@ -0,0 +1,162 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+var { MailServices } = ChromeUtils.importESModule(
+  "resource:///modules/MailServices.sys.mjs"
+);
+
+const { PromiseTestUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/mailnews/PromiseTestUtils.sys.mjs"
+);
+
+var account;
+var server;
+var identity;
+var smtpServer;
+var msgSendLater = Cc["@mozilla.org/messengercompose/sendlater;1"].getService(
+  Ci.nsIMsgSendLater
+);
+
+add_setup(() => {
+  // Ensure we have a local mail account, an normal account and appropriate
+  // servers and identities.
+  account = MailServices.accounts.createAccount();
+  const incomingServer = MailServices.accounts.createIncomingServer(
+    "test",
+    "localhost",
+    "pop3"
+  );
+
+  smtpServer = getBasicSmtpServer(1);
+  identity = getSmtpIdentity("identity@foo.invalid", smtpServer);
+  account.addIdentity(identity);
+  account.defaultIdentity = identity;
+  account.incomingServer = incomingServer;
+  MailServices.accounts.defaultAccount = account;
+
+  Assert.equal(identity.doFcc, true);
+
+  localAccountUtils.loadLocalMailAccount();
+  localAccountUtils.rootFolder.createLocalSubfolder("Sent");
+  MailServices.accounts.setSpecialFolders();
+
+  // Check that the send later service thinks we don't have messages to send
+  Assert.equal(msgSendLater.hasUnsentMessages(identity), false);
+
+  registerCleanupFunction(() => {
+    server.stop();
+
+    MailServices.accounts.removeAccount(account, false);
+
+    var thread = Services.tm.currentThread;
+    while (thread.hasPendingEvents()) {
+      thread.processNextEvent(true);
+    }
+  });
+});
+
+async function test_dsn_message_id(filename, messageIdPattern) {
+  // Now prepare to actually "send" the message later, i.e. dump it in the
+  // unsent messages folder.
+
+  var compFields = Cc[
+    "@mozilla.org/messengercompose/composefields;1"
+  ].createInstance(Ci.nsIMsgCompFields);
+
+  // Setting the compFields sender and recipient to any value is required to
+  // survive mime_sanity_check_fields in nsMsgCompUtils.cpp. Sender and
+  // recipient are required for sendMessageFile but SMTP transaction values will
+  // be used directly from mail body. We don't set the DSN flag here, because
+  // this is handled by the X-Mozilla-Draft-Info header in the message content.
+  compFields.from = "irrelevant@foo.invalid";
+  compFields.to = "irrelevant@foo.invalid";
+
+  const copyListener = new PromiseTestUtils.PromiseCopyListener();
+  // Method from nsIMsgSendListener that's expected on the listener.
+  copyListener.onGetDraftFolderURI = () => {};
+
+  // Queue the message for sending.
+  var msgSend = Cc["@mozilla.org/messengercompose/send;1"].createInstance(
+    Ci.nsIMsgSend
+  );
+
+  msgSend.sendMessageFile(
+    identity,
+    account.key,
+    compFields,
+    do_get_file(filename),
+    false,
+    false,
+    Ci.nsIMsgSend.nsMsgQueueForLater,
+    null,
+    copyListener,
+    null,
+    null
+  );
+  await copyListener.promise;
+
+  // Set up the SMTP server.
+  server = setupServerDaemon();
+
+  // Start the fake SMTP server
+  server.start();
+  smtpServer.QueryInterface(Ci.nsISmtpServer).port = server.port;
+
+  var sendListener = new PromiseTestUtils.PromiseSendLaterListener();
+
+  msgSendLater.addListener(sendListener);
+
+  // Send the unsent message.
+  msgSendLater.sendUnsentMessages(identity);
+  server.performTest();
+  const sendResult = await sendListener.promise;
+
+  // Test that the message has been sent without any issue. We don't need to
+  // look at the nsresult passed to onStopSending, because the promise would
+  // have been rejected if it wasn't NS_OK.
+  Assert.equal(
+    sendResult.totalTried,
+    1,
+    "1 message send should have been attempted"
+  );
+  Assert.equal(
+    sendResult.successful,
+    1,
+    "the message should have been sent successfully"
+  );
+  Assert.equal(
+    msgSendLater.sendingMessages,
+    false,
+    "the nsIMsgSendLater instance should have stopped sending messages"
+  );
+
+  // Test that we sent a message ID that matches with the expected pattern.
+  const mailFromLine = server.playTransaction().them[1];
+  const testRegExp = new RegExp(`RET=FULL ENVID=<${messageIdPattern}>`);
+  Assert.ok(
+    testRegExp.test(mailFromLine),
+    "smtp client should send a valid message ID"
+  );
+}
+
+/**
+ * Tests that a delayed send of a message with DSN turned on over SMTP results
+ * in an SMTP command that includes a valid message ID, even if the message does
+ * not include a Message-ID header.
+ */
+add_task(async function test_dsn_message_id_without_header() {
+  await test_dsn_message_id("data/sendlater_dsn.eml", "[a-z0-9-]+@foo.invalid");
+});
+
+/**
+ * Tests that a delayed send of a message with DSN turned on over SMTP results
+ * in an SMTP command that includes the message ID from the message's Message-ID
+ * header.
+ */
+add_task(async function test_dsn_message_id_with_header() {
+  await test_dsn_message_id(
+    "data/sendlater_dsn_with_message_id.eml",
+    "f30c39e6-b14b-405a-8bf7-2ccc81fd1f6f@foo.invalid"
+  );
+});

mailnews/compose/test/unit/xpcshell.ini

@@ -32,6 +32,7 @@ skip-if = os == 'mac'
 [test_sendMailAddressIDN.js]
 [test_sendMailMessage.js]
 [test_sendMessageFile.js]
+[test_sendMessageLater_dsn_message_id.js]
 [test_sendMessageLater.js]
 [test_sendMessageLater2.js]
 [test_sendMessageLater3.js]

mailnews/test/fakeserver/Smtpd.sys.mjs

@@ -30,7 +30,7 @@ export class SMTP_RFC2821_handler {
   kUsername = "testsmtp";
   kPassword = "smtptest";
   kAuthSchemes = ["CRAM-MD5", "PLAIN", "LOGIN"];
-  kCapabilities = ["8BITMIME", "SIZE", "CLIENTID"];
+  kCapabilities = ["8BITMIME", "SIZE", "CLIENTID", "DSN"];
   _nextAuthFunction = undefined;
 
   constructor(daemon, { username = "testsmtp", password = "smtptest" } = {}) {

mailnews/test/resources/PromiseTestUtils.sys.mjs

@@ -386,3 +386,48 @@ PromiseTestUtils.PromiseStoreScanListener.prototype = {
     return this._promise;
   },
 };
+
+/**
+ * PromiseSendLaterListener is a helper for sending messages with a delay via
+ * nsIMsgSendLater.
+ *
+ * If sending was successful, it resolves with an object that includes the
+ * number of messages the service tried to send, and the number of messages it
+ * successfully sent.
+ *
+ * @implements {nsIMsgSendLaterListener}
+ */
+PromiseTestUtils.PromiseSendLaterListener = class {
+  QueryInterface = ChromeUtils.generateQI(["nsIMsgSendLaterListener"]);
+
+  constructor() {
+    this._promise = new Promise((resolve, reject) => {
+      this._resolve = resolve;
+      this._reject = reject;
+    });
+  }
+
+  onStartSending() {}
+  onMessageStartSending() {}
+  onMessageSendProgress() {}
+
+  onMessageSendError(aCurrentMessage, aMessageHeader, aStatus) {
+    this._reject(aStatus);
+  }
+
+  onStopSending(aStatus, aMsg, aTotalTried, aSuccessful) {
+    if (aStatus != Cr.NS_OK) {
+      this._reject(aStatus);
+      return;
+    }
+
+    this._resolve({
+      totalTried: aTotalTried,
+      successful: aSuccessful,
+    });
+  }
+
+  get promise() {
+    return this._promise;
+  }
+};