Repository: emersion/go-imap
Pull request #465 (2022-05-19 09:18:05)

search: allow users to select us-ascii charset
Add a UseUSASCII bool flag to the search criteria to indicate that this
charset should be used for the search command. Default charset remains
utf-8.

According to RFC 3501, the search command must only support us-ascii;
support for other charset is optional.

Currently, us-ascii is the fallback charset when the search command with
utf-8 returns a badcharset error. However, this means that we always
execute two search calls to servers that don't support utf-8. More
importantly, it often leads to a server-side disconnect after multiple
badcharset errors. This can be avoid with the proposed flag.

emersion: I'd rather not do this. If the goal is to not send multiple UTF-8 search queries when the server doesn't support them, I'd prefer to store a `utf8SearchUnsupported` flag in `Client`, set when the first search query fails.
konimarti: Thanks for the feedback @emersion I'll change the code and push a new commit with your proposal.

Patch:
diff --git a/client/client.go b/client/client.go
index 8b6fc84..d93c4cb 100644
--- a/client/client.go
+++ b/client/client.go
@@ -83,6 +83,12 @@ type Client struct {
 	// access.
 	locker sync.Mutex
 
+	// This flag is set when the first search query fails with a BADCHARSET
+	// error. Subsequent queries will be performed with the US-ASCII
+	// charset. According to RFC 3501, SEARCH must only support US-ASCII;
+	// other charsets are optional.
+	utf8SearchUnsupported bool
+
 	// A channel to which unilateral updates from the server will be sent. An
 	// update can be one of: *StatusUpdate, *MailboxUpdate, *MessageUpdate,
 	// *ExpungeUpdate. Note that blocking this channel blocks the whole client,
diff --git a/client/cmd_selected.go b/client/cmd_selected.go
index 0fb71ad..40fc513 100644
--- a/client/cmd_selected.go
+++ b/client/cmd_selected.go
@@ -117,10 +117,15 @@ func (c *Client) executeSearch(uid bool, criteria *imap.SearchCriteria, charset
 }
 
 func (c *Client) search(uid bool, criteria *imap.SearchCriteria) (ids []uint32, err error) {
-	ids, status, err := c.executeSearch(uid, criteria, "UTF-8")
+	charset := "UTF-8"
+	if c.utf8SearchUnsupported {
+		charset = "US-ASCII"
+	}
+	ids, status, err := c.executeSearch(uid, criteria, charset)
 	if status != nil && status.Code == imap.CodeBadCharset {
 		// Some servers don't support UTF-8
 		ids, _, err = c.executeSearch(uid, criteria, "US-ASCII")
+		c.utf8SearchUnsupported = true
 	}
 	return
 }


Test patch:
diff --git a/client/cmd_selected_test.go b/client/cmd_selected_test.go
index b51f0e7..6b6b7e1 100644
--- a/client/cmd_selected_test.go
+++ b/client/cmd_selected_test.go
@@ -144,6 +144,86 @@ func TestClient_Search(t *testing.T) {
 	}
 }
 
+func TestClient_Search_Badcharset(t *testing.T) {
+	c, s := newTestClient(t)
+	defer s.Close()
+
+	setClientState(c, imap.SelectedState, nil)
+
+	date, _ := time.Parse(imap.DateLayout, "1-Feb-1994")
+	criteria := &imap.SearchCriteria{
+		WithFlags: []string{imap.DeletedFlag},
+		Header:    textproto.MIMEHeader{"From": {"Smith"}},
+		Since:     date,
+		Not: []*imap.SearchCriteria{{
+			Header: textproto.MIMEHeader{"To": {"Pauline"}},
+		}},
+	}
+
+	done := make(chan error, 1)
+	var results []uint32
+	go func() {
+		var err error
+		results, err = c.Search(criteria)
+		done <- err
+	}()
+
+	// first search call with default UTF-8 charset (assume server does not support UTF-8)
+	wantCmd := `SEARCH CHARSET UTF-8 SINCE "1-Feb-1994" FROM "Smith" DELETED NOT (TO "Pauline")`
+	tag, cmd := s.ScanCmd()
+	if cmd != wantCmd {
+		t.Fatalf("client sent command %v, want %v", cmd, wantCmd)
+	}
+
+	s.WriteString(tag + " NO [BADCHARSET (US-ASCII)]\r\n")
+
+	// internal fall-back to US-ASCII which sets utf8SearchUnsupported = true
+	wantCmd = `SEARCH CHARSET US-ASCII SINCE "1-Feb-1994" FROM "Smith" DELETED NOT (TO "Pauline")`
+	tag, cmd = s.ScanCmd()
+	if cmd != wantCmd {
+		t.Fatalf("client sent command %v, want %v", cmd, wantCmd)
+	}
+
+	s.WriteString("* SEARCH 2 84 882\r\n")
+	s.WriteString(tag + " OK SEARCH completed\r\n")
+
+	if err := <-done; err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	want := []uint32{2, 84, 882}
+	if !reflect.DeepEqual(results, want) {
+		t.Errorf("c.Search() = %v, want %v", results, want)
+	}
+
+	if !c.utf8SearchUnsupported {
+		t.Fatal("client should have utf8SearchUnsupported set to true")
+	}
+
+	// second call to search (with utf8SearchUnsupported=true)
+	go func() {
+		var err error
+		results, err = c.Search(criteria)
+		done <- err
+	}()
+
+	tag, cmd = s.ScanCmd()
+	if cmd != wantCmd {
+		t.Fatalf("client sent command %v, want %v", cmd, wantCmd)
+	}
+
+	s.WriteString("* SEARCH 2 84 882\r\n")
+	s.WriteString(tag + " OK SEARCH completed\r\n")
+
+	if err := <-done; err != nil {
+		t.Fatalf("c.Search() = %v", err)
+	}
+
+	if !reflect.DeepEqual(results, want) {
+		t.Errorf("c.Search() = %v, want %v", results, want)
+	}
+}
+
 func TestClient_Search_Uid(t *testing.T) {
 	c, s := newTestClient(t)
 	defer s.Close()
@@ -181,6 +261,81 @@ func TestClient_Search_Uid(t *testing.T) {
 	}
 }
 
+func TestClient_Search_Uid_Badcharset(t *testing.T) {
+	c, s := newTestClient(t)
+	defer s.Close()
+
+	setClientState(c, imap.SelectedState, nil)
+
+	criteria := &imap.SearchCriteria{
+		WithoutFlags: []string{imap.DeletedFlag},
+	}
+
+	done := make(chan error, 1)
+	var results []uint32
+	go func() {
+		var err error
+		results, err = c.UidSearch(criteria)
+		done <- err
+	}()
+
+	// first search call with default UTF-8 charset (assume server does not support UTF-8)
+	wantCmd := "UID SEARCH CHARSET UTF-8 UNDELETED"
+	tag, cmd := s.ScanCmd()
+	if cmd != wantCmd {
+		t.Fatalf("client sent command %v, want %v", cmd, wantCmd)
+	}
+
+	s.WriteString(tag + " NO [BADCHARSET (US-ASCII)]\r\n")
+
+	// internal fall-back to US-ASCII which sets utf8SearchUnsupported = true
+	wantCmd = "UID SEARCH CHARSET US-ASCII UNDELETED"
+	tag, cmd = s.ScanCmd()
+	if cmd != wantCmd {
+		t.Fatalf("client sent command %v, want %v", cmd, wantCmd)
+	}
+
+	s.WriteString("* SEARCH 1 78 2010\r\n")
+	s.WriteString(tag + " OK UID SEARCH completed\r\n")
+
+	if err := <-done; err != nil {
+		t.Fatalf("c.UidSearch() = %v", err)
+	}
+
+	want := []uint32{1, 78, 2010}
+	if !reflect.DeepEqual(results, want) {
+		t.Errorf("c.UidSearch() = %v, want %v", results, want)
+	}
+
+	if !c.utf8SearchUnsupported {
+		t.Fatal("client should have utf8SearchUnsupported set to true")
+	}
+
+	// second call to search (with utf8SearchUnsupported=true)
+	go func() {
+		var err error
+		results, err = c.UidSearch(criteria)
+		done <- err
+	}()
+
+	tag, cmd = s.ScanCmd()
+	if cmd != wantCmd {
+		t.Fatalf("client sent command %v, want %v", cmd, wantCmd)
+	}
+
+	s.WriteString("* SEARCH 1 78 2010\r\n")
+	s.WriteString(tag + " OK UID SEARCH completed\r\n")
+
+	if err := <-done; err != nil {
+		t.Fatalf("c.UidSearch() = %v", err)
+	}
+
+	if !reflect.DeepEqual(results, want) {
+		t.Errorf("c.UidSearch() = %v, want %v", results, want)
+	}
+
+}
+
 func TestClient_Fetch(t *testing.T) {
 	c, s := newTestClient(t)
 	defer s.Close()