diff --git a/imapserver/server.go b/imapserver/server.go index f1ff40f..16264e9 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -2967,7 +2967,11 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) { m.ID = 0 m.UID = uidFirst + store.UID(i) m.MailboxID = mbDst.ID - m.MailboxOrigID = mbSrc.ID + if mbSrc.Name == conf.RejectsMailbox && m.MailboxDestinedID != 0 { + // Incorrectly delivered to Rejects mailbox. Adjust MailboxOrigID so this message + // is used for reputation calculation during future deliveries. + m.MailboxOrigID = m.MailboxDestinedID + } m.TrainedJunk = nil m.JunkFlagsForMailbox(mbDst.Name, conf) err := tx.Insert(&m) @@ -3092,7 +3096,7 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) { c.account.WithWLock(func() { c.xdbwrite(func(tx *bstore.Tx) { - c.xmailboxID(tx, c.mailboxID) // Validate. + mbSrc := c.xmailboxID(tx, c.mailboxID) // Validate. mbDst = c.xmailbox(tx, name, "TRYCREATE") if mbDst.ID == c.mailboxID { xuserErrorf("cannot move to currently selected mailbox") @@ -3128,6 +3132,11 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) { xserverErrorf("internal error: got uid %d, expected %d, for index %d", m.UID, uids[i], i) } m.MailboxID = mbDst.ID + if mbSrc.Name == conf.RejectsMailbox && m.MailboxDestinedID != 0 { + // Incorrectly delivered to Rejects mailbox. Adjust MailboxOrigID so this message + // is used for reputation calculation during future deliveries. + m.MailboxOrigID = m.MailboxDestinedID + } m.UID = uidnext m.JunkFlagsForMailbox(mbDst.Name, conf) uidnext++ diff --git a/smtpserver/analyze.go b/smtpserver/analyze.go index 61da216..db68ede 100644 --- a/smtpserver/analyze.go +++ b/smtpserver/analyze.go @@ -163,8 +163,8 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive err = d.acc.DB.Read(func(tx *bstore.Tx) error { // Set message MailboxID to which mail will be delivered. Reputation is // per-mailbox. If referenced mailbox is not found (e.g. does not yet exist), we - // can still use determine a reputation because we also base it on outgoing - // messages and those account-global. + // can still determine a reputation because we also base it on outgoing + // messages and those are account-global. mailbox := d.rcptAcc.destination.Mailbox if mailbox == "" { mailbox = "Inbox" @@ -174,7 +174,15 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive } mb := d.acc.MailboxFindX(tx, mailbox) if mb != nil { + // We want to deliver to mb.ID, but this message may be rejected and sent to the + // Rejects mailbox instead, which MailboxID overwritten. Record the ID in + // MailboxDestinedID too. If the message is later moved out of the Rejects mailbox, + // we'll adjust the MailboxOrigID so it gets taken into account during reputation + // calculating in future deliveries. If we end up delivering to the intended + // mailbox (i.e. not rejecting), MailboxDestinedID is cleared during delivery so we + // don't store it unnecessarily. d.m.MailboxID = mb.ID + d.m.MailboxDestinedID = mb.ID } else { log.Debug("mailbox not found in database", mlog.Field("mailbox", mailbox)) } diff --git a/store/account.go b/store/account.go index 77eb036..5959109 100644 --- a/store/account.go +++ b/store/account.go @@ -229,11 +229,23 @@ type Message struct { UID UID `bstore:"nonzero"` // UID, for IMAP. Set during deliver. MailboxID int64 `bstore:"nonzero,unique MailboxID+UID,index MailboxID+Received,ref Mailbox"` - // Mailbox message originally delivered to. I.e. not changed when moved to Trash or - // Junk. Useful for per-mailbox reputation. Not a bstore reference to prevent - // having to update all messages in a mailbox when the original mailbox is removed. - // Use of this field requires checking if the mailbox still exists. - MailboxOrigID int64 + // MailboxOrigID is the mailbox the message was originally delivered to. Typically + // Inbox or Rejects, but can also be Postmaster and TLS/DMARC reporting addresses. + // MailboxOrigID is not changed when the message is moved to another mailbox, e.g. + // Archive/Trash/Junk. Used for per-mailbox reputation. + // + // MailboxDestinedID is normally 0, but when a message is delivered to the Rejects + // mailbox, it is set to the intended mailbox according to delivery rules, + // typically that of Inbox. When such a message is moved out of Rejects, the + // MailboxOrigID is corrected by setting it to MailboxDestinedID. This ensures the + // message is used for reputation calculation for future deliveries to that + // mailbox. + // + // These are not bstore references to prevent having to update all messages in a + // mailbox when the original mailbox is removed. Use of these fields requires + // checking if the mailbox still exists. + MailboxOrigID int64 + MailboxDestinedID int64 Received time.Time `bstore:"default now,index"` @@ -600,6 +612,11 @@ func (a *Account) DeliverX(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os m.ParsedBuf = buf } + // If we are delivering to the originally intended mailbox, no need to store the mailbox ID again. + if m.MailboxDestinedID != 0 && m.MailboxDestinedID == m.MailboxOrigID { + m.MailboxDestinedID = 0 + } + err = tx.Insert(m) xcheckf(err, "inserting message")