Skip to content

Commit

Permalink
Follow upstream for HELO during Quit bug
Browse files Browse the repository at this point in the history
I reported the bug I fixed in wneessen@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.
  • Loading branch information
wneessen committed Oct 25, 2024
1 parent 9834c65 commit 8353b4b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
17 changes: 3 additions & 14 deletions smtp/smtp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions smtp/smtp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 8353b4b

Please sign in to comment.