diff --git a/internal/imap/mailbox_messages.go b/internal/imap/mailbox_messages.go index fe0e9f78..e58842d5 100644 --- a/internal/imap/mailbox_messages.go +++ b/internal/imap/mailbox_messages.go @@ -222,6 +222,20 @@ func (im *imapMailbox) labelMessages(uid bool, seqSet *imap.SeqSet, targetLabel return err } + deletedIDs := []string{} + allDeletedIDs, err := im.storeMailbox.GetDeletedAPIIDs() + if err != nil { + log.WithError(err).Warn("Problem to get deleted API IDs") + } else { + for _, messageID := range messageIDs { + for _, deletedID := range allDeletedIDs { + if messageID == deletedID { + deletedIDs = append(deletedIDs, deletedID) + } + } + } + } + // Label messages first to not lose them. If message is only in trash and we unlabel // it, it will be removed completely and we cannot label it back. if err := targetStoreMailbox.LabelMessages(messageIDs); err != nil { @@ -233,6 +247,13 @@ func (im *imapMailbox) labelMessages(uid bool, seqSet *imap.SeqSet, targetLabel } } + // Preserve \Deleted flag at target location. + if len(deletedIDs) > 0 { + if err := targetStoreMailbox.MarkMessagesDeleted(deletedIDs); err != nil { + log.WithError(err).Warn("Problem to preserve deleted flag for copied messages") + } + } + targetSeqSet := targetStoreMailbox.GetUIDList(messageIDs) return uidplus.CopyResponse(targetStoreMailbox.UIDValidity(), sourceSeqSet, targetSeqSet) } diff --git a/internal/imap/store.go b/internal/imap/store.go index 31e9417f..d20bf10a 100644 --- a/internal/imap/store.go +++ b/internal/imap/store.go @@ -70,6 +70,7 @@ type storeMailboxProvider interface { GetAPIIDsFromSequenceRange(start, stop uint32) ([]string, error) GetLatestAPIID() (string, error) GetNextUID() (uint32, error) + GetDeletedAPIIDs() ([]string, error) GetCounts() (dbTotal, dbUnread, dbUnreadSeqNum uint, err error) GetUIDList(apiIDs []string) *uidplus.OrderedSeq GetUIDByHeader(header *mail.Header) uint32 diff --git a/internal/imap/uidplus/extension.go b/internal/imap/uidplus/extension.go index 3f614573..d226a25b 100644 --- a/internal/imap/uidplus/extension.go +++ b/internal/imap/uidplus/extension.go @@ -114,16 +114,10 @@ func (os *OrderedSeq) String() string { return out } -// UIDExpunge implements server.Handler but has no effect because Bridge is not -// using EXPUNGE at all. The message is deleted right after it was flagged as -// \Deleted Bridge should simply ignore this command with empty `OK` response. -// -// If not implemented it would cause harmless IMAP error. -// -// This overrides the standard EXPUNGE functionality. +// UIDExpunge implements server.Handler but Bridge is not supporting +// UID EXPUNGE with specific UIDs. type UIDExpunge struct { expunge *server.Expunge - seqset *imap.SeqSet } func newUIDExpunge() *UIDExpunge { @@ -131,26 +125,10 @@ func newUIDExpunge() *UIDExpunge { } func (e *UIDExpunge) Parse(fields []interface{}) error { - if len(fields) < 1 { // asuming no UID + if len(fields) < 1 { return e.expunge.Parse(fields) } - var err error - if seqset, ok := fields[0].(string); !ok { - return errors.New("sequence set must be an atom") - } else if e.seqset, err = imap.ParseSeqSet(seqset); err != nil { - return err - } - - return nil -} - -func (e *UIDExpunge) Handle(conn server.Conn) error { - log.Traceln("handle") - return e.expunge.Handle(conn) -} -func (e *UIDExpunge) UidHandle(conn server.Conn) error { //nolint[golint] - log.Traceln("uid handle") // RFC4315#section-2.1 // The UID EXPUNGE command permanently removes all messages that both // have the \Deleted flag set and have a UID that is included in the @@ -159,11 +137,18 @@ func (e *UIDExpunge) UidHandle(conn server.Conn) error { //nolint[golint] // that is not included in the specified sequence set, it is not // affected. // - // NOTE missing implementation: It will probably need mailbox interface - // change: ExpungeUIDs(seqSet) not sure how to combine with original + // Current implementation supports only deletion of all messages + // marked as deleted. It will probably need mailbox interface change: + // ExpungeUIDs(seqSet). Not sure how to combine with original // e.expunge.Handle(). - // - // Current implementation deletes all marked as deleted. + return errors.New("UID EXPUNGE with UIDs is not supported") +} + +func (e *UIDExpunge) Handle(conn server.Conn) error { + return e.expunge.Handle(conn) +} + +func (e *UIDExpunge) UidHandle(conn server.Conn) error { //nolint[golint] return e.expunge.Handle(conn) } diff --git a/internal/store/mailbox_ids.go b/internal/store/mailbox_ids.go index 8c1083c0..18961a4b 100644 --- a/internal/store/mailbox_ids.go +++ b/internal/store/mailbox_ids.go @@ -141,8 +141,8 @@ func (storeMailbox *Mailbox) txGetUIDFromBucket(b *bolt.Bucket, apiID string) (u return btoi(v), nil } -// getUID returns IMAP UID in this mailbox for message ID. -func (storeMailbox *Mailbox) getDeletedAPIIDs() (apiIDs []string, err error) { +// GetDeletedAPIIDs returns API IDs in this mailbox for message ID. +func (storeMailbox *Mailbox) GetDeletedAPIIDs() (apiIDs []string, err error) { err = storeMailbox.db().Update(func(tx *bolt.Tx) error { b := storeMailbox.txGetDeletedIDsBucket(tx) c := b.Cursor() diff --git a/internal/store/mailbox_message.go b/internal/store/mailbox_message.go index 193ff6ab..90043b2c 100644 --- a/internal/store/mailbox_message.go +++ b/internal/store/mailbox_message.go @@ -205,7 +205,7 @@ func (storeMailbox *Mailbox) MarkMessagesUndeleted(apiIDs []string) error { func (storeMailbox *Mailbox) RemoveDeleted() error { storeMailbox.log.Trace("Deleting messages") - apiIDs, err := storeMailbox.getDeletedAPIIDs() + apiIDs, err := storeMailbox.GetDeletedAPIIDs() if err != nil { return err } diff --git a/test/features/bridge/imap/message/copy.feature b/test/features/bridge/imap/message/copy.feature index 15849d45..bda8ca09 100644 --- a/test/features/bridge/imap/message/copy.feature +++ b/test/features/bridge/imap/message/copy.feature @@ -3,6 +3,8 @@ Feature: IMAP copy messages Given there is connected user "user" And there is "user" with mailbox "Folders/mbox" And there is "user" with mailbox "Labels/label" + # Messages are inserted in opposite way to keep increasing ID. + # Sequence numbers are then opposite than listed above. And there are messages in mailbox "INBOX" for "user" | from | to | subject | body | read | deleted | | john.doe@mail.com | user@pm.me | foo | hello | true | false | @@ -18,8 +20,8 @@ Feature: IMAP copy messages | john.doe@mail.com | user@pm.me | foo | hello | true | false | | jane.doe@mail.com | name@pm.me | bar | world | false | true | And mailbox "Labels/label" for "user" has messages - | from | to | subject | body | read | deleted | - | jane.doe@mail.com | name@pm.me | bar | world | false | true | + | from | to | subject | body | read | deleted | + | john.doe@mail.com | user@pm.me | foo | hello | true | false | Scenario: Copy all messages to label When IMAP client copies messages "1:*" to "Labels/label" @@ -37,11 +39,11 @@ Feature: IMAP copy messages When IMAP client copies messages "2" to "Folders/mbox" Then IMAP response is "OK" And mailbox "INBOX" for "user" has messages - | from | to | subject | body | read | deleted | - | john.doe@mail.com | user@pm.me | foo | hello | true | false | - And mailbox "Folders/mbox" for "user" has messages | from | to | subject | body | read | deleted | | jane.doe@mail.com | name@pm.me | bar | world | false | true | + And mailbox "Folders/mbox" for "user" has messages + | from | to | subject | body | read | deleted | + | john.doe@mail.com | user@pm.me | foo | hello | true | false | Scenario: Copy all messages to folder does move When IMAP client copies messages "1:*" to "Folders/mbox" diff --git a/test/features/bridge/imap/message/delete.feature b/test/features/bridge/imap/message/delete.feature index c592d805..f7789129 100644 --- a/test/features/bridge/imap/message/delete.feature +++ b/test/features/bridge/imap/message/delete.feature @@ -15,7 +15,7 @@ Feature: IMAP remove messages from mailbox And IMAP response contains "\* 2 FETCH[ (]*FLAGS \([^)]*\\Deleted" When IMAP client sends expunge Then IMAP response is "OK" - And IMAP response contains "* 2 EXPUNGE" + And IMAP response contains "\* 2 EXPUNGE" And mailbox "" for "user" has 9 messages Examples: @@ -34,11 +34,11 @@ Feature: IMAP remove messages from mailbox Then IMAP response is "OK" When IMAP client sends expunge Then IMAP response is "OK" - And IMAP response contains "* 1 EXPUNGE" - And IMAP response contains "* 2 EXPUNGE" - And IMAP response contains "* 3 EXPUNGE" - And IMAP response contains "* 4 EXPUNGE" - And IMAP response contains "* 5 EXPUNGE" + And IMAP response contains "\* 1 EXPUNGE" + And IMAP response contains "\* 2 EXPUNGE" + And IMAP response contains "\* 3 EXPUNGE" + And IMAP response contains "\* 4 EXPUNGE" + And IMAP response contains "\* 5 EXPUNGE" And mailbox "" for "user" has 0 messages Examples: @@ -59,8 +59,8 @@ Feature: IMAP remove messages from mailbox Then IMAP response is "OK" When IMAP client sends expunge Then IMAP response is "OK" - And IMAP response contains "* 4 EXPUNGE" - And IMAP response contains "* 5 EXPUNGE" + And IMAP response contains "\* 4 EXPUNGE" + And IMAP response contains "\* 5 EXPUNGE" And mailbox "" for "user" has 3 messages Examples: diff --git a/test/features/bridge/imap/message/search.feature b/test/features/bridge/imap/message/search.feature index 72b9c309..687f6c25 100644 --- a/test/features/bridge/imap/message/search.feature +++ b/test/features/bridge/imap/message/search.feature @@ -74,12 +74,12 @@ Feature: IMAP search messages Scenario: Search deleted messages When IMAP client searches for "DELETED" Then IMAP response is "OK" - And IMAP response contains "SEARCH 1 2[^0-9]*$" + And IMAP response contains "SEARCH 1[^0-9]*$" Scenario: Search undeleted messages When IMAP client searches for "UNDELETED" Then IMAP response is "OK" - And IMAP response contains "SEARCH 3[^0-9]*$" + And IMAP response contains "SEARCH 2 3[^0-9]*$" Scenario: Search recent messages When IMAP client searches for "RECENT"