From 8353b4b255936fc9ad0401fdcf2e11d27e38d9a3 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 25 Oct 2024 09:33:45 +0200 Subject: [PATCH] Follow upstream for HELO during Quit bug I reported the bug I fixed in https://github.com/wneessen/go-mail/commit/74fa3f6f62728cab1597bca5deea6ee758157e24 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. --- smtp/smtp.go | 17 +++-------------- smtp/smtp_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/smtp/smtp.go b/smtp/smtp.go index 63504fe..4841ec8 100644 --- a/smtp/smtp.go +++ b/smtp/smtp.go @@ -554,20 +554,9 @@ func (c *Client) Noop() error { // Quit sends the QUIT command and closes the connection to the server. func (c *Client) Quit() error { - // If we already tried to send a EHLO/HELO but it failed, we still need to be able to send - // a QUIT to close the connection. - // c.hello() will return the global helloErr of the Client, which will always be set if the HELO - // failed before. Therefore if we already sent a HELO and the error is not nil, we skip another - // EHLO/HELO try - c.mutex.RLock() - didHello := c.didHello - helloErr := c.helloError - c.mutex.RUnlock() - if !didHello || helloErr == nil { - if err := c.hello(); err != nil { - return err - } - } + // See https://github.com/golang/go/issues/70011 + _ = c.hello() // ignore error; we're quitting anyhow + _, _, err := c.cmd(221, "QUIT") if err != nil { return err diff --git a/smtp/smtp_test.go b/smtp/smtp_test.go index 737bd58..4fe0481 100644 --- a/smtp/smtp_test.go +++ b/smtp/smtp_test.go @@ -900,6 +900,35 @@ Goodbye. QUIT ` +func TestHELOFailed(t *testing.T) { + serverLines := `502 EH? +502 EH? +221 OK +` + clientLines := `EHLO localhost +HELO localhost +QUIT +` + server := strings.Join(strings.Split(serverLines, "\n"), "\r\n") + client := strings.Join(strings.Split(clientLines, "\n"), "\r\n") + var cmdbuf strings.Builder + bcmdbuf := bufio.NewWriter(&cmdbuf) + var fake faker + fake.ReadWriter = bufio.NewReadWriter(bufio.NewReader(strings.NewReader(server)), bcmdbuf) + c := &Client{Text: textproto.NewConn(fake), localName: "localhost"} + if err := c.Hello("localhost"); err == nil { + t.Fatal("expected EHLO to fail") + } + if err := c.Quit(); err != nil { + t.Errorf("QUIT failed: %s", err) + } + _ = bcmdbuf.Flush() + actual := cmdbuf.String() + if client != actual { + t.Errorf("Got:\n%s\nWant:\n%s", actual, client) + } +} + func TestExtensions(t *testing.T) { fake := func(server string) (c *Client, bcmdbuf *bufio.Writer, cmdbuf *strings.Builder) { server = strings.Join(strings.Split(server, "\n"), "\r\n")