Add client and server support for SMTP DSN extension #135
Conversation
| if err := validateLine(to); err != nil { | ||
| return err | ||
| } | ||
| if _, _, err := c.cmd(25, "RCPT TO:<%s>", to); err != nil { | ||
| cmdStr := "RCPT TO:<%s>" |
There was a problem hiding this comment.
Is it safe to just append content to cmdStr? The new content could have user-supplied %v sequences, which fmt will choke on.
We should probably set cmdStr = "RCPT TO:<%s>%s" and have a separate extStr string.
There was a problem hiding this comment.
It applies to MAIL FROM too which uses similar code.
| c.WriteResponse(501, EnhancedCode{5, 5, 2}, "Was expecting MAIL arg syntax of FROM:<address>") | ||
| return | ||
| } | ||
| recipient = strings.Trim(recipient, "<>") |
There was a problem hiding this comment.
Can we keep the TODO about this trim being not quite what we want?
There was a problem hiding this comment.
I also don't like the existing TODO, and proposed to use mail.ParseAddress. How is it implemented in postfix/exim mail servers? Shall we introduce a similar function?
| } | ||
|
|
||
| for key, value := range args { | ||
| switch key { |
There was a problem hiding this comment.
Do we want to error out on unrecognized parameter?
|
|
||
| for key, value := range args { | ||
| switch key { | ||
| case "ORCPT": |
There was a problem hiding this comment.
Can we add a // This space is intentionally left blank comment on the next line? IMHO it's too easy to misread this as a fall-through.
|
Do you have plans to add basic test cases for these new features? |
Yeah, they should be added. |
| @@ -71,7 +122,7 @@ type Session interface { | |||
| // Set return path for currently processed message. | |||
| Mail(from string, opts MailOptions) error | |||
| // Add recipient for currently processed message. | |||
| Rcpt(to string) error | |||
| Rcpt(to string, opts RcptOptions) error | |||
There was a problem hiding this comment.
If #145 is accepted, opts should be switched to a pointer as well.
|
Hi, I just wrote a DSN extension also: v0.18.0...ikedas:go-smtp:dsn_extension I think my code is better in the following respects:
Should I submit a separate PR? |
|
Please feel free to submit a new PR, yeah. Do note that we can still do breaking changes to our API if we feel that they are worth it. |
|
Superseded by #233 |
Breaking change since it changes Client.Rcpt and Session.Rcpt signature.
Closes #32.