From 5e727ac3edc7ebee2c252f698b40b25c732d9971 Mon Sep 17 00:00:00 2001
From: diogomr <diogo.moreira@amplemarket.com>
Date: Wed, 17 Apr 2024 20:37:13 +0100
Subject: [PATCH] Do not use HELLO as fallback of EHLO when server responds
 with 421

Change inspired by https://github.com/PHPMailer/PHPMailer/issues/2189

RFC 1869 section 4.5 states that the server will return the code
421 if the SMTP server is no longer available

This change fixes an issue where the actual error response from a
failed EHLO was not surfaced due to always being overridden by the
HELLO response.
---
 client.go      | 21 ++++++++++++++++-----
 client_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/client.go b/client.go
index 66249b5..aaea16b 100644
--- a/client.go
+++ b/client.go
@@ -197,12 +197,23 @@ func (c *Client) greet() error {
 
 // hello runs a hello exchange if needed.
 func (c *Client) hello() error {
-	if !c.didHello {
-		c.didHello = true
-		if err := c.greet(); err != nil {
-			c.helloError = err
-		} else if err := c.ehlo(); err != nil {
+	if c.didHello {
+		return c.helloError
+	}
+
+	c.didHello = true
+	if err := c.greet(); err != nil {
+		c.helloError = err
+		return c.helloError
+	}
+
+	if err := c.ehlo(); err != nil {
+		var smtpError *SMTPError
+		if errors.As(err, &smtpError) && (smtpError.Code == 500 || smtpError.Code == 502) {
+			// The server doesn't support EHLO, fallback to HELO
 			c.helloError = c.helo()
+		} else {
+			c.helloError = err
 		}
 	}
 	return c.helloError
diff --git a/client_test.go b/client_test.go
index 52aea84..296f49f 100644
--- a/client_test.go
+++ b/client_test.go
@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"io"
 	"net"
 	"net/textproto"
@@ -509,6 +510,39 @@ var helloClient = []string{
 	"NOOP\n",
 }
 
+var shuttingDownServerHello = `220 hello world
+421 Service not available, closing transmission channel
+`
+
+func TestHello_421Response(t *testing.T) {
+	server := strings.Join(strings.Split(shuttingDownServerHello, "\n"), "\r\n")
+	client := "EHLO customhost\r\n"
+	var cmdbuf bytes.Buffer
+	bcmdbuf := bufio.NewWriter(&cmdbuf)
+	var fake faker
+	fake.ReadWriter = bufio.NewReadWriter(bufio.NewReader(strings.NewReader(server)), bcmdbuf)
+	c := NewClient(fake)
+	defer c.Close()
+	c.serverName = "fake.host"
+	c.localName = "customhost"
+
+	err := c.Hello("customhost")
+	if err == nil {
+		t.Errorf("Expected Hello to fail")
+	}
+
+	var smtpError *SMTPError
+	if !errors.As(err, &smtpError) || smtpError.Code != 421 || smtpError.Message != "Service not available, closing transmission channel" {
+		t.Errorf("Expected error 421, got %v", err)
+	}
+
+	bcmdbuf.Flush()
+	actualcmds := cmdbuf.String()
+	if client != actualcmds {
+		t.Errorf("Got:\n%s\nExpected:\n%s", actualcmds, client)
+	}
+}
+
 var sendMailServer = `220 hello world
 502 EH?
 250 mx.google.com at your service