diff --git a/imapserver/server.go b/imapserver/server.go index 6381a37..85385e6 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -3378,9 +3378,7 @@ 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) } - mc := m.MailboxCounts() - mbSrc.Sub(mc) - mbDst.Add(mc) + mbSrc.Sub(m.MailboxCounts()) // Copy of message record that we'll insert when UID is freed up. om := *m @@ -3394,7 +3392,9 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) { // is used for reputation calculation during future deliveries. m.MailboxOrigID = m.MailboxDestinedID m.IsReject = false + m.Seen = false } + mbDst.Add(m.MailboxCounts()) m.UID = uidnext m.ModSeq = modseq m.JunkFlagsForMailbox(mbDst, conf) diff --git a/smtpserver/analyze.go b/smtpserver/analyze.go index 294e010..94988a7 100644 --- a/smtpserver/analyze.go +++ b/smtpserver/analyze.go @@ -116,7 +116,49 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive log.Info("forwarded message, clearing identifying signals of forwarding mail server") } + assignMailbox := 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 determine a reputation because we also base it on outgoing + // messages and those are account-global. + mb, err := d.acc.MailboxFind(tx, mailbox) + if err != nil { + return fmt.Errorf("finding destination mailbox: %w", err) + } + if mb != nil { + // We want to deliver to mb.ID, but this message may be rejected and sent to the + // Rejects mailbox instead, with 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)) + } + return nil + } + reject := func(code int, secode string, errmsg string, err error, reason string) analysis { + // We may have set MailboxDestinedID below already while we had a transaction. If + // not, do it now. This makes it possible to use the per-mailbox reputation when a + // user moves the message out of the Rejects mailbox to the intended mailbox + // (typically Inbox). + if d.m.MailboxDestinedID == 0 { + var mberr error + d.acc.WithRLock(func() { + mberr = d.acc.DB.Read(ctx, func(tx *bstore.Tx) error { + return assignMailbox(tx) + }) + }) + if mberr != nil { + return analysis{false, mailbox, smtp.C451LocalErr, smtp.SeSys3Other0, false, "error processing", err, nil, nil, reasonReputationError} + } + d.m.MailboxID = 0 // We plan to reject, no need to set intended MailboxID. + } + accept := false if rs != nil && rs.AcceptRejectsToMailbox != "" { accept = true @@ -203,26 +245,8 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive var err error d.acc.WithRLock(func() { err = d.acc.DB.Read(ctx, 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 determine a reputation because we also base it on outgoing - // messages and those are account-global. - mb, err := d.acc.MailboxFind(tx, mailbox) - if err != nil { - return fmt.Errorf("finding destination mailbox: %w", err) - } - if mb != nil { - // We want to deliver to mb.ID, but this message may be rejected and sent to the - // Rejects mailbox instead, with 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)) + if err := assignMailbox(tx); err != nil { + return err } isjunk, conclusive, method, err = reputation(tx, log, d.m) diff --git a/webmail/api.go b/webmail/api.go index a0e4c84..e5f0948 100644 --- a/webmail/api.go +++ b/webmail/api.go @@ -844,6 +844,7 @@ func (Webmail) MessageMove(ctx context.Context, messageIDs []int64, mailboxID in // is used for reputation calculation during future deliveries. m.MailboxOrigID = m.MailboxDestinedID m.IsReject = false + m.Seen = false } m.UID = mbDst.UIDNext m.ModSeq = modseq