Merge pull request #3 from matjam/master

Handle address validation better.

address.go

@@ -2,18 +2,21 @@ package smtpd
 
 import (
 	"fmt"
-	"strings"
+	"net/mail"
 )
 
 func parseAddress(src string) (string, error) {
-
+	// While a RFC5321 mailbox specification is not the same as an RFC5322
-	if len(src) == 0 || src[0] != '<' || src[len(src)-1] != '>' {
+	// email address specification, it is better to accept that format and
-		return "", fmt.Errorf("Ill-formatted e-mail address: %s", src)
+	// parse it down to the actual address, as there are a lot of badly
+	// behaving MTAs and MUAs that do it wrongly. It therefore makes sense
+	// to rely on Go's built-in address parser. This does have the benefit
+	// of allowing "email@example.com" as input as thats commonly used,
+	// though not RFC compliant.
+	addr, err := mail.ParseAddress(src)
+	if err != nil {
+		return "", fmt.Errorf("malformed e-mail address: %s", src)
 	}
 
-	if strings.Count(src, "@") > 1 {
+	return addr.Address, nil
-		return "", fmt.Errorf("Ill-formatted e-mail address: %s", src)
-	}
-
-	return src[1 : len(src)-1], nil
 }

protocol.go

@@ -212,11 +212,17 @@ func (session *session) handleMAIL(cmd command) {
 		return
 	}
 
-	addr, err := parseAddress(cmd.params[1])
+	var err error
+	addr := "" // null sender
 
-	if err != nil {
+	// We must accept a null sender as per rfc5321 section-6.1.
-		session.reply(502, "Ill-formatted e-mail address")
+	if cmd.params[1] != "<>" {
-		return
+		addr, err = parseAddress(cmd.params[1])
+
+		if err != nil {
+			session.reply(502, "Malformed e-mail address")
+			return
+		}
 	}
 
 	if session.server.SenderChecker != nil {
@@ -256,7 +262,7 @@ func (session *session) handleRCPT(cmd command) {
 	addr, err := parseAddress(cmd.params[1])
 
 	if err != nil {
-		session.reply(502, "Ill-formatted e-mail address")
+		session.reply(502, "Malformed e-mail address")
 		return
 	}
 

smtpd_test.go

@@ -1003,19 +1003,19 @@ func TestHELO(t *testing.T) {
 	}
 
 	if err := cmd(c.Text, 502, "MAIL FROM:<christian@technobabble.dk>"); err != nil {
-		t.Fatalf("MAIL didn't fail: %v", err)
+		t.Fatalf("MAIL before HELO didn't fail: %v", err)
 	}
 
 	if err := cmd(c.Text, 250, "HELO localhost"); err != nil {
 		t.Fatalf("HELO failed: %v", err)
 	}
 
-	if err := cmd(c.Text, 502, "MAIL FROM:christian@technobabble.dk"); err != nil {
+	if err := cmd(c.Text, 250, "MAIL FROM:<christian@technobabble.dk>"); err != nil {
-		t.Fatalf("MAIL didn't fail: %v", err)
+		t.Fatalf("MAIL after HELO failed: %v", err)
 	}
 
 	if err := cmd(c.Text, 250, "HELO localhost"); err != nil {
-		t.Fatalf("HELO failed: %v", err)
+		t.Fatalf("double HELO failed: %v", err)
 	}
 
 	if err := c.Quit(); err != nil {
@@ -1079,6 +1079,56 @@ func TestLOGINAuth(t *testing.T) {
 
 }
 
+func TestNullSender(t *testing.T) {
+
+	addr, closer := runserver(t, &smtpd.Server{})
+
+	defer closer()
+
+	c, err := smtp.Dial(addr)
+	if err != nil {
+		t.Fatalf("Dial failed: %v", err)
+	}
+
+	if err := cmd(c.Text, 250, "HELO localhost"); err != nil {
+		t.Fatalf("HELO failed: %v", err)
+	}
+
+	if err := cmd(c.Text, 250, "MAIL FROM:<>"); err != nil {
+		t.Fatalf("MAIL with null sender failed: %v", err)
+	}
+
+	if err := c.Quit(); err != nil {
+		t.Fatalf("Quit failed: %v", err)
+	}
+
+}
+
+func TestNoBracketsSender(t *testing.T) {
+
+	addr, closer := runserver(t, &smtpd.Server{})
+
+	defer closer()
+
+	c, err := smtp.Dial(addr)
+	if err != nil {
+		t.Fatalf("Dial failed: %v", err)
+	}
+
+	if err := cmd(c.Text, 250, "HELO localhost"); err != nil {
+		t.Fatalf("HELO failed: %v", err)
+	}
+
+	if err := cmd(c.Text, 250, "MAIL FROM:christian@technobabble.dk"); err != nil {
+		t.Fatalf("MAIL without brackets failed: %v", err)
+	}
+
+	if err := c.Quit(); err != nil {
+		t.Fatalf("Quit failed: %v", err)
+	}
+
+}
+
 func TestErrors(t *testing.T) {
 
 	cert, err := tls.X509KeyPair(localhostCert, localhostKey)
@@ -1111,10 +1161,6 @@ func TestErrors(t *testing.T) {
 		t.Fatalf("AUTH didn't fail: %v", err)
 	}
 
-	if err := cmd(c.Text, 502, "MAIL FROM:christian@technobabble.dk"); err != nil {
-		t.Fatalf("MAIL didn't fail: %v", err)
-	}
-
 	if err := c.Mail("sender@example.org"); err != nil {
 		t.Fatalf("MAIL failed: %v", err)
 	}