fix: Drop non-UTF-8 commands

internal/session/command.go

@@ -2,7 +2,9 @@ package session
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
+	"unicode/utf8"
 
 	"github.com/bradenaw/juniper/xslices"
 	"golang.org/x/exp/maps"
@@ -35,10 +37,20 @@ func (s *Session) startCommandReader(ctx context.Context, del string) <-chan com
 				return fmt.Sprintf("%v: '%s'", k, literals[k])
 			})...)
 
+			// If the input is not valid UTF-8, we drop the connection.
+			if !utf8.Valid(line) {
+				reporter.MessageWithContext(ctx,
+					"Received invalid UTF-8 command",
+					reporter.Context{"line (base 64)": base64.StdEncoding.EncodeToString(line)},
+				)
+
+				return
+			}
+
 			tag, cmd, err := parse(line, literals, del)
 			if err != nil {
 				reporter.MessageWithContext(ctx,
-					"Failed to parse imap command",
+					"Failed to parse IMAP command",
 					reporter.Context{"error": err},
 				)
 			} else if cmd.GetStartTLS() != nil {

internal/session/handle_search.go

@@ -27,7 +27,7 @@ func (s *Session) handleSearch(ctx context.Context, tag string, cmd *proto.Searc
 	case *proto.Search_Charset:
 		encoding, err := ianaindex.IANA.Encoding(charset.Charset)
 		if err != nil {
-			return response.No(tag).WithItems(response.ItemBadCharset()), nil //nolint:nilerr
+			return nil, response.No(tag).WithItems(response.ItemBadCharset())
 		}
 
 		decoder = encoding.NewDecoder()

tests/bad_test.go

@@ -0,0 +1,29 @@
+package tests
+
+import (
+	"crypto/tls"
+	"reflect"
+	"testing"
+
+	"github.com/bradenaw/juniper/xslices"
+	"github.com/stretchr/testify/require"
+)
+
+// nolint:gosec
+func TestNonUTF8(t *testing.T) {
+	runOneToOneTest(t, defaultServerOptions(t), func(_ *testConnection, s *testSession) {
+		// Create a new connection.
+		c := s.newConnection()
+
+		// Things work fine when the command is valid UTF-8.
+		c.C("tag capability").OK("tag")
+
+		// Performing a TLS handshake should fail; the server will drop the connection.
+		require.Error(t, tls.Client(c.conn, &tls.Config{InsecureSkipVerify: true}).Handshake())
+
+		// We should have reported the bad UTF-8 command.
+		require.True(t, xslices.Any(s.reporter.getReports(), func(report report) bool {
+			return reflect.DeepEqual(report.val, "Received invalid UTF-8 command")
+		}))
+	})
+}

tests/reporter_test.go

@@ -0,0 +1,60 @@
+package tests
+
+import (
+	"sync"
+
+	"github.com/ProtonMail/gluon/reporter"
+)
+
+type testReporter struct {
+	reports []report
+	lock    sync.RWMutex
+}
+
+type report struct {
+	val any
+	ctx reporter.Context
+}
+
+func (r *testReporter) getReports() []report {
+	r.lock.RLock()
+	defer r.lock.RUnlock()
+
+	return r.reports
+}
+
+func (r *testReporter) ReportException(val any) error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+
+	r.reports = append(r.reports, report{val: val})
+
+	return nil
+}
+
+func (r *testReporter) ReportMessage(val string) error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+
+	r.reports = append(r.reports, report{val: val})
+
+	return nil
+}
+
+func (r *testReporter) ReportMessageWithContext(val string, ctx reporter.Context) error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+
+	r.reports = append(r.reports, report{val: val, ctx: ctx})
+
+	return nil
+}
+
+func (r *testReporter) ReportExceptionWithContext(val any, ctx reporter.Context) error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+
+	r.reports = append(r.reports, report{val: val, ctx: ctx})
+
+	return nil
+}

tests/search_test.go

@@ -4,8 +4,10 @@ import (
 	"fmt"
 	"testing"
 	"time"
+	"unicode/utf8"
 
 	"github.com/ProtonMail/gluon/imap"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/text/encoding/htmlindex"
 )
 
@@ -31,11 +33,18 @@ func TestSearchCharSetISO88591(t *testing.T) {
 		// Encode "ééé" as ISO-8859-1.
 		b := enc("ééé", "ISO-8859-1")
 
+		// Assert that b is no longer valid UTF-8.
+		require.False(t, utf8.Valid(b))
+
 		// Append a message with that as the body.
 		c.doAppend("inbox", "To: [email protected]\r\n\r\nééé").expect("OK")
 
-		// Search for it with ISO-8859-1 encoding.
+		// Search for it with ISO-8859-1 encoding (literal).
 		c.Cf(`TAG SEARCH CHARSET ISO-8859-1 BODY {%v}`, len(b)).Continue().Cb(b).S("* SEARCH 1").OK("TAG")
+
+		// Search for it with ISO-8859-1 encoding (direct).
+		// TODO(GODT-2165): This should work, but it doesn't.
+		// c.Cf(`TAG SEARCH CHARSET ISO-8859-1 BODY ` + string(b)).S("* SEARCH 1").OK("TAG")
 	})
 }
 

tests/server_test.go

@@ -202,6 +202,9 @@ func runServer(tb testing.TB, options *serverOptions, tests func(session *testSe
 	loggerOut := logrus.StandardLogger().WriterLevel(logrus.TraceLevel)
 	defer loggerOut.Close()
 
+	// Create a test reporter to capture reported messages.
+	reporter := new(testReporter)
+
 	// Log the (temporary?) directory to store gluon data.
 	logrus.Tracef("Gluon Data Dir: %v", options.dataDir)
 
@@ -227,6 +230,7 @@ func runServer(tb testing.TB, options *serverOptions, tests func(session *testSe
 		),
 		gluon.WithIdleBulkTime(options.idleBulkTime),
 		gluon.WithStoreBuilder(options.storeBuilder),
+		gluon.WithReporter(reporter),
 	}
 
 	if options.disableParallelism {
@@ -282,7 +286,7 @@ func runServer(tb testing.TB, options *serverOptions, tests func(session *testSe
 
 	// Run the test against the server.
 	logging.DoAnnotated(ctx, func(context.Context) {
-		tests(newTestSession(tb, listener, server, eventCh, userIDs, conns, dbPaths, options))
+		tests(newTestSession(tb, listener, server, eventCh, reporter, userIDs, conns, dbPaths, options))
 	}, logging.Labels{
 		"Action": "Running gluon tests",
 	})

tests/session_test.go

@@ -57,6 +57,7 @@ type testSession struct {
 	listener    net.Listener
 	server      *gluon.Server
 	eventCh     <-chan events.Event
+	reporter    *testReporter
 	userIDs     map[string]string
 	conns       map[string]Connector
 	userDBPaths map[string]string
@@ -68,6 +69,7 @@ func newTestSession(
 	listener net.Listener,
 	server *gluon.Server,
 	eventCh <-chan events.Event,
+	reporter *testReporter,
 	userIDs map[string]string,
 	conns map[string]Connector,
 	userDBPaths map[string]string,
@@ -78,6 +80,7 @@ func newTestSession(
 		listener:    listener,
 		server:      server,
 		eventCh:     eventCh,
+		reporter:    reporter,
 		userIDs:     userIDs,
 		conns:       conns,
 		userDBPaths: userDBPaths,