From 03861efdd8ed902dd8143ee941fa711d47ffaa73 Mon Sep 17 00:00:00 2001
From: Christian Joergensen <christian.joergensen@gmta.info>
Date: Tue, 23 May 2017 21:42:13 +0200
Subject: [PATCH] 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      | 22 +++++++++++++
 protocol_test.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
 smtpd_test.go    | 31 ++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 protocol_test.go

diff --git a/protocol.go b/protocol.go
index d0d68cd..16117ac 100644
--- a/protocol.go
+++ b/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
diff --git a/protocol_test.go b/protocol_test.go
new file mode 100644
index 0000000..aa04b70
--- /dev/null
+++ b/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])
+	}
+
+}
diff --git a/smtpd_test.go b/smtpd_test.go
index 3f6a2a0..57bccba 100644
--- a/smtpd_test.go
+++ b/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)
+	}
+}