Improve the command parser to try to parse malformed commands.

Timon Relitzki reported that some systems, for example the Synology
DSM Rackstations, will send an extra space character with their SMTP
commands.

MAIL FROM:<christian@technobabble.dk>

Becomes:

MAIL FROM: <christian@technobabble.dk>

This confused the command parser. In the spirit of the robustness
principle if have choosen to improve the command parse to also handle
these extra spaces.

The command parser has also been extended with some extra unit tests,
to demonstrate the problem.

protocol.go

@@ -28,10 +28,32 @@ func parseLine(line string) (cmd command) {
 	cmd.fields = strings.Fields(line)
 
 	if len(cmd.fields) > 0 {
+
 		cmd.action = strings.ToUpper(cmd.fields[0])
+
 		if len(cmd.fields) > 1 {
+
+			// Account for some clients breaking the standard and having
+			// an extra whitespace after the ':' character. Example:
+			//
+			// MAIL FROM: <christian@technobabble.dk>
+			//
+			// Should be:
+			//
+			// MAIL FROM:<christian@technobabble.dk>
+			//
+			// Thus, we add a check if the second field ends with ':'
+			// and appends the rest of the third field.
+
+			if cmd.fields[1][len(cmd.fields[1])-1] == ':' && len(cmd.fields) > 2 {
+				cmd.fields[1] = cmd.fields[1] + cmd.fields[2]
+				cmd.fields = cmd.fields[0:2]
+			}
+
 			cmd.params = strings.Split(cmd.fields[1], ":")
+
 		}
+
 	}
 
 	return

protocol_test.go

@@ -0,0 +1,83 @@
+package smtpd
+
+import "testing"
+
+func TestParseLine(t *testing.T) {
+
+	cmd := parseLine("HELO hostname")
+	if cmd.action != "HELO" {
+		t.Fatalf("unexpected action: %s", cmd.action)
+	}
+
+	if len(cmd.fields) != 2 {
+		t.Fatalf("unexpected fields length: %d", len(cmd.fields))
+	}
+
+	if len(cmd.params) != 1 {
+		t.Fatalf("unexpected params length: %d", len(cmd.params))
+	}
+
+	if cmd.params[0] != "hostname" {
+		t.Fatalf("unexpected value for param 0: %v", cmd.params[0])
+	}
+
+	cmd = parseLine("DATA")
+	if cmd.action != "DATA" {
+		t.Fatalf("unexpected action: %s", cmd.action)
+	}
+
+	if len(cmd.fields) != 1 {
+		t.Fatalf("unexpected fields length: %d", len(cmd.fields))
+	}
+
+	if cmd.params != nil {
+		t.Fatalf("unexpected params: %v", cmd.params)
+	}
+
+	cmd = parseLine("MAIL FROM:<christian@technobabble.dk>")
+	if cmd.action != "MAIL" {
+		t.Fatalf("unexpected action: %s", cmd.action)
+	}
+
+	if len(cmd.fields) != 2 {
+		t.Fatalf("unexpected fields length: %d", len(cmd.fields))
+	}
+
+	if len(cmd.params) != 2 {
+		t.Fatalf("unexpected params length: %d", len(cmd.params))
+	}
+
+	if cmd.params[0] != "FROM" {
+		t.Fatalf("unexpected value for param 0: %v", cmd.params[0])
+	}
+
+	if cmd.params[1] != "<christian@technobabble.dk>" {
+		t.Fatalf("unexpected value for param 1: %v", cmd.params[1])
+	}
+
+}
+
+func TestParseLineMailformedMAILFROM(t *testing.T) {
+
+	cmd := parseLine("MAIL FROM: <christian@technobabble.dk>")
+	if cmd.action != "MAIL" {
+		t.Fatalf("unexpected action: %s", cmd.action)
+	}
+
+	if len(cmd.fields) != 2 {
+		t.Fatalf("unexpected fields length: %d", len(cmd.fields))
+	}
+
+	if len(cmd.params) != 2 {
+		t.Fatalf("unexpected params length: %d", len(cmd.params))
+	}
+
+	if cmd.params[0] != "FROM" {
+		t.Fatalf("unexpected value for param 0: %v", cmd.params[0])
+	}
+
+	if cmd.params[1] != "<christian@technobabble.dk>" {
+		t.Fatalf("unexpected value for param 1: %v", cmd.params[1])
+	}
+
+}

smtpd_test.go

@@ -1160,3 +1160,34 @@ func TestErrors(t *testing.T) {
 	}
 
 }
+
+func TestMailformedMAILFROM(t *testing.T) {
+
+	addr, closer := runserver(t, &smtpd.Server{
+		SenderChecker: func(peer smtpd.Peer, addr string) error {
+			if addr != "christian@technobabble.dk" {
+				return smtpd.Error{Code: 502, Message: "Denied"}
+			}
+			return nil
+		},
+	})
+
+	defer closer()
+
+	c, err := smtp.Dial(addr)
+	if err != nil {
+		t.Fatalf("Dial failed: %v", err)
+	}
+
+	if err := c.Hello("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 FROM failed with extra whitespace: %v", err)
+	}
+
+	if err := c.Quit(); err != nil {
+		t.Fatalf("Quit failed: %v", err)
+	}
+}