fix(GOMSRV-128): Remover server personality

The emoticon personality was interfering with the IMAP tests from the dovecot suit. These have now been removed. Since the responses require a bit of text after, we have added the originating command's name where no other message exited before. This requirement was previously masked by the emoticon response.
ProtonMail · Jul 11, 2022 · dd05009 · dd05009
1 parent 3134478
commit dd05009
Show file tree

Hide file tree

Showing 55 changed files with 289 additions and 307 deletions.
diff --git a/internal/response/bad.go b/internal/response/bad.go
@@ -35,10 +35,6 @@ func (r *bad) String() string {
 		parts = append(parts, r.err.Error())
 	}
 
-	if r.tag != "*" {
-		parts = append(parts, faceBad)
-	}
-
 	return join(parts)
 }
 

diff --git a/internal/response/bad_test.go b/internal/response/bad_test.go
@@ -12,9 +12,9 @@ func TestBadUntagged(t *testing.T) {
 }
 
 func TestBadTagged(t *testing.T) {
-	assert.Equal(t, "tag BAD (>_<)", Bad("tag").String())
+	assert.Equal(t, "tag BAD", Bad("tag").String())
 }
 
 func TestBadError(t *testing.T) {
-	assert.Equal(t, "tag BAD erroooooor (>_<)", Bad("tag").WithError(errors.New("erroooooor")).String())
+	assert.Equal(t, "tag BAD erroooooor", Bad("tag").WithError(errors.New("erroooooor")).String())
 }
diff --git a/internal/response/bye.go b/internal/response/bye.go
@@ -18,7 +18,7 @@ func (r *bye) Send(s Session) error {
 }
 
 func (r *bye) String() string {
-	parts := []string{"*", "BYE", faceBye}
+	parts := []string{"*", "BYE"}
 
 	if r.msg != "" {
 		parts = append(parts, r.msg)

diff --git a/internal/response/bye_test.go b/internal/response/bye_test.go
@@ -7,9 +7,9 @@ import (
 )
 
 func TestBye(t *testing.T) {
-	assert.Equal(t, "* BYE (^_^)/~", Bye().String())
+	assert.Equal(t, "* BYE", Bye().String())
 }
 
 func TestByeMessage(t *testing.T) {
-	assert.Equal(t, "* BYE (^_^)/~ message", Bye().WithMessage("message").String())
+	assert.Equal(t, "* BYE message", Bye().WithMessage("message").String())
 }
diff --git a/internal/response/continuation.go b/internal/response/continuation.go
@@ -1,5 +1,7 @@
 package response
 
+import "strings"
+
 type continuation struct {
 	tag string
 }
@@ -15,5 +17,5 @@ func (r *continuation) Send(s Session) error {
 }
 
 func (r *continuation) String() string {
-	return join([]string{r.tag, faceCon})
+	return strings.Join([]string{r.tag, "Ready"}, " ")
 }
diff --git a/internal/response/continuation_test.go b/internal/response/continuation_test.go
@@ -7,5 +7,5 @@ import (
 )
 
 func TestContinuation(t *testing.T) {
-	assert.Equal(t, "+ (*_*)", Continuation().String())
+	assert.Equal(t, "+ Ready", Continuation().String())
 }
diff --git a/internal/response/format.go b/internal/response/format.go
@@ -2,14 +2,6 @@ package response
 
 import "strings"
 
-const (
-	faceOk  = "(^_^)"
-	faceNo  = "(~_~)"
-	faceBad = "(>_<)"
-	faceBye = "(^_^)/~"
-	faceCon = "(*_*)"
-)
-
 func join(items []string, withDel ...string) string {
 	var del string
 

diff --git a/internal/response/no.go b/internal/response/no.go
@@ -53,10 +53,6 @@ func (r *no) String() (res string) {
 		parts = append(parts, r.err.Error())
 	}
 
-	if r.tag != "*" {
-		parts = append(parts, faceNo)
-	}
-
 	return join(parts)
 }
 

diff --git a/internal/response/no_test.go b/internal/response/no_test.go
@@ -12,13 +12,13 @@ func TestNoUntagged(t *testing.T) {
 }
 
 func TestNoTagged(t *testing.T) {
-	assert.Equal(t, "tag NO (~_~)", No("tag").String())
+	assert.Equal(t, "tag NO", No("tag").String())
 }
 
 func TestNoError(t *testing.T) {
-	assert.Equal(t, "tag NO erroooooor (~_~)", No("tag").WithError(errors.New("erroooooor")).String())
+	assert.Equal(t, "tag NO erroooooor", No("tag").WithError(errors.New("erroooooor")).String())
 }
 
 func TestNoTryCreate(t *testing.T) {
-	assert.Equal(t, "tag NO [TRYCREATE] erroooooor (~_~)", No("tag").WithItems(ItemTryCreate()).WithError(errors.New("erroooooor")).String())
+	assert.Equal(t, "tag NO [TRYCREATE] erroooooor", No("tag").WithItems(ItemTryCreate()).WithError(errors.New("erroooooor")).String())
 }
diff --git a/internal/response/ok.go b/internal/response/ok.go
@@ -51,10 +51,6 @@ func (r *ok) String() string {
 		parts = append(parts, fmt.Sprintf("[%v]", join(items)))
 	}
 
-	if r.tag != "*" {
-		parts = append(parts, faceOk)
-	}
-
 	if r.msg != "" {
 		parts = append(parts, r.msg)
 	}

diff --git a/internal/response/ok_test.go b/internal/response/ok_test.go
@@ -13,7 +13,7 @@ func TestOkUntagged(t *testing.T) {
 }
 
 func TestOkTagged(t *testing.T) {
-	assert.Equal(t, `tag OK (^_^)`, Ok("tag").String())
+	assert.Equal(t, `tag OK`, Ok("tag").String())
 }
 
 func TestOkUnseen(t *testing.T) {

diff --git a/internal/session/handle_capability.go b/internal/session/handle_capability.go
@@ -13,7 +13,7 @@ func (s *Session) handleCapability(ctx context.Context, tag string, cmd *proto.C
 
 	ch <- response.Capability().WithCapabilities(s.caps...)
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("CAPABILITY")
 
 	return nil
 }
diff --git a/internal/session/handle_check.go b/internal/session/handle_check.go
@@ -13,7 +13,7 @@ func (s *Session) handleCheck(ctx context.Context, tag string, cmd *proto.Check,
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("CHECK")
 
 	return nil
 }
diff --git a/internal/session/handle_close.go b/internal/session/handle_close.go
@@ -28,7 +28,7 @@ func (s *Session) handleClose(ctx context.Context, tag string, cmd *proto.Close,
 
 	s.name = ""
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("CLOSE")
 
 	return nil
 }
diff --git a/internal/session/handle_create.go b/internal/session/handle_create.go
@@ -24,7 +24,7 @@ func (s *Session) handleCreate(ctx context.Context, tag string, cmd *proto.Creat
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("CREATE")
 
 	return nil
 }
diff --git a/internal/session/handle_delete.go b/internal/session/handle_delete.go
@@ -24,7 +24,7 @@ func (s *Session) handleDelete(ctx context.Context, tag string, cmd *proto.Del,
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("DELETE")
 
 	return nil
 }
diff --git a/internal/session/handle_examine.go b/internal/session/handle_examine.go
@@ -50,7 +50,7 @@ func (s *Session) handleExamine(ctx context.Context, tag string, cmd *proto.Exam
 		return err
 	}
 
-	ch <- response.Ok(tag).WithItems(response.ItemReadOnly())
+	ch <- response.Ok(tag).WithItems(response.ItemReadOnly()).WithMessage("EXAMINE")
 
 	return nil
 }
diff --git a/internal/session/handle_expunge.go b/internal/session/handle_expunge.go
@@ -21,7 +21,7 @@ func (s *Session) handleExpunge(ctx context.Context, tag string, cmd *proto.Expu
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("EXPUNGE")
 
 	return nil
 }
@@ -41,7 +41,7 @@ func (s *Session) handleUIDExpunge(ctx context.Context, tag string, cmd *proto.U
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("EXPUNGE")
 
 	return nil
 }
diff --git a/internal/session/handle_id.go b/internal/session/handle_id.go
@@ -17,7 +17,7 @@ func prepareServerResponse(info *internal.VersionInfo) response.Response {
 
 func (s *Session) handleIDGet(ctx context.Context, tag string, ch chan response.Response) error {
 	ch <- prepareServerResponse(s.version)
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("ID")
 
 	return nil
 }
@@ -34,7 +34,7 @@ func (s *Session) handleIDSet(ctx context.Context, tag string, cmd *proto.IDSet,
 	}
 
 	ch <- prepareServerResponse(s.version)
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("ID")
 
 	return nil
 }
diff --git a/internal/session/handle_idle.go b/internal/session/handle_idle.go
@@ -59,7 +59,7 @@ func (s *Session) handleIdle(ctx context.Context, tag string, cmd *proto.Idle, c
 
 			switch {
 			case cmd.GetDone() != nil:
-				return response.Ok(tag).Send(s)
+				return response.Ok(tag).WithMessage("IDLE").Send(s)
 
 			default:
 				return response.Bad(tag).Send(s)

diff --git a/internal/session/handle_list.go b/internal/session/handle_list.go
@@ -29,7 +29,7 @@ func (s *Session) handleList(ctx context.Context, tag string, cmd *proto.List, c
 				WithAttributes(match.Atts)
 		}
 
-		ch <- response.Ok(tag)
+		ch <- response.Ok(tag).WithMessage("LIST")
 
 		return nil
 	})

diff --git a/internal/session/handle_logout.go b/internal/session/handle_logout.go
@@ -18,7 +18,7 @@ func (s *Session) handleLogout(ctx context.Context, tag string, cmd *proto.Logou
 		return err
 	}
 
-	if err := response.Ok(tag).Send(s); err != nil {
+	if err := response.Ok(tag).WithMessage("LOGOUT").Send(s); err != nil {
 		return err
 	}
 

diff --git a/internal/session/handle_lsub.go b/internal/session/handle_lsub.go
@@ -29,7 +29,7 @@ func (s *Session) handleLsub(ctx context.Context, tag string, cmd *proto.Lsub, c
 				WithAttributes(match.Atts)
 		}
 
-		ch <- response.Ok(tag)
+		ch <- response.Ok(tag).WithMessage("LSUB")
 
 		return nil
 	})

diff --git a/internal/session/handle_rename.go b/internal/session/handle_rename.go
@@ -29,7 +29,7 @@ func (s *Session) handleRename(ctx context.Context, tag string, cmd *proto.Renam
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("RENAME")
 
 	return nil
 }
diff --git a/internal/session/handle_select.go b/internal/session/handle_select.go
@@ -51,7 +51,7 @@ func (s *Session) handleSelect(ctx context.Context, tag string, cmd *proto.Selec
 		return err
 	}
 
-	ch <- response.Ok(tag).WithItems(response.ItemReadWrite())
+	ch <- response.Ok(tag).WithItems(response.ItemReadWrite()).WithMessage("SELECT")
 
 	s.eventCh <- events.EventSelect{
 		SessionID: s.sessionID,

diff --git a/internal/session/handle_status.go b/internal/session/handle_status.go
@@ -56,7 +56,7 @@ func (s *Session) handleStatus(ctx context.Context, tag string, cmd *proto.Statu
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("STATUS")
 
 	return nil
 }
diff --git a/internal/session/handle_sub.go b/internal/session/handle_sub.go
@@ -24,7 +24,7 @@ func (s *Session) handleSub(ctx context.Context, tag string, cmd *proto.Sub, ch
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("SUB")
 
 	return nil
 }
diff --git a/internal/session/handle_unselect.go b/internal/session/handle_unselect.go
@@ -15,7 +15,7 @@ func (s *Session) handleUnselect(ctx context.Context, tag string, cmd *proto.Uns
 
 	s.name = ""
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("UNSELECT")
 
 	return nil
 }
diff --git a/internal/session/handle_unsub.go b/internal/session/handle_unsub.go
@@ -24,7 +24,7 @@ func (s *Session) handleUnsub(ctx context.Context, tag string, cmd *proto.Unsub,
 		return err
 	}
 
-	ch <- response.Ok(tag)
+	ch <- response.Ok(tag).WithMessage("UNSUBSCRIBE")
 
 	return nil
 }
diff --git a/tests/close_test.go b/tests/close_test.go
@@ -16,7 +16,7 @@ func TestClose(t *testing.T) {
 	// There is currently no way to check for this with the go imap client.
 	runOneToOneTestWithAuth(t, defaultServerOptions(t), func(c *testConnection, _ *testSession) {
 		c.C("b001 CREATE saved-messages")
-		c.S("b001 OK (^_^)")
+		c.S("b001 OK CREATE")
 
 		c.doAppend(`saved-messages`, `To: [email protected]`, `\Seen`).expect("OK")
 		c.doAppend(`saved-messages`, `To: [email protected]`).expect("OK")
@@ -25,7 +25,7 @@ func TestClose(t *testing.T) {
 		c.doAppend(`saved-messages`, `To: [email protected]`, `\Seen`).expect("OK")
 
 		c.C(`A002 SELECT saved-messages`)
-		c.Se(`A002 OK [READ-WRITE] (^_^)`)
+		c.Se(`A002 OK [READ-WRITE] SELECT`)
 
 		// TODO: Match flags in any order.
 		c.C(`A003 STORE 1 +FLAGS (\Deleted)`)
@@ -42,12 +42,12 @@ func TestClose(t *testing.T) {
 
 		// TODO: GOMSRV-106 - Ensure this also works for cases where multiple clients have the same mailbox open
 		c.C(`A202 CLOSE`)
-		c.S("A202 OK (^_^)")
+		c.S("A202 OK CLOSE")
 
 		// There are 2 messages in saved-messages.
 		c.C(`A006 STATUS saved-messages (MESSAGES)`)
 		c.S(`* STATUS "saved-messages" (MESSAGES 2)`)
-		c.S(`A006 OK (^_^)`)
+		c.S(`A006 OK STATUS`)
 	})
 }