Skip to content

Commit 8353b4b

Browse files
author
Winni Neessen
committed
Follow upstream for HELO during Quit bug
I reported the bug I fixed in 74fa3f6 to Go upstream. They fixed simpler by just ignoring the error (See: https://go.dev/cl/622476). We follow this patch accordingly. The upstream test has been adopted as well.
1 parent 9834c65 commit 8353b4b

File tree

2 files changed

+32
-14
lines changed

2 files changed

+32
-14
lines changed

smtp/smtp.go

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -554,20 +554,9 @@ func (c *Client) Noop() error {
554554

555555
// Quit sends the QUIT command and closes the connection to the server.
556556
func (c *Client) Quit() error {
557-
// If we already tried to send a EHLO/HELO but it failed, we still need to be able to send
558-
// a QUIT to close the connection.
559-
// c.hello() will return the global helloErr of the Client, which will always be set if the HELO
560-
// failed before. Therefore if we already sent a HELO and the error is not nil, we skip another
561-
// EHLO/HELO try
562-
c.mutex.RLock()
563-
didHello := c.didHello
564-
helloErr := c.helloError
565-
c.mutex.RUnlock()
566-
if !didHello || helloErr == nil {
567-
if err := c.hello(); err != nil {
568-
return err
569-
}
570-
}
557+
// See https://github.com/golang/go/issues/70011
558+
_ = c.hello() // ignore error; we're quitting anyhow
559+
571560
_, _, err := c.cmd(221, "QUIT")
572561
if err != nil {
573562
return err

smtp/smtp_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,35 @@ Goodbye.
900900
QUIT
901901
`
902902

903+
func TestHELOFailed(t *testing.T) {
904+
serverLines := `502 EH?
905+
502 EH?
906+
221 OK
907+
`
908+
clientLines := `EHLO localhost
909+
HELO localhost
910+
QUIT
911+
`
912+
server := strings.Join(strings.Split(serverLines, "\n"), "\r\n")
913+
client := strings.Join(strings.Split(clientLines, "\n"), "\r\n")
914+
var cmdbuf strings.Builder
915+
bcmdbuf := bufio.NewWriter(&cmdbuf)
916+
var fake faker
917+
fake.ReadWriter = bufio.NewReadWriter(bufio.NewReader(strings.NewReader(server)), bcmdbuf)
918+
c := &Client{Text: textproto.NewConn(fake), localName: "localhost"}
919+
if err := c.Hello("localhost"); err == nil {
920+
t.Fatal("expected EHLO to fail")
921+
}
922+
if err := c.Quit(); err != nil {
923+
t.Errorf("QUIT failed: %s", err)
924+
}
925+
_ = bcmdbuf.Flush()
926+
actual := cmdbuf.String()
927+
if client != actual {
928+
t.Errorf("Got:\n%s\nWant:\n%s", actual, client)
929+
}
930+
}
931+
903932
func TestExtensions(t *testing.T) {
904933
fake := func(server string) (c *Client, bcmdbuf *bufio.Writer, cmdbuf *strings.Builder) {
905934
server = strings.Join(strings.Split(server, "\n"), "\r\n")

0 commit comments

Comments
 (0)