-
Notifications
You must be signed in to change notification settings - Fork 331
imapserver: search fixes #712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2
Are you sure you want to change the base?
Changes from all commits
6793216
649289c
d88a0a0
0701eac
876512d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ import ( | |
| "github.com/emersion/go-imap/v2" | ||
| "github.com/emersion/go-imap/v2/internal" | ||
| "github.com/emersion/go-imap/v2/internal/imapwire" | ||
| "golang.org/x/text/cases" | ||
| "golang.org/x/text/language" | ||
| ) | ||
|
|
||
| func (c *Conn) handleSearch(tag string, dec *imapwire.Decoder, numKind NumKind) error { | ||
|
|
@@ -85,7 +87,22 @@ func (c *Conn) handleSearch(tag string, dec *imapwire.Decoder, numKind NumKind) | |
| return err | ||
| } | ||
|
|
||
| if c.enabled.Has(imap.CapIMAP4rev2) || extended { | ||
| var supportsESEARCH bool | ||
| if capSession, ok := c.session.(SessionCapabilities); ok { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this is the only place where I do think it would be nice to be able to customize supported caps per-conn, FWIW. |
||
| sessionCaps := capSession.GetCapabilities() | ||
| supportsESEARCH = sessionCaps.Has(imap.CapESearch) || sessionCaps.Has(imap.CapIMAP4rev2) | ||
| } else { | ||
| availableCaps := c.availableCaps() | ||
| for _, cap := range availableCaps { | ||
| if cap == imap.CapESearch || cap == imap.CapIMAP4rev2 { | ||
| supportsESEARCH = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Use ESEARCH format only if session supports it AND client used extended syntax | ||
| if supportsESEARCH && extended { | ||
| return c.writeESearch(tag, data, &options, numKind) | ||
| } else { | ||
| return c.writeSearch(data.All) | ||
|
|
@@ -98,15 +115,15 @@ func (c *Conn) writeESearch(tag string, data *imap.SearchData, options *imap.Sea | |
|
|
||
| enc.Atom("*").SP().Atom("ESEARCH") | ||
| if tag != "" { | ||
| enc.SP().Special('(').Atom("TAG").SP().Atom(tag).Special(')') | ||
| enc.SP().Special('(').Atom("TAG").SP().String(tag).Special(')') | ||
emersion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| if numKind == NumKindUID { | ||
| enc.SP().Atom("UID") | ||
| } | ||
| // When there is no result, we need to send an ESEARCH response with no ALL | ||
| // keyword | ||
| if options.ReturnAll && !isNumSetEmpty(data.All) { | ||
| enc.SP().Atom("ALL").SP().NumSet(data.All) | ||
|
|
||
| if options.ReturnAll && data.All != nil && !isNumSetEmpty(data.All) { | ||
| enc.SP().Atom("ALL") | ||
| enc.SP().NumSet(data.All) | ||
| } | ||
| if options.ReturnMin && data.Min > 0 { | ||
| enc.SP().Atom("MIN").SP().Number(data.Min) | ||
|
|
@@ -136,24 +153,28 @@ func (c *Conn) writeSearch(numSet imap.NumSet) error { | |
| defer enc.end() | ||
|
|
||
| enc.Atom("*").SP().Atom("SEARCH") | ||
| var ok bool | ||
| switch numSet := numSet.(type) { | ||
| case imap.SeqSet: | ||
| var nums []uint32 | ||
| nums, ok = numSet.Nums() | ||
| for _, num := range nums { | ||
| enc.SP().Number(num) | ||
|
|
||
| if numSet != nil { | ||
| var ok bool | ||
| switch numSet := numSet.(type) { | ||
| case imap.SeqSet: | ||
| var nums []uint32 | ||
| nums, ok = numSet.Nums() | ||
| for _, num := range nums { | ||
| enc.SP().Number(num) | ||
| } | ||
| case imap.UIDSet: | ||
| var uids []imap.UID | ||
| uids, ok = numSet.Nums() | ||
| for _, uid := range uids { | ||
| enc.SP().UID(uid) | ||
| } | ||
| } | ||
| case imap.UIDSet: | ||
| var uids []imap.UID | ||
| uids, ok = numSet.Nums() | ||
| for _, uid := range uids { | ||
| enc.SP().UID(uid) | ||
| if !ok { | ||
| return fmt.Errorf("imapserver: failed to enumerate message numbers in SEARCH response (dynamic set?)") | ||
| } | ||
| } | ||
| if !ok { | ||
| return fmt.Errorf("imapserver: failed to enumerate message numbers in SEARCH response") | ||
| } | ||
|
|
||
| return enc.CRLF() | ||
| } | ||
|
|
||
|
|
@@ -178,7 +199,7 @@ func readSearchReturnOpts(dec *imapwire.Decoder, options *imap.SearchOptions) er | |
| case "SAVE": | ||
| options.ReturnSave = true | ||
| default: | ||
| return newClientBugError("unknown SEARCH RETURN option") | ||
| // RFC 4731: A server MUST ignore any unrecognized return options. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see where this is specified in RFC 4731? This would also be surprising to clients: they ask for something and silently get a reply without it. RFC 4466 says that return options may be followed by optional |
||
| } | ||
| return nil | ||
| }) | ||
|
|
@@ -196,7 +217,15 @@ func readSearchKey(criteria *imap.SearchCriteria, dec *imapwire.Decoder) error { | |
| return readSearchKeyWithAtom(criteria, dec, key) | ||
| } | ||
| return dec.ExpectList(func() error { | ||
| return readSearchKey(criteria, dec) | ||
| for { | ||
| if err := readSearchKey(criteria, dec); err != nil { | ||
| return err | ||
| } | ||
| if !dec.SP() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why stop on non-spaces? |
||
| break | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -241,7 +270,7 @@ func readSearchKeyWithAtom(criteria *imap.SearchCriteria, dec *imapwire.Decoder, | |
| return dec.Err() | ||
| } | ||
| criteria.Header = append(criteria.Header, imap.SearchCriteriaHeaderField{ | ||
| Key: strings.Title(strings.ToLower(key)), | ||
| Key: cases.Title(language.English).String(strings.ToLower(key)), | ||
| Value: value, | ||
| }) | ||
| case "HEADER": | ||
|
|
@@ -308,7 +337,7 @@ func readSearchKeyWithAtom(criteria *imap.SearchCriteria, dec *imapwire.Decoder, | |
| } | ||
| var not imap.SearchCriteria | ||
| if err := readSearchKey(¬, dec); err != nil { | ||
| return nil | ||
| return err | ||
| } | ||
| criteria.Not = append(criteria.Not, not) | ||
| case "OR": | ||
|
|
@@ -317,13 +346,13 @@ func readSearchKeyWithAtom(criteria *imap.SearchCriteria, dec *imapwire.Decoder, | |
| } | ||
| var or [2]imap.SearchCriteria | ||
| if err := readSearchKey(&or[0], dec); err != nil { | ||
| return nil | ||
| return err | ||
| } | ||
| if !dec.ExpectSP() { | ||
| return dec.Err() | ||
| } | ||
| if err := readSearchKey(&or[1], dec); err != nil { | ||
| return nil | ||
| return err | ||
| } | ||
| criteria.Or = append(criteria.Or, or) | ||
| case "$": | ||
|
|
@@ -339,5 +368,5 @@ func readSearchKeyWithAtom(criteria *imap.SearchCriteria, dec *imapwire.Decoder, | |
| } | ||
|
|
||
| func searchKeyFlag(key string) imap.Flag { | ||
| return imap.Flag("\\" + strings.Title(strings.ToLower(key))) | ||
| return imap.Flag("\\" + cases.Title(language.English).String(strings.ToLower(key))) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll never get anything other than ASCII here. I'd prefer not to introduce a new dependency with large Unicode tables just for this. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar from RFC 4731 says that ALL must always be followed by a seq-set:
Is a client not conforming?