[go] net/smtp: preserve EHLO error for non-502 responses
3 views
Skip to first unread message
sachin ruhil (Gerrit)
Dec 25, 2025, 12:46:19 AM (3 days ago) Dec 25
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
sachin ruhil has uploaded the change for review![]( "Open in Gerrit")
Commit message
net/smtp: preserve EHLO error for non-502 responses
The hello() function was incorrectly falling back to HELO for
all EHLO errors, not just 502 (command not supported). This
caused the original error to be lost when HELO also failed,
making debugging difficult.
Now hello() only falls back to HELO when EHLO returns error
code 502. For all other errors (like 117 for IP whitelisting),
the original error is preserved.
Fixes #56125
Change-Id: Ida084822fd333eba50a50022d23c52e83f52ccc7
Change diff
diff --git a/src/net/smtp/smtp.go b/src/net/smtp/smtp.go
index 522d80e..ae1987f 100644
--- a/src/net/smtp/smtp.go
+++ b/src/net/smtp/smtp.go
@@ -82,9 +82,13 @@
func (c *Client) hello() error {
if !c.didHello {
c.didHello = true
- err := c.ehlo()
- if err != nil {
- c.helloError = c.helo()
+ c.helloError = c.ehlo()
+ if c.helloError != nil {
+ // Only fall back to HELO if EHLO failed with 502 (not supported).
+ // For other errors, preserve the original error instead of masking it.
+ if textErr, ok := c.helloError.(*textproto.Error); ok && textErr.Code == 502 {
+ c.helloError = c.helo()
+ }
}
}
return c.helloError
diff --git a/src/net/smtp/smtp_test.go b/src/net/smtp/smtp_test.go
index 427ed0f7..e84ae9d 100644
--- a/src/net/smtp/smtp_test.go
+++ b/src/net/smtp/smtp_test.go
@@ -1191,3 +1191,57 @@
-----END RSA TESTING KEY-----`))
func testingKey(s string) string { return strings.ReplaceAll(s, "TESTING KEY", "PRIVATE KEY") }
+
+// TestHelloErrorPreservationBug reproduces issue #56125.
+// Demonstrates that when EHLO fails with a non-502 error (like 117),
+// the code incorrectly falls back to HELO, losing the original error.
+func TestHelloErrorPreservationBug(t *testing.T) {
+ fake := func(server string) (c *Client, bcmdbuf *bufio.Writer, cmdbuf *strings.Builder) {
+ server = strings.Join(strings.Split(server, "\n"), "\r\n")
+
+ 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"}
+
+ return c, bcmdbuf, cmdbuf
+ }
+
+ t.Run("non-502 error preserved", func(t *testing.T) {
+ // Server returns 117 (IP not whitelisted) - NOT 502
+ // After fix: Code should NOT fall back to HELO, preserving the original error
+ server := `117 The IP Address is not whitelisted: 127.0.0.1
+`
+
+ // After fix: Only EHLO should be sent, HELO should NOT be called
+ expectedCommands := `EHLO localhost
+`
+
+ c, bcmdbuf, cmdbuf := fake(server)
+ defer c.Close()
+
+ err := c.Hello("localhost")
+
+ // Verify the original error is preserved
+ errStr := err.Error()
+ hasOriginalError := strings.Contains(errStr, "117") || strings.Contains(errStr, "whitelisted")
+ if !hasOriginalError {
+ t.Errorf("Original error lost. Got '%v', expected error containing '117' or 'whitelisted'", err)
+ }
+
+ // Verify HELO was NOT called for non-502 error
+ bcmdbuf.Flush()
+ actualcmds := cmdbuf.String()
+ expected := strings.Join(strings.Split(expectedCommands, "\n"), "\r\n")
+
+ if strings.Contains(actualcmds, "HELO localhost") {
+ t.Errorf("HELO was incorrectly called for non-502 error. Commands sent: %q", actualcmds)
+ }
+
+ if actualcmds != expected {
+ t.Errorf("Commands sent: %q, want %q", actualcmds, expected)
+ }
+ })
+
+}
Change information
Files:
- M src/net/smtp/smtp.go
- M src/net/smtp/smtp_test.go
Change size: M
Delta: 2 files changed, 61 insertions(+), 3 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Liao (Gerrit)
Dec 27, 2025, 3:57:52 PM (20 hours ago) Dec 27
to sachin ruhil, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Brad Fitzpatrick and sachin ruhil
Sean Liao voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Brad Fitzpatrick
- sachin ruhil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages