Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ type Conn struct {
dataResult chan error
bytesReceived int64 // counts total size of chunks when BDAT is used

fromReceived bool
recipients []string
didAuth bool
fromReceived bool
recipients []string
didAuth bool
hasReceivedData bool // tracks if connection has sent any SMTP data
}

func newConn(c net.Conn, s *Server) *Conn {
Expand Down Expand Up @@ -1305,7 +1306,22 @@ func (c *Conn) readLine() (string, error) {
}
}

return c.text.ReadLine()
line, err := c.text.ReadLine()
if err == nil {
c.hasReceivedData = true // Mark that connection sent data
} else if err != nil && !c.hasReceivedData {
// If we haven't received any data and get a connection error,
// treat it as EOF (client connected and immediately disconnected)
if netErr, ok := err.(*net.OpError); ok && netErr.Op == "read" {
return "", io.EOF
}
// Also handle textproto connection errors
if strings.Contains(err.Error(), "connection reset by peer") ||
strings.Contains(err.Error(), "broken pipe") {
return "", io.EOF
}
}
return line, err
}

func (c *Conn) reset() {
Expand Down
3 changes: 2 additions & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ func (s *Server) Serve(l net.Listener) error {
go func() {
defer s.wg.Done()

err := s.handleConn(newConn(c, s))
conn := newConn(c, s)
err := s.handleConn(conn)
if err != nil {
s.ErrorLog.Printf("error handling %v: %s", c.RemoteAddr(), err)
}
Expand Down
103 changes: 103 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1650,3 +1650,106 @@ func TestServerDELIVERBY(t *testing.T) {
t.Fatal("Incorrect BY parameter value:", fmt.Sprintf("expected %#v, got %#v", expectedDeliverByOpts, deliverByOpts))
}
}

func TestZeroByteConnectionLogging(t *testing.T) {
// Test for Issue #236: https://github.com/emersion/go-smtp/issues/236
// Zero-byte connections (health checks) should not generate error logs

t.Run("HealthCheckNoLogging", func(t *testing.T) {
// Capture error logs
var logBuffer bytes.Buffer
l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
defer l.Close()

be := new(backend)
s := smtp.NewServer(be)
s.Domain = "localhost"
s.ErrorLog = log.New(&logBuffer, "", 0)

done := make(chan error, 1)
go func() {
done <- s.Serve(l)
}()

// Simulate health check: connect and immediately close (no data sent)
conn, err := net.Dial("tcp", l.Addr().String())
if err != nil {
t.Fatal(err)
}

// Wait just enough for server to start processing, then force close
time.Sleep(10 * time.Millisecond)
if tcpConn, ok := conn.(*net.TCPConn); ok {
tcpConn.SetLinger(0) // Force RST to trigger "connection reset by peer"
}
conn.Close()

// Wait for server to process the connection reset
time.Sleep(100 * time.Millisecond)

s.Close()
<-done

// Verify no error logs for zero-byte connection
logOutput := logBuffer.String()
if logOutput != "" {
t.Errorf("Expected no error logs for health check, but got: %s", logOutput)
}
})

t.Run("NormalErrorLogging", func(t *testing.T) {
// Verify that normal SMTP errors are still logged when data was sent
var logBuffer bytes.Buffer
l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
defer l.Close()

be := new(backend)
s := smtp.NewServer(be)
s.Domain = "localhost"
s.ErrorLog = log.New(&logBuffer, "", 0)

done := make(chan error, 1)
go func() {
done <- s.Serve(l)
}()

// Connect and send some SMTP data, then abruptly close
conn, err := net.Dial("tcp", l.Addr().String())
if err != nil {
t.Fatal(err)
}

// Read greeting
buffer := make([]byte, 1024)
conn.SetReadDeadline(time.Now().Add(1 * time.Second))
conn.Read(buffer)

// Send EHLO command (this marks the connection as having received data)
conn.Write([]byte("EHLO test\r\n"))

// Wait for response and then abruptly close
time.Sleep(50 * time.Millisecond)
if tcpConn, ok := conn.(*net.TCPConn); ok {
tcpConn.SetLinger(0) // Force RST
}
conn.Close()

// Wait for server to process the connection reset
time.Sleep(100 * time.Millisecond)

s.Close()
<-done

// Verify that error was logged (connection had sent data)
logOutput := logBuffer.String()
if !strings.Contains(logOutput, "error handling") {
t.Errorf("Expected error log for connection with data, but got: %s", logOutput)
}
})
}
94 changes: 94 additions & 0 deletions zero_byte_connection_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package smtp_test

import (
"bytes"
"log"
"net"
"strings"
"testing"
"time"

"github.com/emersion/go-smtp"
)

// TestZeroByteConnection verifies that health check connections that
// connect and immediately disconnect don't generate error logs
func TestZeroByteConnection(t *testing.T) {
// Capture error logs
var logBuf bytes.Buffer
logger := log.New(&logBuf, "test: ", 0)

be := &backend{}
s := smtp.NewServer(be)
s.ErrorLog = logger
defer s.Close()

l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
defer l.Close()

go s.Serve(l)

// Simulate health check connections
for i := 0; i < 5; i++ {
conn, err := net.Dial("tcp", l.Addr().String())
if err != nil {
t.Fatal(err)
}
// Immediately close without sending any data
conn.Close()
}

// Give server time to process connections
time.Sleep(100 * time.Millisecond)

// Check logs
logs := logBuf.String()
if strings.Contains(logs, "error handling") {
t.Errorf("Zero-byte connections should not generate error logs, but got:\n%s", logs)
}
}

// TestNormalConnectionError verifies that real connection errors are still logged
func TestNormalConnectionError(t *testing.T) {
// Capture error logs
var logBuf bytes.Buffer
logger := log.New(&logBuf, "test: ", 0)

be := &backend{}
s := smtp.NewServer(be)
s.ErrorLog = logger
s.ReadTimeout = 50 * time.Millisecond // Short timeout to force error
defer s.Close()

l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
defer l.Close()

go s.Serve(l)

// Connect and send partial command
conn, err := net.Dial("tcp", l.Addr().String())
if err != nil {
t.Fatal(err)
}
defer conn.Close()

// Read greeting
buf := make([]byte, 1024)
conn.Read(buf)

// Send partial command and wait for timeout
conn.Write([]byte("HELO"))
time.Sleep(100 * time.Millisecond)

// Check that timeout error was logged
logs := logBuf.String()
if !strings.Contains(logs, "error handling") {
t.Error("Connection timeouts should still be logged")
}
}