Summary
Several functions in internal/imap/client.go report success even when the target message UIDs aren't present in the selected mailbox. I ran into this when an automation tried to remove a label from an email but used the wrong label name -- it picked a label that exists, but the email wasn't in that folder. The CLI said it removed the label, but nothing changed.
The same class of bug affects DeleteMessages, CopyMessages, and MoveMessages.
Steps to Reproduce
# Email is in Labels/📰 Newsletters but not in Labels/📚 Learning
# Try to remove it from the wrong label
pm-cli mail label remove --label="📚 Learning" uid:1234
# Says "Label removed from 1 message(s)" but nothing happened
Root Cause
DeleteMessages: IMAP STORE on a UID that isn't in the selected mailbox is a valid no-op per RFC 3501. go-imap v2 returns no error. The code calls storeCmd.Close() which drains the response without checking how many messages were actually modified. Store() returns a *FetchCommand that yields one FetchMessageData per modified message via .Next(). If nothing was modified, .Next() returns nil immediately, but the code never checks.
CopyMessages: copyCmd.Wait() returns CopyData with SourceUIDs/DestUIDs indicating what was actually copied, but the return value is discarded. Some servers (Proton Bridge included) return a BAD error for invalid sequence numbers on COPY, but not for UID sets that simply don't match anything.
MoveMessages: Has both issues -- it reimplements COPY + STORE inline without checking either response.
Note: SetFlagsMultiple has the same STORE pattern, but the fix doesn't apply there. The server returns zero affected items both when a UID doesn't exist and when the flag is already set (e.g. starring an already-starred message). A different approach would be needed to distinguish those cases.
Suggested Fix
DeleteMessages: Count results from storeCmd.Next() before Close(), error if zero:
affected := 0
for storeCmd.Next() != nil {
affected++
}
if err := storeCmd.Close(); err != nil {
return fmt.Errorf("failed to mark message(s) for deletion: %w", err)
}
if affected == 0 {
return fmt.Errorf("no messages matched the given ID(s) in %s", mailbox)
}
CopyMessages: Check CopyData.SourceUIDs/DestUIDs after Wait():
copyData, err := copyCmd.Wait()
if err != nil {
return fmt.Errorf("failed to copy messages to %s: %w", destMailbox, err)
}
if copyData != nil && len(copyData.SourceUIDs) == 0 && len(copyData.DestUIDs) == 0 {
return fmt.Errorf("no messages matched the given ID(s) in %s", mailbox)
}
MoveMessages: Same checks applied to both the COPY and STORE steps.
No extra IMAP round-trips needed in any case since the data is already in the response stream.
Environment
- go-imap v2.0.0-beta.8
- Proton Bridge 3.x (IMAP on localhost:1143)
Summary
Several functions in
internal/imap/client.goreport success even when the target message UIDs aren't present in the selected mailbox. I ran into this when an automation tried to remove a label from an email but used the wrong label name -- it picked a label that exists, but the email wasn't in that folder. The CLI said it removed the label, but nothing changed.The same class of bug affects
DeleteMessages,CopyMessages, andMoveMessages.Steps to Reproduce
Root Cause
DeleteMessages: IMAP STORE on a UID that isn't in the selected mailbox is a valid no-op per RFC 3501. go-imap v2 returns no error. The code calls
storeCmd.Close()which drains the response without checking how many messages were actually modified.Store()returns a*FetchCommandthat yields oneFetchMessageDataper modified message via.Next(). If nothing was modified,.Next()returns nil immediately, but the code never checks.CopyMessages:
copyCmd.Wait()returnsCopyDatawithSourceUIDs/DestUIDsindicating what was actually copied, but the return value is discarded. Some servers (Proton Bridge included) return aBADerror for invalid sequence numbers on COPY, but not for UID sets that simply don't match anything.MoveMessages: Has both issues -- it reimplements COPY + STORE inline without checking either response.
Note:
SetFlagsMultiplehas the same STORE pattern, but the fix doesn't apply there. The server returns zero affected items both when a UID doesn't exist and when the flag is already set (e.g. starring an already-starred message). A different approach would be needed to distinguish those cases.Suggested Fix
DeleteMessages: Count results from
storeCmd.Next()beforeClose(), error if zero:CopyMessages: Check
CopyData.SourceUIDs/DestUIDsafterWait():MoveMessages: Same checks applied to both the COPY and STORE steps.
No extra IMAP round-trips needed in any case since the data is already in the response stream.
Environment