diff --git a/imapserver/replace.go b/imapserver/replace.go index 26a6d31..f0cf831 100644 --- a/imapserver/replace.go +++ b/imapserver/replace.go @@ -166,7 +166,7 @@ func (c *conn) cmdxReplace(isUID bool, tag, cmd string, p *parser) { } var file *os.File - var newMsgPath string + var newID int64 // Delivered message ID, file removed on error. var f io.Writer var commit bool @@ -186,17 +186,14 @@ func (c *conn) cmdxReplace(isUID bool, tag, cmd string, p *parser) { var err error file, err = store.CreateMessageTemp(c.log, "imap-replace") xcheckf(err, "creating temp file for message") - newMsgPath = file.Name() + defer store.CloseRemoveTempFile(c.log, file, "temporary message file") f = file defer func() { - if file != nil { - err := file.Close() - c.xsanity(err, "close temporary file for replace") - } - if newMsgPath != "" && !commit { - err := os.Remove(newMsgPath) - c.xsanity(err, "remove temporary file for replace") + if !commit && newID != 0 { + p := c.account.MessagePath(newID) + err := os.Remove(p) + c.xsanity(err, "remove message file for replace after error") } }() } @@ -289,8 +286,7 @@ func (c *conn) cmdxReplace(isUID bool, tag, cmd string, p *parser) { err = c.account.MessageAdd(c.log, tx, &mbDst, &nm, file, store.AddOpts{}) xcheckf(err, "delivering message") - // Update path to what is stored in the account. We may still have to clean it up on errors. - newMsgPath = c.account.MessagePath(nm.ID) + newID = nm.ID changes = append(changes, nm.ChangeAddUID(), mbDst.ChangeCounts()) if nkeywords != len(mbDst.Keywords) { diff --git a/imapserver/server.go b/imapserver/server.go index a193bfb..810a013 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -3275,24 +3275,19 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) { time time.Time file *os.File // Message file we are appending. Can be nil if we are writing to a nopWriteCloser due to being over quota. - path string // Path if an actual file, either a temporary file, or of the message file stored in the account. mw *message.Writer - m store.Message + m store.Message // New message. Delivered file for m.ID is removed on error. } var appends []*appendMsg var commit bool defer func() { for _, a := range appends { - if a.file != nil { - err := a.file.Close() - c.xsanity(err, "closing APPEND temporary file") - } - - if !commit && a.path != "" { - err := os.Remove(a.path) - c.xsanity(err, "removing APPEND temporary file") + if !commit && a.m.ID != 0 { + p := c.account.MessagePath(a.m.ID) + err := os.Remove(p) + c.xsanity(err, "cleaning up temporary append file after error") } } }() @@ -3385,7 +3380,7 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) { var err error a.file, err = store.CreateMessageTemp(c.log, "imap-append") xcheckf(err, "creating temp file for message") - a.path = a.file.Name() + defer store.CloseRemoveTempFile(c.log, a.file, "temporary message file") f = a.file c.writelinef("+ ") @@ -3398,7 +3393,7 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) { var err error a.file, err = store.CreateMessageTemp(c.log, "imap-append") xcheckf(err, "creating temp file for message") - a.path = a.file.Name() + defer store.CloseRemoveTempFile(c.log, a.file, "temporary message file") f = a.file } } @@ -3483,12 +3478,10 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) { // todo: do a single junk training err = c.account.MessageAdd(c.log, tx, &mb, &a.m, a.file, store.AddOpts{SkipDirSync: true}) xcheckf(err, "delivering message") - // Update path to what is stored in the account. We may still have to clean it up on errors. - a.path = c.account.MessagePath(a.m.ID) changes = append(changes, a.m.ChangeAddUID()) - msgDirs[filepath.Dir(a.path)] = struct{}{} + msgDirs[filepath.Dir(c.account.MessagePath(a.m.ID))] = struct{}{} } changes = append(changes, mb.ChangeCounts()) diff --git a/store/account.go b/store/account.go index 868de75..b510985 100644 --- a/store/account.go +++ b/store/account.go @@ -2043,6 +2043,11 @@ type AddOpts struct { // MessageAdd delivers a mail message to the account. // +// The file is hardlinked or copied, the caller must clean up the original file. If +// this call succeeds, but the database transaction with the change can't be +// committed, the caller must clean up the delivered message file identified by +// m.ID. +// // If the message does not fit in the quota, an error with ErrOverQuota is returned // and the mailbox and message are unchanged and the transaction can continue. For // other errors, the caller must abort the transaction.