implement storing non-system/well-known flags (keywords) for messages and mailboxes, with imap

the mailbox select/examine responses now return all flags used in a mailbox in
the FLAGS response. and indicate in the PERMANENTFLAGS response that clients
can set new keywords. we store these values on the new Message.Keywords field.
system/well-known flags are still in Message.Flags, so we're recognizing those
and handling them separately.

the imap store command handles the new flags. as does the append command, and
the search command.

we store keywords in a mailbox when a message in that mailbox gets the keyword.
we don't automatically remove the keywords from a mailbox. there is currently
no way at all to remove a keyword from a mailbox.

the import commands now handle non-system/well-known keywords too, when
importing from mbox/maildir.

jmap requires keyword support, so best to get it out of the way now.
This commit is contained in:
Mechiel Lukkien
2023-06-24 00:24:43 +02:00
parent afefadf2c0
commit 40163bd145
30 changed files with 1927 additions and 145 deletions

View File

@ -44,14 +44,14 @@ func TestAppend(t *testing.T) {
tc2.transactf("no", "append nobox (\\Seen) \" 1-Jan-2022 10:10:00 +0100\" {1}")
tc2.xcode("TRYCREATE")
tc2.transactf("ok", "append inbox (\\Seen) \" 1-Jan-2022 10:10:00 +0100\" {1+}\r\nx")
tc2.transactf("ok", "append inbox (\\Seen Label1 $label2) \" 1-Jan-2022 10:10:00 +0100\" {1+}\r\nx")
tc2.xuntagged(imapclient.UntaggedExists(1))
tc2.xcodeArg(imapclient.CodeAppendUID{UIDValidity: 1, UID: 1})
tc.transactf("ok", "noop")
uid1 := imapclient.FetchUID(1)
flagsSeen := imapclient.FetchFlags{`\Seen`}
tc.xuntagged(imapclient.UntaggedExists(1), imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, flagsSeen}})
flags := imapclient.FetchFlags{`\Seen`, "label1", "$label2"}
tc.xuntagged(imapclient.UntaggedExists(1), imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, flags}})
tc3.transactf("ok", "noop")
tc3.xuntagged() // Inbox is not selected, nothing to report.

View File

@ -189,12 +189,12 @@ func (cmd *fetchCmd) process(atts []fetchAtt) {
err := cmd.tx.Update(m)
xcheckf(err, "marking message as seen")
cmd.changes = append(cmd.changes, store.ChangeFlags{MailboxID: cmd.mailboxID, UID: cmd.uid, Mask: store.Flags{Seen: true}, Flags: m.Flags})
cmd.changes = append(cmd.changes, store.ChangeFlags{MailboxID: cmd.mailboxID, UID: cmd.uid, Mask: store.Flags{Seen: true}, Flags: m.Flags, Keywords: m.Keywords})
}
if cmd.needFlags {
m := cmd.xensureMessage()
data = append(data, bare("FLAGS"), flaglist(m.Flags))
data = append(data, bare("FLAGS"), flaglist(m.Flags, m.Keywords))
}
// Write errors are turned into panics because we write through c.

View File

@ -376,8 +376,18 @@ func (p *parser) remainder() string {
return p.orig[p.o:]
}
// ../rfc/9051:6565
func (p *parser) xflag() string {
return p.xtakelist(`\`, "$") + p.xatom()
w, _ := p.takelist(`\`, "$")
s := w + p.xatom()
if s[0] == '\\' {
switch strings.ToLower(s) {
case `\answered`, `\flagged`, `\deleted`, `\seen`, `\draft`:
default:
p.xerrorf("unknown system flag %s", s)
}
}
return s
}
func (p *parser) xflagList() (l []string) {

View File

@ -309,19 +309,24 @@ func (s *search) match(sk searchKey) bool {
case "FLAGGED":
return s.m.Flagged
case "KEYWORD":
switch sk.atom {
case "$Forwarded":
kw := strings.ToLower(sk.atom)
switch kw {
case "$forwarded":
return s.m.Forwarded
case "$Junk":
case "$junk":
return s.m.Junk
case "$NotJunk":
case "$notjunk":
return s.m.Notjunk
case "$Phishing":
case "$phishing":
return s.m.Phishing
case "$MDNSent":
case "$mdnsent":
return s.m.MDNSent
default:
c.log.Info("search with unknown keyword", mlog.Field("keyword", sk.atom))
for _, k := range s.m.Keywords {
if k == kw {
return true
}
}
return false
}
case "SEEN":
@ -333,20 +338,25 @@ func (s *search) match(sk searchKey) bool {
case "UNFLAGGED":
return !s.m.Flagged
case "UNKEYWORD":
switch sk.atom {
case "$Forwarded":
kw := strings.ToLower(sk.atom)
switch kw {
case "$forwarded":
return !s.m.Forwarded
case "$Junk":
case "$junk":
return !s.m.Junk
case "$NotJunk":
case "$notjunk":
return !s.m.Notjunk
case "$Phishing":
case "$phishing":
return !s.m.Phishing
case "$MDNSent":
case "$mdnsent":
return !s.m.MDNSent
default:
c.log.Info("search with unknown keyword", mlog.Field("keyword", sk.atom))
return false
for _, k := range s.m.Keywords {
if k == kw {
return false
}
}
return true
}
case "UNSEEN":
return !s.m.Seen

View File

@ -79,6 +79,8 @@ func TestSearch(t *testing.T) {
`$Notjunk`,
`$Phishing`,
`$MDNSent`,
`custom1`,
`Custom2`,
}
tc.client.Append("inbox", mostFlags, &received, []byte(searchMsg))
@ -123,6 +125,12 @@ func TestSearch(t *testing.T) {
tc.transactf("ok", `search keyword $Forwarded`)
tc.xsearch(3)
tc.transactf("ok", `search keyword Custom1`)
tc.xsearch(3)
tc.transactf("ok", `search keyword custom2`)
tc.xsearch(3)
tc.transactf("ok", `search new`)
tc.xsearch() // New requires a message to be recent. We pretend all messages are not recent.
@ -162,6 +170,9 @@ func TestSearch(t *testing.T) {
tc.transactf("ok", `search unkeyword $Junk`)
tc.xsearch(1, 2)
tc.transactf("ok", `search unkeyword custom1`)
tc.xsearch(1, 2)
tc.transactf("ok", `search unseen`)
tc.xsearch(1, 2)

View File

@ -32,8 +32,9 @@ func testSelectExamine(t *testing.T, examine bool) {
uclosed := imapclient.UntaggedResult{Status: imapclient.OK, RespText: imapclient.RespText{Code: "CLOSED", More: "x"}}
flags := strings.Split(`\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent`, " ")
permflags := strings.Split(`\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent \*`, " ")
uflags := imapclient.UntaggedFlags(flags)
upermflags := imapclient.UntaggedResult{Status: imapclient.OK, RespText: imapclient.RespText{Code: "PERMANENTFLAGS", CodeArg: imapclient.CodeList{Code: "PERMANENTFLAGS", Args: flags}, More: "x"}}
upermflags := imapclient.UntaggedResult{Status: imapclient.OK, RespText: imapclient.RespText{Code: "PERMANENTFLAGS", CodeArg: imapclient.CodeList{Code: "PERMANENTFLAGS", Args: permflags}, More: "x"}}
urecent := imapclient.UntaggedRecent(0)
uexists0 := imapclient.UntaggedExists(0)
uexists1 := imapclient.UntaggedExists(1)

View File

@ -1233,7 +1233,7 @@ func (c *conn) applyChanges(changes []store.Change, initial bool) {
c.bwritelinef("* %d EXISTS", len(c.uids))
for _, add := range adds {
seq := c.xsequence(add.UID)
c.bwritelinef("* %d FETCH (UID %d FLAGS %s)", seq, add.UID, flaglist(add.Flags).pack(c))
c.bwritelinef("* %d FETCH (UID %d FLAGS %s)", seq, add.UID, flaglist(add.Flags, add.Keywords).pack(c))
}
continue
}
@ -1265,7 +1265,7 @@ func (c *conn) applyChanges(changes []store.Change, initial bool) {
continue
}
if !initial {
c.bwritelinef("* %d FETCH (UID %d FLAGS %s)", seq, ch.UID, flaglist(ch.Flags).pack(c))
c.bwritelinef("* %d FETCH (UID %d FLAGS %s)", seq, ch.UID, flaglist(ch.Flags, ch.Keywords).pack(c))
}
case store.ChangeRemoveMailbox:
// Only announce \NonExistent to modern clients, otherwise they may ignore the
@ -1862,8 +1862,12 @@ func (c *conn) cmdSelectExamine(isselect bool, tag, cmd string, p *parser) {
})
c.applyChanges(c.comm.Get(), true)
c.bwritelinef(`* FLAGS (\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent)`)
c.bwritelinef(`* OK [PERMANENTFLAGS (\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent)] x`)
var flags string
if len(mb.Keywords) > 0 {
flags = " " + strings.Join(mb.Keywords, " ")
}
c.bwritelinef(`* FLAGS (\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent%s)`, flags)
c.bwritelinef(`* OK [PERMANENTFLAGS (\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent \*)] x`)
if !c.enabled[capIMAP4rev2] {
c.bwritelinef(`* 0 RECENT`)
}
@ -2436,7 +2440,7 @@ func (c *conn) xstatusLine(tx *bstore.Tx, mb store.Mailbox, attrs []string) stri
return fmt.Sprintf("* STATUS %s (%s)", astring(mb.Name).pack(c), strings.Join(status, " "))
}
func xparseStoreFlags(l []string, syntax bool) (flags store.Flags) {
func xparseStoreFlags(l []string, syntax bool) (flags store.Flags, keywords []string) {
fields := map[string]*bool{
`\answered`: &flags.Answered,
`\flagged`: &flags.Flagged,
@ -2449,20 +2453,24 @@ func xparseStoreFlags(l []string, syntax bool) (flags store.Flags) {
`$phishing`: &flags.Phishing,
`$mdnsent`: &flags.MDNSent,
}
seen := map[string]bool{}
for _, f := range l {
if field, ok := fields[strings.ToLower(f)]; !ok {
if syntax {
xsyntaxErrorf("unknown flag %q", f)
}
xuserErrorf("unknown flag %q", f)
} else {
f = strings.ToLower(f)
if field, ok := fields[f]; ok {
*field = true
} else if seen[f] {
if moxvar.Pedantic {
xuserErrorf("duplicate keyword %s", f)
}
} else {
keywords = append(keywords, f)
seen[f] = true
}
}
return
}
func flaglist(fl store.Flags) listspace {
func flaglist(fl store.Flags, keywords []string) listspace {
l := listspace{}
flag := func(v bool, s string) {
if v {
@ -2479,6 +2487,9 @@ func flaglist(fl store.Flags) listspace {
flag(fl.Notjunk, `$NotJunk`)
flag(fl.Phishing, `$Phishing`)
flag(fl.MDNSent, `$MDNSent`)
for _, k := range keywords {
l = append(l, bare(k))
}
return l
}
@ -2494,9 +2505,10 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) {
name := p.xmailbox()
p.xspace()
var storeFlags store.Flags
var keywords []string
if p.hasPrefix("(") {
// Error must be a syntax error, to properly abort the connection due to literal.
storeFlags = xparseStoreFlags(p.xflagList(), true)
storeFlags, keywords = xparseStoreFlags(p.xflagList(), true)
p.xspace()
}
var tm time.Time
@ -2570,11 +2582,21 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) {
c.account.WithWLock(func() {
c.xdbwrite(func(tx *bstore.Tx) {
mb = c.xmailbox(tx, name, "TRYCREATE")
// Ensure keywords are stored in mailbox.
var changed bool
mb.Keywords, changed = store.MergeKeywords(mb.Keywords, keywords)
if changed {
err := tx.Update(&mb)
xcheckf(err, "updating keywords in mailbox")
}
msg = store.Message{
MailboxID: mb.ID,
MailboxOrigID: mb.ID,
Received: tm,
Flags: storeFlags,
Keywords: keywords,
Size: size,
MsgPrefix: msgPrefix,
}
@ -2589,7 +2611,7 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) {
}
// Broadcast the change to other connections.
c.broadcast([]store.Change{store.ChangeAddUID{MailboxID: mb.ID, UID: msg.UID, Flags: msg.Flags}})
c.broadcast([]store.Change{store.ChangeAddUID{MailboxID: mb.ID, UID: msg.UID, Flags: msg.Flags, Keywords: msg.Keywords}})
})
err = msgFile.Close()
@ -2952,6 +2974,7 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
var mbDst store.Mailbox
var origUIDs, newUIDs []store.UID
var flags []store.Flags
var keywords [][]string
c.account.WithWLock(func() {
c.xdbwrite(func(tx *bstore.Tx) {
@ -3017,6 +3040,7 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
newUIDs = append(newUIDs, m.UID)
newMsgIDs = append(newMsgIDs, m.ID)
flags = append(flags, m.Flags)
keywords = append(keywords, m.Keywords)
qmr := bstore.QueryTx[store.Recipient](tx)
qmr.FilterNonzero(store.Recipient{MessageID: origID})
@ -3048,7 +3072,7 @@ func (c *conn) cmdxCopy(isUID bool, tag, cmd string, p *parser) {
if len(newUIDs) > 0 {
changes := make([]store.Change, len(newUIDs))
for i, uid := range newUIDs {
changes[i] = store.ChangeAddUID{MailboxID: mbDst.ID, UID: uid, Flags: flags[i]}
changes[i] = store.ChangeAddUID{MailboxID: mbDst.ID, UID: uid, Flags: flags[i], Keywords: keywords[i]}
}
c.broadcast(changes)
}
@ -3187,7 +3211,7 @@ func (c *conn) cmdxMove(isUID bool, tag, cmd string, p *parser) {
changes = append(changes, store.ChangeRemoveUIDs{MailboxID: c.mailboxID, UIDs: uids})
for _, m := range msgs {
newUIDs = append(newUIDs, m.UID)
changes = append(changes, store.ChangeAddUID{MailboxID: mbDst.ID, UID: m.UID, Flags: m.Flags})
changes = append(changes, store.ChangeAddUID{MailboxID: mbDst.ID, UID: m.UID, Flags: m.Flags, Keywords: m.Keywords})
}
})
@ -3240,25 +3264,21 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
xuserErrorf("mailbox open in read-only mode")
}
var mask, flags store.Flags
flags, keywords := xparseStoreFlags(flagstrs, false)
var mask store.Flags
if plus {
mask = xparseStoreFlags(flagstrs, false)
flags = store.FlagsAll
mask, flags = flags, store.FlagsAll
} else if minus {
mask = xparseStoreFlags(flagstrs, false)
flags = store.Flags{}
mask, flags = flags, store.Flags{}
} else {
mask = store.FlagsAll
flags = xparseStoreFlags(flagstrs, false)
}
updates := store.FlagsQuerySet(mask, flags)
var updated []store.Message
c.account.WithWLock(func() {
c.xdbwrite(func(tx *bstore.Tx) {
c.xmailboxID(tx, c.mailboxID) // Validate.
mb := c.xmailboxID(tx, c.mailboxID) // Validate.
uidargs := c.xnumSetCondition(isUID, nums)
@ -3266,27 +3286,41 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
return
}
// Ensure keywords are in mailbox.
if !minus {
var changed bool
mb.Keywords, changed = store.MergeKeywords(mb.Keywords, keywords)
if changed {
err := tx.Update(&mb)
xcheckf(err, "updating mailbox with keywords")
}
}
q := bstore.QueryTx[store.Message](tx)
q.FilterNonzero(store.Message{MailboxID: c.mailboxID})
q.FilterEqual("UID", uidargs...)
if len(updates) == 0 {
var err error
updated, err = q.List()
xcheckf(err, "listing for flags")
} else {
q.Gather(&updated)
_, err := q.UpdateFields(updates)
xcheckf(err, "updating flags")
}
err := q.ForEach(func(m store.Message) error {
m.Flags = m.Flags.Set(mask, flags)
if minus {
m.Keywords = store.RemoveKeywords(m.Keywords, keywords)
} else if plus {
m.Keywords, _ = store.MergeKeywords(m.Keywords, keywords)
} else {
m.Keywords = keywords
}
updated = append(updated, m)
return tx.Update(&m)
})
xcheckf(err, "storing flags in messages")
err := c.account.RetrainMessages(context.TODO(), c.log, tx, updated, false)
err = c.account.RetrainMessages(context.TODO(), c.log, tx, updated, false)
xcheckf(err, "training messages")
})
// Broadcast changes to other connections.
changes := make([]store.Change, len(updated))
for i, m := range updated {
changes[i] = store.ChangeFlags{MailboxID: m.MailboxID, UID: m.UID, Mask: mask, Flags: m.Flags}
changes[i] = store.ChangeFlags{MailboxID: m.MailboxID, UID: m.UID, Mask: mask, Flags: m.Flags, Keywords: m.Keywords}
}
c.broadcast(changes)
})
@ -3294,7 +3328,7 @@ func (c *conn) cmdxStore(isUID bool, tag, cmd string, p *parser) {
for _, m := range updated {
if !silent {
// ../rfc/9051:6749 ../rfc/3501:4869
c.bwritelinef("* %d FETCH (UID %d FLAGS %s)", c.xsequence(m.UID), m.UID, flaglist(m.Flags).pack(c))
c.bwritelinef("* %d FETCH (UID %d FLAGS %s)", c.xsequence(m.UID), m.UID, flaglist(m.Flags, m.Keywords).pack(c))
}
}

View File

@ -191,6 +191,10 @@ func (tc *testconn) xcodeArg(v any) {
}
func (tc *testconn) xuntagged(exps ...any) {
tc.xuntaggedCheck(true, exps...)
}
func (tc *testconn) xuntaggedCheck(all bool, exps ...any) {
tc.t.Helper()
last := append([]imapclient.Untagged{}, tc.lastUntagged...)
next:
@ -212,7 +216,7 @@ next:
}
tc.t.Fatalf("did not find untagged response %#v %T (%d) in %v%s", exp, exp, ei, tc.lastUntagged, next)
}
if len(last) > 0 {
if len(last) > 0 && all {
tc.t.Fatalf("leftover untagged responses %v", last)
}
}
@ -525,7 +529,7 @@ func TestScenario(t *testing.T) {
tc.transactf("ok", `store 1 flags.silent (\seen \answered)`)
tc.transactf("ok", `store 1 -flags.silent (\answered)`)
tc.transactf("ok", `store 1 +flags.silent (\answered)`)
tc.transactf("no", `store 1 flags (\badflag)`)
tc.transactf("bad", `store 1 flags (\badflag)`)
tc.transactf("ok", "noop")
tc.transactf("ok", "copy 1 Trash")

View File

@ -1,6 +1,7 @@
package imapserver
import (
"strings"
"testing"
"github.com/mjl-/mox/imapclient"
@ -54,15 +55,30 @@ func TestStore(t *testing.T) {
tc.transactf("ok", "uid store 1 flags ()")
tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, noflags}})
tc.transactf("bad", "store") // Need numset, flags and args.
tc.transactf("bad", "store 1") // Need flags.
tc.transactf("bad", "store 1 +") // Need flags.
tc.transactf("bad", "store 1 -") // Need flags.
tc.transactf("bad", "store 1 flags ") // Need flags.
tc.transactf("bad", "store 1 flags ") // Need flags.
tc.transactf("bad", "store 1 flags (bogus)") // Unknown flag.
tc.transactf("ok", "store 1 flags (new)") // New flag.
tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, imapclient.FetchFlags{"new"}}})
tc.transactf("ok", "store 1 flags (new new a b c)") // Duplicates are ignored.
tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, imapclient.FetchFlags{"new", "a", "b", "c"}}})
tc.transactf("ok", "store 1 +flags (new new c d e)")
tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, imapclient.FetchFlags{"new", "a", "b", "c", "d", "e"}}})
tc.transactf("ok", "store 1 -flags (new new e a c)")
tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, imapclient.FetchFlags{"b", "d"}}})
tc.transactf("ok", "store 1 flags ($Forwarded Different)")
tc.xuntagged(imapclient.UntaggedFetch{Seq: 1, Attrs: []imapclient.FetchAttr{uid1, imapclient.FetchFlags{"$Forwarded", "different"}}})
tc.transactf("bad", "store") // Need numset, flags and args.
tc.transactf("bad", "store 1") // Need flags.
tc.transactf("bad", "store 1 +") // Need flags.
tc.transactf("bad", "store 1 -") // Need flags.
tc.transactf("bad", "store 1 flags ") // Need flags.
tc.transactf("bad", "store 1 flags ") // Need flags.
tc.client.Unselect()
tc.client.Examine("inbox") // Open read-only.
tc.transactf("ok", "examine inbox") // Open read-only.
// Flags are added to mailbox, not removed.
flags := strings.Split(`\Seen \Answered \Flagged \Deleted \Draft $Forwarded $Junk $NotJunk $Phishing $MDNSent new a b c d e different`, " ")
tc.xuntaggedCheck(false, imapclient.UntaggedFlags(flags))
tc.transactf("no", `store 1 flags ()`) // No permission to set flags.
}