From 1dd7b840fa7736e777d7bdd583c1611eaaf87380 Mon Sep 17 00:00:00 2001 From: Ahmed Laban <79754427+amlaban65@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:47:18 -0400 Subject: [PATCH] smtpconn: Treat wrapped dial errors as network errors Since fd6d880, attemptConnect wraps dial errors as "dialer: %w". wrapClientErr matched *net.OpError with a type switch, so wrapped dial errors fell through to the default case and were reported as permanent "554 5.0.0 Internal server error" instead of "450 4.4.2 Network I/O error". For LMTP this made maddy reject RCPT permanently while the LMTP server was restarting, instead of asking the sender to retry. Look for *net.OpError with errors.As in the default case, so the wrapped error is classified as before while keeping the added context. Fixes #872 Signed-off-by: Ahmed Laban --- internal/smtpconn/smtpconn.go | 73 ++++++++++++++++++------------ internal/smtpconn/smtpconn_test.go | 54 ++++++++++++++++++++++ 2 files changed, 97 insertions(+), 30 deletions(-) diff --git a/internal/smtpconn/smtpconn.go b/internal/smtpconn/smtpconn.go index 50d91a0e..76e1789c 100644 --- a/internal/smtpconn/smtpconn.go +++ b/internal/smtpconn/smtpconn.go @@ -138,41 +138,54 @@ func (c *C) wrapClientErr(err error, serverName string) error { Err: err, } case *net.OpError: - if _, ok := err.Err.(*net.DNSError); ok { - reason, misc := exterrors.UnwrapDNSErr(err) - misc["remote_server"] = err.Addr - misc["io_op"] = err.Op - - code := exterrors.SMTPCode(err, 450, 550) - enhancedCode := exterrors.SMTPEnchCode(err, exterrors.EnhancedCode{0, 4, 4}) - if c.DNSErrorsTemporary { - code = 451 - enhancedCode = exterrors.EnhancedCode{4, 4, 4} - } + return c.wrapNetErr(err, err) + default: + // Dial errors are wrapped by attemptConnect, so they need to be + // unwrapped to be recognized as (temporary) network errors. + var opErr *net.OpError + if errors.As(err, &opErr) { + return c.wrapNetErr(err, opErr) + } + return exterrors.WithFields(err, map[string]interface{}{ + "remote_server": serverName, + }) + } +} - return &exterrors.SMTPError{ - Code: code, - EnhancedCode: enhancedCode, - Message: "DNS error", - Err: err, - Reason: reason, - Misc: misc, - } +// wrapNetErr converts a network error into an SMTP error. err is the error to +// wrap (it may contain additional context) and opErr is the *net.OpError +// found in it. +func (c *C) wrapNetErr(err error, opErr *net.OpError) error { + if _, ok := opErr.Err.(*net.DNSError); ok { + reason, misc := exterrors.UnwrapDNSErr(opErr) + misc["remote_server"] = opErr.Addr + misc["io_op"] = opErr.Op + + code := exterrors.SMTPCode(opErr, 450, 550) + enhancedCode := exterrors.SMTPEnchCode(opErr, exterrors.EnhancedCode{0, 4, 4}) + if c.DNSErrorsTemporary { + code = 451 + enhancedCode = exterrors.EnhancedCode{4, 4, 4} } + return &exterrors.SMTPError{ - Code: 450, - EnhancedCode: exterrors.EnhancedCode{4, 4, 2}, - Message: "Network I/O error", + Code: code, + EnhancedCode: enhancedCode, + Message: "DNS error", Err: err, - Misc: map[string]interface{}{ - "remote_addr": err.Addr, - "io_op": err.Op, - }, + Reason: reason, + Misc: misc, } - default: - return exterrors.WithFields(err, map[string]interface{}{ - "remote_server": serverName, - }) + } + return &exterrors.SMTPError{ + Code: 450, + EnhancedCode: exterrors.EnhancedCode{4, 4, 2}, + Message: "Network I/O error", + Err: err, + Misc: map[string]interface{}{ + "remote_addr": opErr.Addr, + "io_op": opErr.Op, + }, } } diff --git a/internal/smtpconn/smtpconn_test.go b/internal/smtpconn/smtpconn_test.go index b8fd647e..fba6d041 100644 --- a/internal/smtpconn/smtpconn_test.go +++ b/internal/smtpconn/smtpconn_test.go @@ -19,11 +19,18 @@ along with this program. If not, see . package smtpconn import ( + "context" + "errors" "flag" "math/rand" + "net" "os" "strconv" "testing" + + "github.com/foxcpp/maddy/framework/config" + "github.com/foxcpp/maddy/framework/exterrors" + "github.com/foxcpp/maddy/internal/testutils" ) var testPort string @@ -39,3 +46,50 @@ func TestMain(m *testing.M) { testPort = *remoteSmtpPort os.Exit(m.Run()) } + +// TestConnectRefused checks that a failed dial is reported as a temporary +// "Network I/O error", so the sender retries instead of bouncing the message. +func TestConnectRefused(t *testing.T) { + // Get a local port with nothing listening on it. + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + port := strconv.Itoa(l.Addr().(*net.TCPAddr).Port) + if err := l.Close(); err != nil { + t.Fatal(err) + } + endp := config.Endpoint{Scheme: "tcp", Host: "127.0.0.1", Port: port} + + connect := map[string]func(*C) error{ + "Connect": func(c *C) error { + _, err := c.Connect(context.Background(), endp, false, nil) + return err + }, + "ConnectLMTP": func(c *C) error { + _, err := c.ConnectLMTP(context.Background(), endp, false, nil) + return err + }, + } + for name, fn := range connect { + t.Run(name, func(t *testing.T) { + c := New() + c.Log = testutils.Logger(t, "smtpconn") + + err := fn(c) + if err == nil { + t.Fatal("expected an error") + } + var smtpErr *exterrors.SMTPError + if !errors.As(err, &smtpErr) { + t.Fatalf("expected *exterrors.SMTPError, got %T: %v", err, err) + } + if smtpErr.Code != 450 || smtpErr.EnhancedCode != (exterrors.EnhancedCode{4, 4, 2}) { + t.Errorf("expected 450 4.4.2, got %d %v", smtpErr.Code, smtpErr.EnhancedCode) + } + if !exterrors.IsTemporary(err) { + t.Error("expected a temporary error") + } + }) + } +}