Repository: emersion/go-smtp (go)

Issue:
Should 'Too many recipients' return 452 4.5.3 instead of 552 5.5.3?
I've been looking into this response a bit for something I'm working on. The current response when we have a recipient limit is

```
552 5.5.3 Maximum limit of %v recipients reached
```

From my reading though, it seems the following is the more 'standard' response for this case

```
452 4.5.3 Too many recipients
```

with some providers including the limit there. Specifically I'm concerned with how the big players say to handle this status code to make sure that they send correctly with fewer recipients. I'm about to do testing with the current code and will report my findings, but I think this should be changed.

**Gmail**: [https://support.google.com/a/answer/3726730](https://support.google.com/a/answer/3726730)
> 452, "4.5.3", Your message has too many recipients. For more information regarding Google's sending limits, visit [Limits for sending & getting mail.](https://support.google.com/mail/answer/22839)

**Microsoft**: [https://support.google.com/a/answer/3726730](https://support.google.com/a/answer/3726730)
> For inbound email, the Receive connector MaxRecipientsPerMessage is verified first. However, if the number of recipients exceeds the limit, the message is not rejected; the connection receives the error, 452 4.5.3 Too many recipients. Most mail servers understand this error and they will continue to resend the message in another connection until the message is delivered to all recipients.

Granted they are talking about their own servers and how they expect clients to handle it, so I would hope they respond the same way. The rfc's may suggest that 5xx is more correct (I don't think they specifically say what to do for too many recipients), but if gmail and ms agree on something else, I would consider it the de-facto standard.

Pull request:
server: Correct status codes in several responses.
- The excess number of recipients should be responded with `4XX`.
- Disconnections due to network errors or idle timeouts should be `4XX`.
- `3XX` codes do not have enhanced codes.
- Other typos.
---
This PR may fix #195 .

Patch:
diff --git a/conn.go b/conn.go
index 72a67d8..339f40b 100644
--- a/conn.go
+++ b/conn.go
@@ -281,7 +281,7 @@ func (c *Conn) handleGreet(enhanced bool, arg string) {
 // READY state -> waiting for MAIL
 func (c *Conn) handleMail(arg string) {
 	if c.helo == "" {
-		c.writeResponse(502, EnhancedCode{2, 5, 1}, "Please introduce yourself first.")
+		c.writeResponse(502, EnhancedCode{5, 5, 1}, "Please introduce yourself first.")
 		return
 	}
 	if c.bdatPipe != nil {
@@ -467,7 +467,7 @@ func (c *Conn) handleRcpt(arg string) {
 	recipient := strings.Trim(arg[3:], "<> ")
 
 	if c.server.MaxRecipients > 0 && len(c.recipients) >= c.server.MaxRecipients {
-		c.writeResponse(552, EnhancedCode{5, 5, 3}, fmt.Sprintf("Maximum limit of %v recipients reached", c.server.MaxRecipients))
+		c.writeResponse(452, EnhancedCode{4, 5, 3}, fmt.Sprintf("Maximum limit of %v recipients reached", c.server.MaxRecipients))
 		return
 	}
 
@@ -626,7 +626,7 @@ func (c *Conn) handleData(arg string) {
 	}
 
 	// We have recipients, go to accept data
-	c.writeResponse(354, EnhancedCode{2, 0, 0}, "Go ahead. End your data with <CR><LF>.<CR><LF>")
+	c.writeResponse(354, NoEnhancedCode, "Go ahead. End your data with <CR><LF>.<CR><LF>")
 
 	defer c.reset()
 
@@ -924,7 +924,11 @@ func (c *Conn) Reject() {
 }
 
 func (c *Conn) greet() {
-	c.writeResponse(220, NoEnhancedCode, fmt.Sprintf("%v ESMTP Service Ready", c.server.Domain))
+	protocol := "ESMTP"
+	if c.server.LMTP {
+		protocol = "LMTP"
+	}
+	c.writeResponse(220, NoEnhancedCode, fmt.Sprintf("%v %s Service Ready", c.server.Domain, protocol))
 }
 
 func (c *Conn) writeResponse(code int, enhCode EnhancedCode, text ...string) {
diff --git a/server.go b/server.go
index 3e6c3e3..341ac09 100644
--- a/server.go
+++ b/server.go
@@ -201,11 +201,11 @@ func (s *Server) handleConn(c *Conn) error {
 			}
 
 			if neterr, ok := err.(net.Error); ok && neterr.Timeout() {
-				c.writeResponse(221, EnhancedCode{2, 4, 2}, "Idle timeout, bye bye")
+				c.writeResponse(421, EnhancedCode{4, 4, 2}, "Idle timeout, bye bye")
 				return nil
 			}
 
-			c.writeResponse(221, EnhancedCode{2, 4, 0}, "Connection error, sorry")
+			c.writeResponse(421, EnhancedCode{4, 4, 0}, "Connection error, sorry")
 			return err
 		}
 	}


Test patch:
diff --git a/lmtp_server_test.go b/lmtp_server_test.go
index 9cd98c2..980b3a3 100644
--- a/lmtp_server_test.go
+++ b/lmtp_server_test.go
@@ -4,12 +4,24 @@ import (
 	"bufio"
 	"errors"
 	"io"
+	"net"
 	"strings"
 	"testing"
 
 	"github.com/emersion/go-smtp"
 )
 
+func testServerGreetedLMTP(t *testing.T, fn ...serverConfigureFunc) (be *backend, s *smtp.Server, c net.Conn, scanner *bufio.Scanner) {
+	be, s, c, scanner = testServer(t, fn...)
+
+	scanner.Scan()
+	if scanner.Text() != "220 localhost LMTP Service Ready" {
+		t.Fatal("Invalid greeting:", scanner.Text())
+	}
+
+	return
+}
+
 func sendDeliveryCmdsLMTP(t *testing.T, scanner *bufio.Scanner, c io.Writer) {
 	sendLHLO(t, scanner, c)
 
@@ -43,7 +55,7 @@ func sendLHLO(t *testing.T, scanner *bufio.Scanner, c io.Writer) {
 }
 
 func TestServer_LMTP(t *testing.T) {
-	be, s, c, scanner := testServerGreeted(t, func(s *smtp.Server) {
+	be, s, c, scanner := testServerGreetedLMTP(t, func(s *smtp.Server) {
 		s.LMTP = true
 		be := s.Backend.(*backend)
 		be.implementLMTPData = true
@@ -80,7 +92,7 @@ func TestServer_LMTP_Early(t *testing.T) {
 
 	lmtpStatusSync := make(chan struct{})
 
-	be, s, c, scanner := testServerGreeted(t, func(s *smtp.Server) {
+	be, s, c, scanner := testServerGreetedLMTP(t, func(s *smtp.Server) {
 		s.LMTP = true
 		be := s.Backend.(*backend)
 		be.implementLMTPData = true
@@ -124,7 +136,7 @@ func TestServer_LMTP_Expand(t *testing.T) {
 	// correctly expands results if backend doesn't
 	// implement LMTPSession.
 
-	be, s, c, scanner := testServerGreeted(t, func(s *smtp.Server) {
+	be, s, c, scanner := testServerGreetedLMTP(t, func(s *smtp.Server) {
 		s.LMTP = true
 	})
 	defer s.Close()
@@ -147,7 +159,7 @@ func TestServer_LMTP_Expand(t *testing.T) {
 }
 
 func TestServer_LMTP_DuplicatedRcpt(t *testing.T) {
-	be, s, c, scanner := testServerGreeted(t, func(s *smtp.Server) {
+	be, s, c, scanner := testServerGreetedLMTP(t, func(s *smtp.Server) {
 		s.LMTP = true
 		be := s.Backend.(*backend)
 		be.implementLMTPData = true