Repository: emersion/go-smtp
Pull request #77 (2019-11-29 22:24:05)

server: Rewrite MAIL FROM arguments handling 
SMTPUTF8 and REQUIRETLS arguments are specified without the '=' sign.

Note to myself: If you think you can get away without writting tests for
something, you are terribly wrong and should suffer.

emersion: Err, tests are failing. BTW, would be nice to add a test for `SMTPUTF8`.
foxcpp: I guess this is even better.
codecov[bot]: # [Codecov](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=h1) Report
> Merging [#77](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=desc) into [master](https://codecov.io/gh/emersion/go-smtp/commit/437866b1e73fff79a55c52b68563587fc9850837?src=pr&el=desc) will **decrease** coverage by `0.05%`.
> The diff coverage is `65.51%`.

[![Impacted file tree graph](https://codecov.io/gh/emersion/go-smtp/pull/77/graphs/tree.svg?width=650&token=vj7HO8ABwC&height=150&src=pr)](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
- Coverage   63.32%   63.26%   -0.06%     
==========================================
  Files           8        8              
  Lines         867      882      +15     
==========================================
+ Hits          549      558       +9     
- Misses        233      238       +5     
- Partials       85       86       +1
```


| [Impacted Files](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [conn.go](https://codecov.io/gh/emersion/go-smtp/pull/77/diff?src=pr&el=tree#diff-Y29ubi5nbw==) | `62.46% <60.86%> (+0.03%)` | :arrow_up: |
| [parse.go](https://codecov.io/gh/emersion/go-smtp/pull/77/diff?src=pr&el=tree#diff-cGFyc2UuZ28=) | `52.63% <83.33%> (-0.31%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=footer). Last update [437866b...8771f1b](https://codecov.io/gh/emersion/go-smtp/pull/77?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/conn.go b/conn.go
index 595da1a..3e71955 100644
--- a/conn.go
+++ b/conn.go
@@ -302,26 +302,38 @@ func (c *Conn) handleMail(arg string) {
 			return
 		}
 
-		if args["SIZE"] != "" {
-			size, err := strconv.ParseInt(args["SIZE"], 10, 32)
-			if err != nil {
-				c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Unable to parse SIZE as an integer")
-				return
-			}
-
-			if c.server.MaxMessageBytes > 0 && int(size) > c.server.MaxMessageBytes {
-				c.WriteResponse(552, EnhancedCode{5, 3, 4}, "Max message size exceeded")
+		for key, value := range args {
+			switch key {
+			case "SIZE":
+				size, err := strconv.ParseInt(value, 10, 32)
+				if err != nil {
+					c.WriteResponse(501, EnhancedCode{5, 5, 4}, "Unable to parse SIZE as an integer")
+					return
+				}
+
+				if c.server.MaxMessageBytes > 0 && int(size) > c.server.MaxMessageBytes {
+					c.WriteResponse(552, EnhancedCode{5, 3, 4}, "Max message size exceeded")
+					return
+				}
+
+				opts.Size = int(size)
+			case "SMTPUTF8":
+				if !c.server.EnableSMTPUTF8 {
+					c.WriteResponse(504, EnhancedCode{5, 5, 4}, "SMTPUTF8 is not implemented")
+					return
+				}
+				opts.UTF8 = true
+			case "REQUIRETLS":
+				if !c.server.EnableREQUIRETLS {
+					c.WriteResponse(504, EnhancedCode{5, 5, 4}, "REQUIRETLS is not implemented")
+					return
+				}
+				opts.RequireTLS = true
+			default:
+				c.WriteResponse(500, EnhancedCode{5, 5, 4}, "Unknown MAIL FROM argument")
 				return
 			}
-
-			opts.Size = int(size)
 		}
-
-		_, ok := args["SMTPUTF8"]
-		opts.UTF8 = ok
-
-		_, ok = args["REQUIRETLS"]
-		opts.RequireTLS = ok
 	}
 
 	if err := c.Session().Mail(from, opts); err != nil {
diff --git a/parse.go b/parse.go
index 3299238..df73786 100644
--- a/parse.go
+++ b/parse.go
@@ -37,7 +37,7 @@ func parseCmd(line string) (cmd string, arg string, err error) {
 // Takes the arguments proceeding a command and files them
 // into a map[string]string after uppercasing each key.  Sample arg
 // string:
-//		" BODY=8BITMIME SIZE=1024"
+//		" BODY=8BITMIME SIZE=1024 SMTPUTF8"
 // The leading space is mandatory.
 func parseArgs(args []string) (map[string]string, error) {
 	argMap := map[string]string{}
@@ -46,10 +46,14 @@ func parseArgs(args []string) (map[string]string, error) {
 			continue
 		}
 		m := strings.Split(arg, "=")
-		if len(m) != 2 {
+		switch len(m) {
+		case 2:
+			argMap[strings.ToUpper(m[0])] = m[1]
+		case 1:
+			argMap[strings.ToUpper(m[0])] = ""
+		default:
 			return nil, fmt.Errorf("Failed to parse arg string: %q", arg)
 		}
-		argMap[strings.ToUpper(m[0])] = m[1]
 	}
 	return argMap, nil
 }


Test patch:
diff --git a/server_test.go b/server_test.go
index b684f48..ed66927 100644
--- a/server_test.go
+++ b/server_test.go
@@ -285,12 +285,41 @@ func TestServerPanicRecover(t *testing.T) {
 	return
 }
 
-func TestServerBadESMTPVar(t *testing.T) {
+func TestServerSMTPUTF8(t *testing.T) {
 	_, s, c, scanner := testServerAuthenticated(t)
+	s.EnableSMTPUTF8 = true
 	defer s.Close()
 	defer c.Close()
 
-	io.WriteString(c, "MAIL FROM:<alice@wonderland.book> RABBIT\r\n")
+	io.WriteString(c, "MAIL FROM:<alice@wonderland.book> SMTPUTF8\r\n")
+	scanner.Scan()
+	if !strings.HasPrefix(scanner.Text(), "250 ") {
+		t.Fatal("Invalid MAIL response:", scanner.Text())
+	}
+
+	return
+}
+
+func TestServerSMTPUTF8_Disabled(t *testing.T) {
+	_, s, c, scanner := testServerAuthenticated(t)
+	defer s.Close()
+	defer c.Close()
+
+	io.WriteString(c, "MAIL FROM:<alice@wonderland.book> SMTPUTF8\r\n")
+	scanner.Scan()
+	if strings.HasPrefix(scanner.Text(), "250 ") {
+		t.Fatal("Invalid MAIL response:", scanner.Text())
+	}
+
+	return
+}
+
+func TestServerUnknownArg(t *testing.T) {
+	_, s, c, scanner := testServerAuthenticated(t)
+	defer s.Close()
+	defer c.Close()
+
+	io.WriteString(c, "MAIL FROM:<alice@wonderland.book> RABIIT\r\n")
 	scanner.Scan()
 	if strings.HasPrefix(scanner.Text(), "250 ") {
 		t.Fatal("Invalid MAIL response:", scanner.Text())