make it harder to forget to set smtputf8 on message.Composer

we should do better: first gather all headers, and only write it when we start
on the body, and then calculate smtputf8 ourselves.
This commit is contained in:
Mechiel Lukkien 2024-03-16 20:54:10 +01:00
parent 8b2c97808d
commit cef83341e5
No known key found for this signature in database
5 changed files with 37 additions and 34 deletions

View file

@ -875,7 +875,15 @@ Period: %s - %s UTC
} }
func composeAggregateReport(ctx context.Context, log mlog.Log, mf *os.File, fromAddr smtp.Address, recipients []message.NameAddress, subject, text, filename string, reportXMLGzipFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { func composeAggregateReport(ctx context.Context, log mlog.Log, mf *os.File, fromAddr smtp.Address, recipients []message.NameAddress, subject, text, filename string, reportXMLGzipFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) {
xc := message.NewComposer(mf, 100*1024*1024) // We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
smtputf8 = fromAddr.Localpart.IsInternational()
for _, r := range recipients {
if smtputf8 {
smtputf8 = r.Address.Localpart.IsInternational()
break
}
}
xc := message.NewComposer(mf, 100*1024*1024, smtputf8)
defer func() { defer func() {
x := recover() x := recover()
if x == nil { if x == nil {
@ -888,14 +896,6 @@ func composeAggregateReport(ctx context.Context, log mlog.Log, mf *os.File, from
panic(x) panic(x)
}() }()
// We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
for _, a := range recipients {
if a.Address.Localpart.IsInternational() {
xc.SMTPUTF8 = true
break
}
}
xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}}) xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}})
xc.HeaderAddrs("To", recipients) xc.HeaderAddrs("To", recipients)
xc.Subject(subject) xc.Subject(subject)
@ -1015,7 +1015,15 @@ Submitting-URI: %s
} }
func composeErrorReport(ctx context.Context, log mlog.Log, mf *os.File, fromAddr smtp.Address, recipients []message.NameAddress, subject, text string) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { func composeErrorReport(ctx context.Context, log mlog.Log, mf *os.File, fromAddr smtp.Address, recipients []message.NameAddress, subject, text string) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) {
xc := message.NewComposer(mf, 100*1024*1024) // We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
smtputf8 = fromAddr.Localpart.IsInternational()
for _, r := range recipients {
if smtputf8 {
smtputf8 = r.Address.Localpart.IsInternational()
break
}
}
xc := message.NewComposer(mf, 100*1024*1024, smtputf8)
defer func() { defer func() {
x := recover() x := recover()
if x == nil { if x == nil {
@ -1028,14 +1036,6 @@ func composeErrorReport(ctx context.Context, log mlog.Log, mf *os.File, fromAddr
panic(x) panic(x)
}() }()
// We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
for _, a := range recipients {
if a.Address.Localpart.IsInternational() {
xc.SMTPUTF8 = true
break
}
}
xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}}) xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}})
xc.HeaderAddrs("To", recipients) xc.HeaderAddrs("To", recipients)
xc.Header("Subject", subject) xc.Header("Subject", subject)

View file

@ -32,10 +32,14 @@ type Composer struct {
// NewComposer initializes a new composer with a buffered writer around w, and // NewComposer initializes a new composer with a buffered writer around w, and
// with a maximum message size if maxSize is greater than zero. // with a maximum message size if maxSize is greater than zero.
//
// smtputf8 must be set when the message must be delivered with smtputf8: if any
// email address localpart has non-ascii (utf-8).
//
// Operations on a Composer do not return an error. Caller must use recover() to // Operations on a Composer do not return an error. Caller must use recover() to
// catch ErrCompose and optionally ErrMessageSize errors. // catch ErrCompose and optionally ErrMessageSize errors.
func NewComposer(w io.Writer, maxSize int64) *Composer { func NewComposer(w io.Writer, maxSize int64, smtputf8 bool) *Composer {
return &Composer{bw: bufio.NewWriter(w), maxSize: maxSize} return &Composer{bw: bufio.NewWriter(w), maxSize: maxSize, SMTPUTF8: smtputf8, Has8bit: smtputf8}
} }
// Write implements io.Writer, but calls panic (that is handled higher up) on // Write implements io.Writer, but calls panic (that is handled higher up) on

View file

@ -91,7 +91,8 @@ func ExampleComposer() {
var b bytes.Buffer var b bytes.Buffer
// NewComposer. Keep in mind that operations on a Composer will panic on error. // NewComposer. Keep in mind that operations on a Composer will panic on error.
xc := message.NewComposer(&b, 10*1024*1024) const smtputf8 = false
xc := message.NewComposer(&b, 10*1024*1024, smtputf8)
// Catch and handle errors when composing. // Catch and handle errors when composing.
defer func() { defer func() {

View file

@ -620,7 +620,15 @@ Period: %s - %s UTC
} }
func composeMessage(ctx context.Context, log mlog.Log, mf *os.File, policyDomain dns.Domain, confDKIM config.DKIM, fromAddr smtp.Address, recipients []message.NameAddress, subject, text, filename string, reportFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { func composeMessage(ctx context.Context, log mlog.Log, mf *os.File, policyDomain dns.Domain, confDKIM config.DKIM, fromAddr smtp.Address, recipients []message.NameAddress, subject, text, filename string, reportFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) {
xc := message.NewComposer(mf, 100*1024*1024) // We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
smtputf8 = fromAddr.Localpart.IsInternational()
for _, r := range recipients {
if smtputf8 {
smtputf8 = r.Address.Localpart.IsInternational()
break
}
}
xc := message.NewComposer(mf, 100*1024*1024, smtputf8)
defer func() { defer func() {
x := recover() x := recover()
if x == nil { if x == nil {
@ -633,14 +641,6 @@ func composeMessage(ctx context.Context, log mlog.Log, mf *os.File, policyDomain
panic(x) panic(x)
}() }()
// We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
for _, a := range recipients {
if a.Address.Localpart.IsInternational() {
xc.SMTPUTF8 = true
break
}
}
xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}}) xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}})
xc.HeaderAddrs("To", recipients) xc.HeaderAddrs("To", recipients)
xc.Subject(subject) xc.Subject(subject)

View file

@ -366,8 +366,6 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
xcheckf(ctx, err, "checking send limit") xcheckf(ctx, err, "checking send limit")
}) })
has8bit := false // We update this later on.
// We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains. // We only use smtputf8 if we have to, with a utf-8 localpart. For IDNA, we use ASCII domains.
smtputf8 := false smtputf8 := false
for _, a := range recipients { for _, a := range recipients {
@ -387,7 +385,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
defer store.CloseRemoveTempFile(log, dataFile, "message to submit") defer store.CloseRemoveTempFile(log, dataFile, "message to submit")
// If writing to the message file fails, we abort immediately. // If writing to the message file fails, we abort immediately.
xc := message.NewComposer(dataFile, w.maxMessageSize) xc := message.NewComposer(dataFile, w.maxMessageSize, smtputf8)
defer func() { defer func() {
x := recover() x := recover()
if x == nil { if x == nil {
@ -643,7 +641,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) {
Localpart: rcpt.Localpart, Localpart: rcpt.Localpart,
IPDomain: dns.IPDomain{Domain: rcpt.Domain}, IPDomain: dns.IPDomain{Domain: rcpt.Domain},
} }
qm := queue.MakeMsg(fromPath, toPath, has8bit, smtputf8, msgSize, messageID, []byte(rcptMsgPrefix), m.RequireTLS, now) qm := queue.MakeMsg(fromPath, toPath, xc.Has8bit, xc.SMTPUTF8, msgSize, messageID, []byte(rcptMsgPrefix), m.RequireTLS, now)
if m.FutureRelease != nil { if m.FutureRelease != nil {
ival := time.Until(*m.FutureRelease) ival := time.Until(*m.FutureRelease)
if ival < 0 { if ival < 0 {