Add a LenientSMTP option. #12
Conversation
There's probably more to do here. Allow out of order MAIL/RCPTs and so forth.
conn.go
Outdated
| // Match FROM, while accepting '>' as quoted pair and in double quoted strings | ||
| // (?i) makes the regex case insensitive, (?:) is non-grouping sub-match | ||
| re := regexp.MustCompile("(?i)^FROM:\\s*<((?:\\\\>|[^>])+|\"[^\"]+\"@[^>]+)>( [\\w= ]+)?$") | ||
| re := regexp.MustCompile(mailFromRE) |
There was a problem hiding this comment.
Hmmm, this regexp is a leftover from the past I really want to get rid of. Possible to parse this with strings instead?
There was a problem hiding this comment.
I honestly don't know a huge amount about the ESMTP extensions to MAIL FROM but I can go look.
There was a problem hiding this comment.
Hmm, I'll just create a follow-up issue for this one if you don't feel like fixing it right now.
There was a problem hiding this comment.
Done. That removes the need for regexp everywhere.
server.go
Outdated
| @@ -25,6 +25,7 @@ type Server struct { | |||
| MaxIdleSeconds int | |||
| MaxMessageBytes int | |||
| AllowInsecureAuth bool | |||
| LenientSMTP bool | |||
There was a problem hiding this comment.
The default behaviour should probably to be lenient (since RCPT TO is lenient right now). So maybe we can change this to be a "Strict" flag instead?
There was a problem hiding this comment.
I didn't want to check existing behaviour, but yes, my first pass had StrictSMTP - I had to raid the antonyms page to get the most neutral opposite I could find. But happy to switch to StrictSMTP, sure.
|
Can you add a test for the new behaviour? |
This makes the default behavious lenient. And it now removes the need for regexp.
Codecov Report
@@ Coverage Diff @@
## master #12 +/- ##
==========================================
+ Coverage 61.09% 65.56% +4.47%
==========================================
Files 5 5
Lines 568 575 +7
==========================================
+ Hits 347 377 +30
+ Misses 166 145 -21
+ Partials 55 53 -2
Continue to review full report at Codecov.
|
conn.go
Outdated
| // Match FROM, while accepting '>' as quoted pair and in double quoted strings | ||
| // (?i) makes the regex case insensitive, (?:) is non-grouping sub-match | ||
| re := regexp.MustCompile("(?i)^FROM:\\s*<((?:\\\\>|[^>])+|\"[^\"]+\"@[^>]+)>( [\\w= ]+)?$") | ||
| re := regexp.MustCompile(mailFromRE) |
There was a problem hiding this comment.
Done. That removes the need for regexp everywhere.
|
OK, I think that's it. Increased coverage till I found a bug! Good? |
conn.go
Outdated
| re := regexp.MustCompile("(?i)^FROM:\\s*<((?:\\\\>|[^>])+|\"[^\"]+\"@[^>]+)>( [\\w= ]+)?$") | ||
| m := re.FindStringSubmatch(arg) | ||
| if m == nil { | ||
| if (len(arg) < 6) || (strings.ToUpper(arg[0:5]) != "FROM:") { |
conn.go
Outdated
| c.WriteResponse(501, "Was expecting MAIL arg syntax of FROM:<address>") | ||
| return | ||
| } | ||
| fromArgs := strings.Split(strings.Trim(arg[5:], " "), " ") |
There was a problem hiding this comment.
What's the purpose of this strings.Split?
server.go
Outdated
| @@ -25,6 +25,7 @@ type Server struct { | |||
| MaxIdleSeconds int | |||
| MaxMessageBytes int | |||
| AllowInsecureAuth bool | |||
| StrictSMTP bool | |||
There was a problem hiding this comment.
Since this is the smtp package, is it possible to rename this to simply Strict?
There's probably more to do here. Allow out of order MAIL/RCPTs and so
forth.
Fixes #11