Skip to content

Commit

Permalink
Refactor DSN handling from client.go to smtp.go
Browse files Browse the repository at this point in the history 
This PR refactors the the DSN (RFC 1891) SMTP client handling, that was introduced in f4cdc61.

While most of the Client options stay the same, the whole workaround logic for the SMTP client has been removed and added as part of the SMTP client instead.

This was we got rid of the Client's own `mail()`, `rcpt()`, `dsnRcpt()`, `dsnMail()` methods as well as the copies of the `cmd()` and `validateLine()` methods. The Client is now using the proper `Mail()` and `Rcpt()` methods of the SMTP client instead.
  • Loading branch information
@wneessen
wneessen committed Jan 18, 2023
1 parent f48c832 commit 63d8cef
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 114 deletions.
88 changes: 0 additions & 88 deletions client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,91 +633,3 @@ func (c *Client) auth() error {
}
return nil
}

// mail is an extension to the Go std library mail method. It decideds wether to call the
// original mail method from the std library or in case DSN is enabled on the Client to
// call our own method instead
func (c *Client) mail(f string) error {
ok, _ := c.sc.Extension("DSN")
if ok && c.dsn {
return c.dsnMail(f)
}
return c.sc.Mail(f)
}

// rcpt is an extension to the Go std library rcpt method. It decideds wether to call
// original rcpt method from the std library or in case DSN is enabled on the Client to
// call our own method instead
func (c *Client) rcpt(t string) error {
ok, _ := c.sc.Extension("DSN")
if ok && c.dsn {
return c.dsnRcpt(t)
}
return c.sc.Rcpt(t)
}

// dsnRcpt issues a RCPT command to the server using the provided email address.
// A call to rcpt must be preceded by a call to mail and may be followed by
// a Data call or another rcpt call.
//
// This is a copy of the original Go std library net/smtp function with additions
// for the DSN extension
func (c *Client) dsnRcpt(t string) error {
if err := validateLine(t); err != nil {
return err
}
if len(c.dsnrntype) <= 0 {
return c.sc.Rcpt(t)
}

rno := strings.Join(c.dsnrntype, ",")
_, _, err := c.cmd(25, "RCPT TO:<%s> NOTIFY=%s", t, rno)
return err
}

// dsnMail issues a MAIL command to the server using the provided email address.
// If the server supports the 8BITMIME extension, mail adds the BODY=8BITMIME
// parameter. If the server supports the SMTPUTF8 extension, mail adds the
// SMTPUTF8 parameter.
// This initiates a mail transaction and is followed by one or more rcpt calls.
//
// This is a copy of the original Go std library net/smtp function with additions
// for the DSN extension
func (c *Client) dsnMail(f string) error {
if err := validateLine(f); err != nil {
return err
}
cmdStr := "MAIL FROM:<%s>"
if ok, _ := c.sc.Extension("8BITMIME"); ok {
cmdStr += " BODY=8BITMIME"
}
if ok, _ := c.sc.Extension("SMTPUTF8"); ok {
cmdStr += " SMTPUTF8"
}
cmdStr += fmt.Sprintf(" RET=%s", c.dsnmrtype)

_, _, err := c.cmd(250, cmdStr, f)
return err
}

// validateLine checks to see if a line has CR or LF as per RFC 5321
// This is a 1:1 copy of the method from the original Go std library net/smtp
func validateLine(line string) error {
if strings.ContainsAny(line, "\n\r") {
return errors.New("smtp: A line must not contain CR or LF")
}
return nil
}

// cmd is a convenience function that sends a command and returns the response
// This is a 1:1 copy of the method from the original Go std library net/smtp
func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, string, error) {
id, err := c.sc.Text.Cmd(format, args...)
if err != nil {
return 0, "", err
}
c.sc.Text.StartResponse(id)
defer c.sc.Text.EndResponse(id)
code, msg, err := c.sc.Text.ReadResponse(expectCode)
return code, msg, err
}
13 changes: 11 additions & 2 deletions client_119.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

package mail

import "strings"

// Send sends out the mail message
func (c *Client) Send(ml ...*Msg) error {
if cerr := c.checkConn(); cerr != nil {
Expand Down Expand Up @@ -38,7 +40,12 @@ func (c *Client) Send(ml ...*Msg) error {
continue
}

if err := c.mail(f); err != nil {
if c.dsn {
if c.dsnmrtype != "" {
c.sc.SetDSNMailReturnOption(string(c.dsnmrtype))
}
}
if err := c.sc.Mail(f); err != nil {
se := &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)}
if reserr := c.sc.Reset(); reserr != nil {
se.errlist = append(se.errlist, reserr)
Expand All @@ -51,8 +58,10 @@ func (c *Client) Send(ml ...*Msg) error {
rse := &SendError{}
rse.errlist = make([]error, 0)
rse.rcpt = make([]string, 0)
rno := strings.Join(c.dsnrntype, ",")
c.sc.SetDSNRcptNotifyOption(rno)
for _, r := range rl {
if err := c.rcpt(r); err != nil {
if err := c.sc.Rcpt(r); err != nil {
rse.Reason = ErrSMTPRcptTo
rse.errlist = append(rse.errlist, err)
rse.rcpt = append(rse.rcpt, r)
Expand Down
12 changes: 10 additions & 2 deletions client_120.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package mail

import (
"errors"
"strings"
)

// Send sends out the mail message
Expand Down Expand Up @@ -39,7 +40,12 @@ func (c *Client) Send(ml ...*Msg) (rerr error) {
continue
}

if err := c.mail(f); err != nil {
if c.dsn {
if c.dsnmrtype != "" {
c.sc.SetDSNMailReturnOption(string(c.dsnmrtype))
}
}
if err := c.sc.Mail(f); err != nil {
m.sendError = &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)}
rerr = errors.Join(rerr, m.sendError)
if reserr := c.sc.Reset(); reserr != nil {
Expand All @@ -51,8 +57,10 @@ func (c *Client) Send(ml ...*Msg) (rerr error) {
rse := &SendError{}
rse.errlist = make([]error, 0)
rse.rcpt = make([]string, 0)
rno := strings.Join(c.dsnrntype, ",")
c.sc.SetDSNRcptNotifyOption(rno)
for _, r := range rl {
if err := c.rcpt(r); err != nil {
if err := c.sc.Rcpt(r); err != nil {
rse.Reason = ErrSMTPRcptTo
rse.errlist = append(rse.errlist, err)
rse.rcpt = append(rse.rcpt, r)
Expand Down
21 changes: 0 additions & 21 deletions client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,27 +994,6 @@ func TestClient_auth(t *testing.T) {
}
}

// TestValidateLine tests the validateLine() method
func TestValidateLine(t *testing.T) {
tests := []struct {
name string
value string
sf bool
}{
{"valid line", "valid line", false},
{`invalid line: \n`, "invalid line\n", true},
{`invalid line: \r`, "invalid line\r", true},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := validateLine(tt.value); err != nil && !tt.sf {
t.Errorf("validateLine failed: %s", err)
}
})
}
}

// TestClient_Send_MsgSendError tests the Client.Send method with a broken recipient and verifies
// that the SendError type works properly
func TestClient_Send_MsgSendError(t *testing.T) {
Expand Down
25 changes: 24 additions & 1 deletion smtp/smtp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
// 8BITMIME RFC 1652
// AUTH RFC 2554
// STARTTLS RFC 3207
// DSN RFC 1891
package smtp

import (
Expand Down Expand Up @@ -50,9 +51,12 @@ type Client struct {
localName string // the name to use in HELO/EHLO
didHello bool // whether we've said HELO/EHLO
helloError error // the error from the hello

// debug logging
debug bool // debug logging is enabled
logger *log.Logger // logger will be used for debug logging
// DSN support
dsnmrtype string // dsnmrtype defines the mail return option in case DSN is enabled
dsnrntype string // dsnrntype defines the recipient notify option in case DSN is enabled
}

// logDirection is a type wrapper for the direction a debug log message goes
Expand Down Expand Up @@ -256,6 +260,10 @@ func (c *Client) Mail(from string) error {
if _, ok := c.ext["SMTPUTF8"]; ok {
cmdStr += " SMTPUTF8"
}
_, ok := c.ext["DSN"]
if ok && c.dsnmrtype != "" {
cmdStr += fmt.Sprintf(" RET=%s", c.dsnmrtype)
}
}
_, _, err := c.cmd(250, cmdStr, from)
return err
Expand All @@ -268,6 +276,11 @@ func (c *Client) Rcpt(to string) error {
if err := validateLine(to); err != nil {
return err
}
_, ok := c.ext["DSN"]
if ok && c.dsnrntype != "" {
_, _, err := c.cmd(25, "RCPT TO:<%s> NOTIFY=%s", to, c.dsnrntype)
return err
}
_, _, err := c.cmd(25, "RCPT TO:<%s>", to)
return err
}
Expand Down Expand Up @@ -434,6 +447,16 @@ func (c *Client) SetDebugLog(v bool) {
c.logger = nil
}

// SetDSNMailReturnOption sets the DSN mail return option for the Mail method
func (c *Client) SetDSNMailReturnOption(d string) {
c.dsnmrtype = d
}

// SetDSNRcptNotifyOption sets the DSN recipient notify option for the Mail method
func (c *Client) SetDSNRcptNotifyOption(d string) {
c.dsnrntype = d
}

// debugLog checks if the debug flag is set and if so logs the provided message to StdErr
func (c *Client) debugLog(d logDirection, f string, a ...interface{}) {
if c.debug {
Expand Down

0 comments on commit 63d8cef

Please sign in to comment.