diff --git a/internal/web/service/email/email.go b/internal/web/service/email/email.go index f35be585e..5bcbdab7f 100644 --- a/internal/web/service/email/email.go +++ b/internal/web/service/email/email.go @@ -33,6 +33,15 @@ func NewEmailService(settingService service.SettingService) *EmailService { return &EmailService{settingService: settingService} } +// smtpConnectTimeout bounds the TCP dial. smtpDeadline bounds every SMTP +// protocol step after the connection is up, so a server that accepts the socket +// but then stalls cannot block the sender goroutine (and leak its socket) long +// after the caller's own timeout has already fired. smtpDeadline is a var only +// so tests can shorten it. +const smtpConnectTimeout = 10 * time.Second + +var smtpDeadline = 30 * time.Second + // Send sends an HTML email to all configured recipients. func (s *EmailService) Send(subject, body string) error { host, err := s.settingService.GetSmtpHost() @@ -82,7 +91,7 @@ func (s *EmailService) Send(subject, body string) error { case "tls": ch <- result{s.sendWithTLS(addr, auth, from, recipients, msg, host)} case "starttls", "none": - ch <- result{smtp.SendMail(addr, auth, from, recipients, msg)} + ch <- result{s.sendPlain(addr, auth, from, recipients, msg, host)} default: ch <- result{fmt.Errorf("unknown SMTP encryption type: %s", encryptionType)} } @@ -147,6 +156,7 @@ func (s *EmailService) TestConnection() SMTPTestResult { return SMTPTestResult{false, "connect", classifySMTPError(err)} } defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(smtpDeadline)) // Stage 2: Handshake + Auth client, err := smtp.NewClient(conn, host) @@ -207,7 +217,7 @@ func (s *EmailService) TestConnection() SMTPTestResult { func (s *EmailService) sendWithTLS(addr string, auth smtp.Auth, from string, to []string, msg []byte, host string) error { // Dial with explicit timeout - dialer := &net.Dialer{Timeout: 10 * time.Second} + dialer := &net.Dialer{Timeout: smtpConnectTimeout} conn, err := (&tls.Dialer{NetDialer: dialer, Config: &tls.Config{ ServerName: host, InsecureSkipVerify: false, @@ -216,6 +226,7 @@ func (s *EmailService) sendWithTLS(addr string, auth smtp.Auth, from string, to return err } defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(smtpDeadline)) client, err := smtp.NewClient(conn, host) if err != nil { @@ -249,6 +260,56 @@ func (s *EmailService) sendWithTLS(addr string, auth smtp.Auth, from string, to return w.Close() } +// sendPlain delivers over a plain TCP connection, opportunistically upgrading +// via STARTTLS when the server advertises it (the behavior net/smtp.SendMail +// gives the "starttls" and "none" transports). Unlike SendMail it dials with a +// timeout and arms a connection deadline, so a server that never speaks or +// stalls mid-protocol cannot block the sender goroutine past smtpDeadline. +func (s *EmailService) sendPlain(addr string, auth smtp.Auth, from string, to []string, msg []byte, host string) error { + conn, err := (&net.Dialer{Timeout: smtpConnectTimeout}).Dial("tcp", addr) + if err != nil { + return err + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(smtpDeadline)) + + client, err := smtp.NewClient(conn, host) + if err != nil { + return err + } + defer client.Close() + + if err = client.Hello("localhost"); err != nil { + return err + } + if ok, _ := client.Extension("STARTTLS"); ok { + if err = client.StartTLS(&tls.Config{ServerName: host}); err != nil { + return err + } + } + if auth != nil { + if err = client.Auth(auth); err != nil { + return err + } + } + if err = client.Mail(from); err != nil { + return err + } + for _, r := range to { + if err = client.Rcpt(r); err != nil { + return err + } + } + w, err := client.Data() + if err != nil { + return err + } + if _, err = w.Write(msg); err != nil { + return err + } + return w.Close() +} + // SendTest sends a test email and returns any error with detail. func (s *EmailService) SendTest() error { return s.Send( diff --git a/internal/web/service/email/email_deadline_test.go b/internal/web/service/email/email_deadline_test.go new file mode 100644 index 000000000..b34b00b84 --- /dev/null +++ b/internal/web/service/email/email_deadline_test.go @@ -0,0 +1,50 @@ +package email + +import ( + "net" + "testing" + "time" +) + +// A server that accepts the TCP connection but then never speaks must not block +// the sender goroutine indefinitely: sendPlain arms a connection deadline so the +// SMTP greeting read fails instead of hanging until the OS TCP timeout, long +// after the caller's own 30s budget has passed. +func TestSendPlainReturnsOnStalledServer(t *testing.T) { + orig := smtpDeadline + smtpDeadline = 300 * time.Millisecond + t.Cleanup(func() { smtpDeadline = orig }) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + + stall := make(chan struct{}) + defer close(stall) + go func() { + conn, err := ln.Accept() + if err != nil { + return + } + defer conn.Close() + <-stall + }() + + s := &EmailService{} + done := make(chan error, 1) + go func() { + done <- s.sendPlain(ln.Addr().String(), nil, "from@example.com", + []string{"to@example.com"}, []byte("body"), "example.com") + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected an error from a silent SMTP server, got nil") + } + case <-time.After(3 * time.Second): + t.Fatal("sendPlain did not return on a stalled server within the deadline") + } +}