in DSNs, add a References header pointing to the message with deliverability issues

so mail user agents will show DSNs threaded/grouped with the original message.
we store the MessageID in the message queue, so we have the value within reach
when we need it.

i saw a references header in a DSN from gmail on a test account. makes sense to me.

dsn/dsn.go

@@ -45,6 +45,13 @@ type Message struct {
 	// Message subject header, e.g. describing mail delivery failure.
 	Subject string
 
+	// Set when message is composed.
+	MessageID string
+
+	// References header, with Message-ID of original message this DSN is about. So
+	// mail user-agents will thread the DSN with the original message.
+	References string
+
 	// Human-readable text explaining the failure. Line endings should be
 	// bare newlines, not \r\n. They are converted to \r\n when composing.
 	TextBody string
@@ -158,7 +165,11 @@ func (m *Message) Compose(log *mlog.Log, smtputf8 bool) ([]byte, error) {
 	header("From", fmt.Sprintf("<%s>", m.From.XString(smtputf8))) // todo: would be good to have a local ascii-only name for this address.
 	header("To", fmt.Sprintf("<%s>", m.To.XString(smtputf8)))     // todo: we could just leave this out if it has utf-8 and remote does not support utf-8.
 	header("Subject", m.Subject)
-	header("Message-Id", fmt.Sprintf("<%s>", mox.MessageIDGen(smtputf8)))
+	m.MessageID = mox.MessageIDGen(smtputf8)
+	header("Message-Id", fmt.Sprintf("<%s>", m.MessageID))
+	if m.References != "" {
+		header("References", m.References)
+	}
 	header("Date", time.Now().Format(message.RFC5322Z))
 	header("MIME-Version", "1.0")
 	mp := multipart.NewWriter(msgw)

gentestdata.go

@@ -233,7 +233,7 @@ Accounts:
 	const qmsg = "From: <test0@mox.example>\r\nTo: <other@remote.example>\r\nSubject: test\r\n\r\nthe message...\r\n"
 	_, err = fmt.Fprint(mf, qmsg)
 	xcheckf(err, "writing message")
-	_, err = queue.Add(ctxbg, mlog.New("gentestdata"), "test0", mailfrom, rcptto, false, false, int64(len(qmsg)), prefix, mf, nil, true)
+	_, err = queue.Add(ctxbg, mlog.New("gentestdata"), "test0", mailfrom, rcptto, false, false, int64(len(qmsg)), "<test@localhost>", prefix, mf, nil, true)
 	xcheckf(err, "enqueue message")
 
 	// Create three accounts.

http/adminapi.json

@@ -2945,6 +2945,13 @@
 						"int64"
 					]
 				},
+				{
+					"Name": "MessageID",
+					"Docs": "Used when composing a DSN, in its References header.",
+					"Typewords": [
+						"string"
+					]
+				},
 				{
 					"Name": "MsgPrefix",
 					"Docs": "",

queue/dsn.go

@@ -101,11 +101,12 @@ func queueDSN(log *mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg stri
 	}
 
 	dsnMsg := &dsn.Message{
-		SMTPUTF8: m.SMTPUTF8,
+		SMTPUTF8:   m.SMTPUTF8,
-		From:     smtp.Path{Localpart: "postmaster", IPDomain: dns.IPDomain{Domain: mox.Conf.Static.HostnameDomain}},
+		From:       smtp.Path{Localpart: "postmaster", IPDomain: dns.IPDomain{Domain: mox.Conf.Static.HostnameDomain}},
-		To:       m.Sender(),
+		To:         m.Sender(),
-		Subject:  subject,
+		Subject:    subject,
-		TextBody: textBody,
+		References: m.MessageID,
+		TextBody:   textBody,
 
 		ReportingMTA: mox.Conf.Static.HostnameDomain.ASCII,
 		ArrivalDate:  m.Queued,

queue/queue.go

@@ -101,9 +101,10 @@ type Msg struct {
 	NextAttempt        time.Time           // For scheduling.
 	LastAttempt        *time.Time
 	LastError          string
-	Has8bit            bool  // Whether message contains bytes with high bit set, determines whether 8BITMIME SMTP extension is needed.
+	Has8bit            bool   // Whether message contains bytes with high bit set, determines whether 8BITMIME SMTP extension is needed.
-	SMTPUTF8           bool  // Whether message requires use of SMTPUTF8.
+	SMTPUTF8           bool   // Whether message requires use of SMTPUTF8.
-	Size               int64 // Full size of message, combined MsgPrefix with contents of message file.
+	Size               int64  // Full size of message, combined MsgPrefix with contents of message file.
+	MessageID          string // Used when composing a DSN, in its References header.
 	MsgPrefix          []byte
 
 	// If set, this message is a DSN and this is a version using utf-8, for the case
@@ -199,7 +200,7 @@ func Count(ctx context.Context) (int, error) {
 // this data is used as the message when delivering the DSN and the remote SMTP
 // server supports SMTPUTF8. If the remote SMTP server does not support SMTPUTF8,
 // the regular non-utf8 message is delivered.
-func Add(ctx context.Context, log *mlog.Log, senderAccount string, mailFrom, rcptTo smtp.Path, has8bit, smtputf8 bool, size int64, msgPrefix []byte, msgFile *os.File, dsnutf8Opt []byte, consumeFile bool) (int64, error) {
+func Add(ctx context.Context, log *mlog.Log, senderAccount string, mailFrom, rcptTo smtp.Path, has8bit, smtputf8 bool, size int64, messageID string, msgPrefix []byte, msgFile *os.File, dsnutf8Opt []byte, consumeFile bool) (int64, error) {
 	// todo: Add should accept multiple rcptTo if they are for the same domain. so we can queue them for delivery in one (or just a few) session(s), transferring the data only once. ../rfc/5321:3759
 
 	if Localserve {
@@ -220,7 +221,7 @@ func Add(ctx context.Context, log *mlog.Log, senderAccount string, mailFrom, rcp
 	}()
 
 	now := time.Now()
-	qm := Msg{0, now, senderAccount, mailFrom.Localpart, mailFrom.IPDomain, rcptTo.Localpart, rcptTo.IPDomain, formatIPDomain(rcptTo.IPDomain), 0, nil, now, nil, "", has8bit, smtputf8, size, msgPrefix, dsnutf8Opt, ""}
+	qm := Msg{0, now, senderAccount, mailFrom.Localpart, mailFrom.IPDomain, rcptTo.Localpart, rcptTo.IPDomain, formatIPDomain(rcptTo.IPDomain), 0, nil, now, nil, "", has8bit, smtputf8, size, messageID, msgPrefix, dsnutf8Opt, ""}
 
 	if err := tx.Insert(&qm); err != nil {
 		return 0, err

queue/queue_test.go

@@ -85,11 +85,11 @@ func TestQueue(t *testing.T) {
 	}
 
 	path := smtp.Path{Localpart: "mjl", IPDomain: dns.IPDomain{Domain: dns.Domain{ASCII: "mox.example"}}}
-	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), nil, prepareFile(t), nil, true)
+	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), "<test@localhost>", nil, prepareFile(t), nil, true)
 	tcheck(t, err, "add message to queue for delivery")
 
 	mf2 := prepareFile(t)
-	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), nil, mf2, nil, false)
+	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), "<test@localhost>", nil, mf2, nil, false)
 	tcheck(t, err, "add message to queue for delivery")
 	os.Remove(mf2.Name())
 
@@ -293,7 +293,7 @@ func TestQueue(t *testing.T) {
 
 	// Add a message to be delivered with submit because of its route.
 	topath := smtp.Path{Localpart: "mjl", IPDomain: dns.IPDomain{Domain: dns.Domain{ASCII: "submit.example"}}}
-	_, err = Add(ctxbg, xlog, "mjl", path, topath, false, false, int64(len(testmsg)), nil, prepareFile(t), nil, true)
+	_, err = Add(ctxbg, xlog, "mjl", path, topath, false, false, int64(len(testmsg)), "<test@localhost>", nil, prepareFile(t), nil, true)
 	tcheck(t, err, "add message to queue for delivery")
 	wasNetDialer = testDeliver(fakeSubmitServer)
 	if !wasNetDialer {
@@ -301,7 +301,7 @@ func TestQueue(t *testing.T) {
 	}
 
 	// Add a message to be delivered with submit because of explicitly configured transport, that uses TLS.
-	msgID, err := Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), nil, prepareFile(t), nil, true)
+	msgID, err := Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), "<test@localhost>", nil, prepareFile(t), nil, true)
 	tcheck(t, err, "add message to queue for delivery")
 	transportSubmitTLS := "submittls"
 	n, err = Kick(ctxbg, msgID, "", "", &transportSubmitTLS)
@@ -325,7 +325,7 @@ func TestQueue(t *testing.T) {
 	}
 
 	// Add a message to be delivered with socks.
-	msgID, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), nil, prepareFile(t), nil, true)
+	msgID, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), "<test@localhost>", nil, prepareFile(t), nil, true)
 	tcheck(t, err, "add message to queue for delivery")
 	transportSocks := "socks"
 	n, err = Kick(ctxbg, msgID, "", "", &transportSocks)
@@ -339,7 +339,7 @@ func TestQueue(t *testing.T) {
 	}
 
 	// Add another message that we'll fail to deliver entirely.
-	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), nil, prepareFile(t), nil, true)
+	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), "<test@localhost>", nil, prepareFile(t), nil, true)
 	tcheck(t, err, "add message to queue for delivery")
 
 	msgs, err = List(ctxbg)
@@ -473,7 +473,7 @@ func TestQueueStart(t *testing.T) {
 	}
 
 	path := smtp.Path{Localpart: "mjl", IPDomain: dns.IPDomain{Domain: dns.Domain{ASCII: "mox.example"}}}
-	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), nil, prepareFile(t), nil, true)
+	_, err = Add(ctxbg, xlog, "mjl", path, path, false, false, int64(len(testmsg)), "<test@localhost>", nil, prepareFile(t), nil, true)
 	tcheck(t, err, "add message to queue for delivery")
 	checkDialed(true)
 

smtpserver/dsn.go

@@ -46,7 +46,7 @@ func queueDSN(ctx context.Context, c *conn, rcptTo smtp.Path, m dsn.Message) err
 	// ../rfc/3464:433
 	const has8bit = false
 	const smtputf8 = false
-	if _, err := queue.Add(ctx, c.log, "", smtp.Path{}, rcptTo, has8bit, smtputf8, int64(len(buf)), nil, f, bufUTF8, true); err != nil {
+	if _, err := queue.Add(ctx, c.log, "", smtp.Path{}, rcptTo, has8bit, smtputf8, int64(len(buf)), m.MessageID, nil, f, bufUTF8, true); err != nil {
 		return err
 	}
 	err = f.Close()

smtpserver/server.go

@@ -1678,8 +1678,10 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
 
 	// Add Message-Id header if missing.
 	// ../rfc/5321:4131 ../rfc/6409:751
-	if header.Get("Message-Id") == "" {
+	messageID := header.Get("Message-Id")
-		msgPrefix = append(msgPrefix, fmt.Sprintf("Message-Id: <%s>\r\n", mox.MessageIDGen(c.smtputf8))...)
+	if messageID == "" {
+		messageID = mox.MessageIDGen(c.smtputf8)
+		msgPrefix = append(msgPrefix, fmt.Sprintf("Message-Id: <%s>\r\n", messageID)...)
 	}
 
 	// ../rfc/6409:745
@@ -1870,7 +1872,7 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
 			}
 
 			msgSize := int64(len(xmsgPrefix)) + msgWriter.Size
-			if _, err := queue.Add(ctx, c.log, c.account.Name, *c.mailFrom, rcptAcc.rcptTo, msgWriter.Has8bit, c.smtputf8, msgSize, xmsgPrefix, dataFile, nil, i == len(c.recipients)-1); err != nil {
+			if _, err := queue.Add(ctx, c.log, c.account.Name, *c.mailFrom, rcptAcc.rcptTo, msgWriter.Has8bit, c.smtputf8, msgSize, messageID, xmsgPrefix, dataFile, nil, i == len(c.recipients)-1); err != nil {
 				// Aborting the transaction is not great. But continuing and generating DSNs will
 				// probably result in errors as well...
 				metricSubmission.WithLabelValues("queueerror").Inc()
@@ -2560,10 +2562,11 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW
 	if len(deliverErrors) > 0 {
 		now := time.Now()
 		dsnMsg := dsn.Message{
-			SMTPUTF8: c.smtputf8,
+			SMTPUTF8:   c.smtputf8,
-			From:     smtp.Path{Localpart: "postmaster", IPDomain: deliverErrors[0].rcptTo.IPDomain},
+			From:       smtp.Path{Localpart: "postmaster", IPDomain: deliverErrors[0].rcptTo.IPDomain},
-			To:       *c.mailFrom,
+			To:         *c.mailFrom,
-			Subject:  "mail delivery failure",
+			Subject:    "mail delivery failure",
+			References: messageID,
 
 			// Per-message details.
 			ReportingMTA:    mox.Conf.Static.HostnameDomain.ASCII,