Skip to content

Commit

Permalink
feat(GODT-1611): Handle optional charset in SEARCH command
Browse files Browse the repository at this point in the history
The IMAP SEARCH command supports an optional charset. If this is
provided, search terms should be decoded before being compared with
search targets.

In addition, RFC3501 says that search targets should be decoded before
comparison, but this is out of scope for this PR, as it requires parsing
and decoding of messages and addresses.
  • Loading branch information
jameshoulahan committed Aug 5, 2022
1 parent b6d0b58 commit 8a9c450
Show file tree
Hide file tree
Showing 8 changed files with 223 additions and 95 deletions.
175 changes: 108 additions & 67 deletions internal/backend/mailbox_search.go

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions internal/parser/proto/imap.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/parser/proto/imap.proto
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ message Search {
message SearchKey {
SearchKeyword keyword = 1;

string text = 2;
bytes text = 2;
string date = 3;
string flag = 4;
string field = 5;
Expand Down
30 changes: 15 additions & 15 deletions internal/parser/tests/parser/parser_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ TEST_F(ParserTest, SearchNotFrom) {
"flag": "",
"keyword": "SearchKWFrom",
"size": 0,
"text": "Smith"
"text": "U21pdGg="
},
"size": 0,
"text": ""
Expand Down Expand Up @@ -407,7 +407,7 @@ TEST_F(ParserTest, SearchOrFrom) {
"flag": "",
"keyword": "SearchKWFrom",
"size": 0,
"text": "Smith"
"text": "U21pdGg="
},
"rightOp": {
"children": [],
Expand All @@ -416,7 +416,7 @@ TEST_F(ParserTest, SearchOrFrom) {
"flag": "",
"keyword": "SearchKWFrom",
"size": 0,
"text": "Bob"
"text": "Qm9i"
},
"size": 0,
"text": ""
Expand All @@ -440,7 +440,7 @@ TEST_F(ParserTest, SearchBcc) {
"flag": "",
"keyword": "SearchKWBcc",
"size": 0,
"text": "mail@example.com"
"text": "bWFpbEBleGFtcGxlLmNvbQ=="
}
]
}
Expand Down Expand Up @@ -585,7 +585,7 @@ TEST_F(ParserTest, SearchBody) {
"flag": "",
"keyword": "SearchKWBody",
"size": 0,
"text": "some body"
"text": "c29tZSBib2R5"
}
]
}
Expand All @@ -605,7 +605,7 @@ TEST_F(ParserTest, SearchCc) {
"flag": "",
"keyword": "SearchKWCc",
"size": 0,
"text": "mail@example.com"
"text": "bWFpbEBleGFtcGxlLmNvbQ=="
}
]
}
Expand All @@ -626,7 +626,7 @@ TEST_F(ParserTest, SearchFrom) {
"flag": "",
"keyword": "SearchKWFrom",
"size": 0,
"text": "mail@example.com"
"text": "bWFpbEBleGFtcGxlLmNvbQ=="
}
]
}
Expand Down Expand Up @@ -688,7 +688,7 @@ TEST_F(ParserTest, SearchSubject) {
"flag": "",
"keyword": "SearchKWSubject",
"size": 0,
"text": "some subject"
"text": "c29tZSBzdWJqZWN0"
}
]
}
Expand All @@ -708,7 +708,7 @@ TEST_F(ParserTest, SearchText) {
"flag": "",
"keyword": "SearchKWText",
"size": 0,
"text": "some text"
"text": "c29tZSB0ZXh0"
}
]
}
Expand All @@ -729,7 +729,7 @@ TEST_F(ParserTest, SearchTo) {
"flag": "",
"keyword": "SearchKWTo",
"size": 0,
"text": "mail@example.com"
"text": "bWFpbEBleGFtcGxlLmNvbQ=="
}
]
}
Expand Down Expand Up @@ -790,7 +790,7 @@ TEST_F(ParserTest, SearchHeader) {
"flag": "",
"keyword": "SearchKWHeader",
"size": 0,
"text": "string"
"text": "c3RyaW5n"
}
]
}
Expand Down Expand Up @@ -898,7 +898,7 @@ TEST_F(ParserTest, SearchTextWithCharset) {
"flag": "",
"keyword": "SearchKWText",
"size": 0,
"text": "some text"
"text": "c29tZSB0ZXh0"
}
]
}
Expand All @@ -921,7 +921,7 @@ TEST_F(ParserTest, SearchChildren) {
"flag": "",
"keyword": "SearchKWText",
"size": 0,
"text": "some text"
"text": "c29tZSB0ZXh0"
},
{
"children": [],
Expand All @@ -930,7 +930,7 @@ TEST_F(ParserTest, SearchChildren) {
"flag": "",
"keyword": "SearchKWText",
"size": 0,
"text": "some other text"
"text": "c29tZSBvdGhlciB0ZXh0"
}
],
"date": "",
Expand Down Expand Up @@ -962,7 +962,7 @@ TEST_F(ParserTest, SearchLiteralText) {
"flag": "",
"keyword": "SearchKWText",
"size": 0,
"text": "hello!"
"text": "aGVsbG8h"
}
]
}
Expand Down
11 changes: 11 additions & 0 deletions internal/response/item_badcharset.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package response

type itemBadCharset struct{}

func ItemBadCharset() *itemBadCharset {
return &itemBadCharset{}
}

func (c *itemBadCharset) String() string {
return "BADCHARSET"
}
19 changes: 18 additions & 1 deletion internal/session/handle_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,27 @@ import (
"github.com/ProtonMail/gluon/internal/backend"
"github.com/ProtonMail/gluon/internal/parser/proto"
"github.com/ProtonMail/gluon/internal/response"
"golang.org/x/text/encoding"
"golang.org/x/text/encoding/ianaindex"
)

func (s *Session) handleSearch(ctx context.Context, tag string, cmd *proto.Search, mailbox *backend.Mailbox, ch chan response.Response) error {
seq, err := mailbox.Search(ctx, cmd.GetKeys())
var decoder *encoding.Decoder

switch charset := cmd.GetOptionalCharset().(type) {
case *proto.Search_Charset:
encoding, err := ianaindex.IANA.Encoding(charset.Charset)
if err != nil {
return response.No(tag).WithItems(response.ItemBadCharset())
}

decoder = encoding.NewDecoder()

default:
decoder = encoding.Nop.NewDecoder()
}

seq, err := mailbox.Search(ctx, cmd.GetKeys(), decoder)
if err != nil {
return err
}
Expand Down
15 changes: 15 additions & 0 deletions tests/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ func (s *testConnection) C(value string) *testConnection {
return s
}

func (s *testConnection) Cb(b []byte) *testConnection {
n, err := s.conn.Write(append(b, []byte("\r\n")...))
require.NoError(s.tb, err)
require.Greater(s.tb, n, 0)

return s
}

func (s *testConnection) Cf(format string, a ...any) *testConnection {
return s.C(fmt.Sprintf(format, a...))
}
Expand Down Expand Up @@ -119,6 +127,13 @@ func (s *testConnection) Sxe(want ...string) *testConnection {
return s
}

// Continue is a shortcut for a server continuation request.
func (s *testConnection) Continue() *testConnection {
s.Sx("\\+")

return s
}

// OK is a shortcut that we eventually get a tagged OK response of some kind.
func (s *testConnection) OK(tag string, items ...string) {
want := tag + " OK"
Expand Down
56 changes: 50 additions & 6 deletions tests/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,51 @@ import (
"fmt"
"testing"
"time"

"golang.org/x/text/encoding/htmlindex"
)

func TestSearchCharSet(t *testing.T) {
func TestSearchCharSetUTF8(t *testing.T) {
runOneToOneTestWithAuth(t, defaultServerOptions(t), func(c *testConnection, s *testSession) {
c.C(`tag select inbox`).OK("tag")

// Encode "ééé" as UTF-8.
b := enc("ééé", "UTF-8")

// Append a message with that as the body.
c.doAppend("inbox", "To: 1@pm.me\r\n\r\nééé").expect("OK")

// Search for it with UTF-8 encoding.
c.Cf(`TAG SEARCH CHARSET UTF-8 BODY {%v}`, len(b)).Continue().Cb(b).S("* SEARCH 1").OK("TAG")
})
}

func TestSearchCharSetISO88591(t *testing.T) {
runOneToOneTestWithAuth(t, defaultServerOptions(t), func(c *testConnection, s *testSession) {
c.C(`tag select inbox`).OK("tag")

// Encode "ééé" as ISO-8859-1.
b := enc("ééé", "ISO-8859-1")

// Append a message with that as the body.
c.doAppend("inbox", "To: 1@pm.me\r\n\r\nééé").expect("OK")

// Search for it with ISO-8859-1 encoding.
c.Cf(`TAG SEARCH CHARSET ISO-8859-1 BODY {%v}`, len(b)).Continue().Cb(b).S("* SEARCH 1").OK("TAG")
})
}

func TestSearchCharSetASCII(t *testing.T) {
runOneToOneTestWithData(t, defaultServerOptions(t), func(c *testConnection, s *testSession, mbox, mboxID string) {
c.C("A001 search CHARSET UTF-8 TEXT foo")
c.C("A001 search CHARSET US-ASCII TEXT foo")
c.S("* SEARCH 75")
c.OK("A001")
})
}

// TODO: GOMSRV-184 .
func _TestSearchCharSetInvalid(t *testing.T) {
func TestSearchCharSetInvalid(t *testing.T) {
runOneToOneTestWithData(t, defaultServerOptions(t), func(c *testConnection, s *testSession, mbox, mboxID string) {
c.C("A001 search CHARSET invalid-charset TEXT foo")
c.NO("A001")
c.C("A001 search CHARSET invalid-charset TEXT foo").NO("A001", "BADCHARSET")
})
}

Expand Down Expand Up @@ -673,3 +703,17 @@ func TestSearchList(t *testing.T) {
c.OK("A004")
})
}

func enc(text, encoding string) []byte {
enc, err := htmlindex.Get(encoding)
if err != nil {
panic(err)
}

b, err := enc.NewEncoder().Bytes([]byte(text))
if err != nil {
panic(err)
}

return b
}

0 comments on commit 8a9c450

Please sign in to comment.