From cef83341e590b85163136079702045ac29d9c41c Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Sat, 16 Mar 2024 20:54:10 +0100 Subject: [PATCH] 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. --- dmarcdb/eval.go | 36 ++++++++++++++++++------------------ message/compose.go | 8 ++++++-- message/examples_test.go | 3 ++- tlsrptsend/send.go | 18 +++++++++--------- webmail/api.go | 6 ++---- 5 files changed, 37 insertions(+), 34 deletions(-) diff --git a/dmarcdb/eval.go b/dmarcdb/eval.go index dfee9b7..e15c450 100644 --- a/dmarcdb/eval.go +++ b/dmarcdb/eval.go @@ -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) { - 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() { x := recover() if x == nil { @@ -888,14 +896,6 @@ func composeAggregateReport(ctx context.Context, log mlog.Log, mf *os.File, from 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("To", recipients) 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) { - 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() { x := recover() if x == nil { @@ -1028,14 +1036,6 @@ func composeErrorReport(ctx context.Context, log mlog.Log, mf *os.File, fromAddr 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("To", recipients) xc.Header("Subject", subject) diff --git a/message/compose.go b/message/compose.go index 0ff46d6..4289fee 100644 --- a/message/compose.go +++ b/message/compose.go @@ -32,10 +32,14 @@ type Composer struct { // NewComposer initializes a new composer with a buffered writer around w, and // 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 // catch ErrCompose and optionally ErrMessageSize errors. -func NewComposer(w io.Writer, maxSize int64) *Composer { - return &Composer{bw: bufio.NewWriter(w), maxSize: maxSize} +func NewComposer(w io.Writer, maxSize int64, smtputf8 bool) *Composer { + 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 diff --git a/message/examples_test.go b/message/examples_test.go index 9b465ab..4f31bf8 100644 --- a/message/examples_test.go +++ b/message/examples_test.go @@ -91,7 +91,8 @@ func ExampleComposer() { var b bytes.Buffer // 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. defer func() { diff --git a/tlsrptsend/send.go b/tlsrptsend/send.go index 4370191..131dc82 100644 --- a/tlsrptsend/send.go +++ b/tlsrptsend/send.go @@ -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) { - 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() { x := recover() if x == nil { @@ -633,14 +641,6 @@ func composeMessage(ctx context.Context, log mlog.Log, mf *os.File, policyDomain 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("To", recipients) xc.Subject(subject) diff --git a/webmail/api.go b/webmail/api.go index 5a5d3c1..82c54af 100644 --- a/webmail/api.go +++ b/webmail/api.go @@ -366,8 +366,6 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { 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. smtputf8 := false 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") // 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() { x := recover() if x == nil { @@ -643,7 +641,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { Localpart: rcpt.Localpart, 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 { ival := time.Until(*m.FutureRelease) if ival < 0 {