From 42f6f9cbb3b39e6ddb9014e746dba57df326dd50 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Thu, 9 Nov 2023 21:15:27 +0100 Subject: [PATCH] change the message composing code from webmail over to message.Composer too --- dmarcdb/eval.go | 51 ++++++++---- message/compose.go | 58 ++++++++------ tlsrptsend/send.go | 26 ++++-- webmail/api.go | 194 ++++++++++----------------------------------- 4 files changed, 132 insertions(+), 197 deletions(-) diff --git a/dmarcdb/eval.go b/dmarcdb/eval.go index bc9a184..6d3f9dd 100644 --- a/dmarcdb/eval.go +++ b/dmarcdb/eval.go @@ -6,6 +6,7 @@ import ( "compress/gzip" "context" "encoding/xml" + "errors" "fmt" "io" "mime" @@ -779,9 +780,9 @@ Period: %s - %s UTC // The attached file follows the naming convention from the RFC. ../rfc/7489:1812 reportFilename := fmt.Sprintf("%s!%s!%d!%d.xml.gz", mox.Conf.Static.HostnameDomain.ASCII, dom.ASCII, beginTime.Unix(), endTime.Add(-time.Second).Unix()) - var addrs []smtp.Address + var addrs []message.NameAddress for _, rcpt := range recipients { - addrs = append(addrs, rcpt.address) + addrs = append(addrs, message.NameAddress{Address: rcpt.address}) } // Compose the message. @@ -841,19 +842,29 @@ Period: %s - %s UTC return true, nil } -func composeAggregateReport(ctx context.Context, log *mlog.Log, mf *os.File, fromAddr smtp.Address, recipients []smtp.Address, subject, text, filename string, reportXMLGzipFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { - xc := message.NewComposer(mf) - defer xc.Recover(&rerr) +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) + defer func() { + x := recover() + if x == nil { + return + } + if err, ok := x.(error); ok && errors.Is(err, message.ErrCompose) { + rerr = err + return + } + 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.Localpart.IsInternational() { + if a.Address.Localpart.IsInternational() { xc.SMTPUTF8 = true break } } - xc.HeaderAddrs("From", []smtp.Address{fromAddr}) + xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}}) xc.HeaderAddrs("To", recipients) xc.Subject(subject) messageID = fmt.Sprintf("<%s>", mox.MessageIDGen(xc.SMTPUTF8)) @@ -904,7 +915,7 @@ func composeAggregateReport(ctx context.Context, log *mlog.Log, mf *os.File, fro // Though this functionality is quite underspecified, we'll do our best to send our // an error report in case our report is too large for all recipients. // ../rfc/7489:1918 -func sendErrorReport(ctx context.Context, log *mlog.Log, fromAddr smtp.Address, recipients []smtp.Address, reportDomain dns.Domain, reportID string, reportMsgSize int64) error { +func sendErrorReport(ctx context.Context, log *mlog.Log, fromAddr smtp.Address, recipients []message.NameAddress, reportDomain dns.Domain, reportID string, reportMsgSize int64) error { log.Debug("no reporting addresses willing to accept report given size, queuing short error message") msgf, err := store.CreateMessageTemp("dmarcreportmsg-out") @@ -915,7 +926,7 @@ func sendErrorReport(ctx context.Context, log *mlog.Log, fromAddr smtp.Address, var recipientStrs []string for _, rcpt := range recipients { - recipientStrs = append(recipientStrs, rcpt.String()) + recipientStrs = append(recipientStrs, rcpt.Address.String()) } subject := fmt.Sprintf("DMARC aggregate reporting error report for %s", reportDomain.ASCII) @@ -941,7 +952,7 @@ Submitting-URI: %s msgSize := int64(len(msgPrefix)) + msgInfo.Size() for _, rcpt := range recipients { - qm := queue.MakeMsg(mox.Conf.Static.Postmaster.Account, fromAddr.Path(), rcpt.Path(), has8bit, smtputf8, msgSize, messageID, []byte(msgPrefix), nil) + qm := queue.MakeMsg(mox.Conf.Static.Postmaster.Account, fromAddr.Path(), rcpt.Address.Path(), has8bit, smtputf8, msgSize, messageID, []byte(msgPrefix), nil) // Don't try as long as regular deliveries, and stop before we would send the // delayed DSN. Though we also won't send that due to IsDMARCReport. qm.MaxAttempts = 5 @@ -958,19 +969,29 @@ Submitting-URI: %s return nil } -func composeErrorReport(ctx context.Context, log *mlog.Log, mf *os.File, fromAddr smtp.Address, recipients []smtp.Address, subject, text string) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { - xc := message.NewComposer(mf) - defer xc.Recover(&rerr) +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) + defer func() { + x := recover() + if x == nil { + return + } + if err, ok := x.(error); ok && errors.Is(err, message.ErrCompose) { + rerr = err + return + } + 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.Localpart.IsInternational() { + if a.Address.Localpart.IsInternational() { xc.SMTPUTF8 = true break } } - xc.HeaderAddrs("From", []smtp.Address{fromAddr}) + xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}}) xc.HeaderAddrs("To", recipients) xc.Header("Subject", subject) messageID = fmt.Sprintf("<%s>", mox.MessageIDGen(xc.SMTPUTF8)) diff --git a/message/compose.go b/message/compose.go index a196778..0ff46d6 100644 --- a/message/compose.go +++ b/message/compose.go @@ -13,46 +13,50 @@ import ( "github.com/mjl-/mox/smtp" ) -var errCompose = errors.New("compose") +var ( + ErrMessageSize = errors.New("message too large") + ErrCompose = errors.New("compose") +) -// Composer helps compose a message. Operations that fail call panic, which can be -// caught with Composer.Recover. Writes are buffered. +// Composer helps compose a message. Operations that fail call panic, which should +// be caught with recover(), checking for ErrCompose and optionally ErrMessageSize. +// Writes are buffered. type Composer struct { - Has8bit bool // Whether message contains 8bit data. - SMTPUTF8 bool // Whether message needs to be sent with SMTPUTF8 extension. + Has8bit bool // Whether message contains 8bit data. + SMTPUTF8 bool // Whether message needs to be sent with SMTPUTF8 extension. + Size int64 // Total bytes written. - bw *bufio.Writer + bw *bufio.Writer + maxSize int64 // If greater than zero, writes beyond maximum size raise ErrMessageSize. } -func NewComposer(w io.Writer) *Composer { - return &Composer{bw: bufio.NewWriter(w)} +// NewComposer initializes a new composer with a buffered writer around w, and +// with a maximum message size if maxSize is greater than zero. +// 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} } // Write implements io.Writer, but calls panic (that is handled higher up) on // i/o errors. func (c *Composer) Write(buf []byte) (int, error) { + if c.maxSize > 0 && c.Size+int64(len(buf)) > c.maxSize { + c.Checkf(ErrMessageSize, "writing message") + } n, err := c.bw.Write(buf) + if n > 0 { + c.Size += int64(n) + } c.Checkf(err, "write") return n, nil } -// Recover recovers the sentinel panic error value, storing it into rerr. -func (c *Composer) Recover(rerr *error) { - x := recover() - if x == nil { - return - } - if err, ok := x.(error); ok && errors.Is(err, errCompose) { - *rerr = err - } else { - panic(x) - } -} - // Checkf checks err, panicing with sentinel error value. func (c *Composer) Checkf(err error, format string, args ...any) { if err != nil { - panic(fmt.Errorf("%w: %s: %v", errCompose, err, fmt.Sprintf(format, args...))) + // We expose the original error too, needed at least for ErrMessageSize. + panic(fmt.Errorf("%w: %w: %v", ErrCompose, err, fmt.Sprintf(format, args...))) } } @@ -67,8 +71,14 @@ func (c *Composer) Header(k, v string) { fmt.Fprintf(c, "%s: %s\r\n", k, v) } +// NameAddress holds both an address display name, and an SMTP path address. +type NameAddress struct { + DisplayName string + Address smtp.Address +} + // HeaderAddrs writes a message header with addresses. -func (c *Composer) HeaderAddrs(k string, l []smtp.Address) { +func (c *Composer) HeaderAddrs(k string, l []NameAddress) { if len(l) == 0 { return } @@ -79,7 +89,7 @@ func (c *Composer) HeaderAddrs(k string, l []smtp.Address) { v += "," linelen++ } - addr := mail.Address{Address: a.Pack(c.SMTPUTF8)} + addr := mail.Address{Name: a.DisplayName, Address: a.Address.Pack(c.SMTPUTF8)} s := addr.String() if v != "" && linelen+1+len(s) > 77 { v += "\r\n\t" diff --git a/tlsrptsend/send.go b/tlsrptsend/send.go index 540a716..5fa5478 100644 --- a/tlsrptsend/send.go +++ b/tlsrptsend/send.go @@ -276,7 +276,7 @@ func sendReportDomain(ctx context.Context, log *mlog.Log, resolver dns.Resolver, return cleanup, fmt.Errorf("looking up current tlsrpt record for reporting addresses: %v", err) } - var recipients []smtp.Address + var recipients []message.NameAddress for _, l := range record.RUAs { for _, s := range l { @@ -292,7 +292,7 @@ func sendReportDomain(ctx context.Context, log *mlog.Log, resolver dns.Resolver, log.Debugx("parsing mailto uri in tlsrpt record rua value, ignoring", err, mlog.Field("rua", s)) continue } - recipients = append(recipients, addr) + recipients = append(recipients, message.NameAddress{Address: addr}) } else if u.Scheme == "https" { // Although "report" is ambiguous and could mean both only the JSON data or an // entire message (including DKIM-Signature) with the JSON data, it appears the @@ -414,7 +414,7 @@ Period: %s - %s UTC msgSize := int64(len(msgPrefix)) + msgInfo.Size() for _, rcpt := range recipients { - qm := queue.MakeMsg(mox.Conf.Static.Postmaster.Account, from.Path(), rcpt.Path(), has8bit, smtputf8, msgSize, messageID, []byte(msgPrefix), nil) + qm := queue.MakeMsg(mox.Conf.Static.Postmaster.Account, from.Path(), rcpt.Address.Path(), has8bit, smtputf8, msgSize, messageID, []byte(msgPrefix), nil) // Don't try as long as regular deliveries, and stop before we would send the // delayed DSN. Though we also won't send that due to IsTLSReport. // ../rfc/8460:1077 @@ -442,19 +442,29 @@ Period: %s - %s UTC return true, nil } -func composeMessage(ctx context.Context, log *mlog.Log, mf *os.File, policyDomain dns.Domain, fromAddr smtp.Address, recipients []smtp.Address, subject, text, filename string, reportFile *os.File) (msgPrefix string, has8bit, smtputf8 bool, messageID string, rerr error) { - xc := message.NewComposer(mf) - defer xc.Recover(&rerr) +func composeMessage(ctx context.Context, log *mlog.Log, mf *os.File, policyDomain dns.Domain, 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) + defer func() { + x := recover() + if x == nil { + return + } + if err, ok := x.(error); ok && errors.Is(err, message.ErrCompose) { + rerr = err + return + } + 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.Localpart.IsInternational() { + if a.Address.Localpart.IsInternational() { xc.SMTPUTF8 = true break } } - xc.HeaderAddrs("From", []smtp.Address{fromAddr}) + xc.HeaderAddrs("From", []message.NameAddress{{Address: fromAddr}}) xc.HeaderAddrs("To", recipients) xc.Subject(subject) // ../rfc/8460:926 diff --git a/webmail/api.go b/webmail/api.go index 66a1847..a162fa4 100644 --- a/webmail/api.go +++ b/webmail/api.go @@ -1,7 +1,6 @@ package webmail import ( - "bufio" "context" "encoding/base64" "encoding/json" @@ -10,7 +9,6 @@ import ( "io" "mime" "mime/multipart" - "mime/quotedprintable" "net" "net/http" "net/mail" @@ -180,48 +178,20 @@ type File struct { DataURI string // Full data of the attachment, with base64 encoding and including content-type. } -// xerrWriter is an io.Writer that panics with a *sherpa.Error when Write -// returns an error. -type xerrWriter struct { - ctx context.Context - w *bufio.Writer - size int64 - max int64 -} - -// Write implements io.Writer, but calls panic (that is handled higher up) on -// i/o errors. -func (w *xerrWriter) Write(buf []byte) (int, error) { - n, err := w.w.Write(buf) - xcheckf(w.ctx, err, "writing message file") - if n > 0 { - w.size += int64(n) - if w.size > w.max { - xcheckuserf(w.ctx, errors.New("max message size reached"), "writing message file") - } - } - return n, err -} - -type nameAddress struct { - Name string - Address smtp.Address -} - // parseAddress expects either a plain email address like "user@domain", or a // single address as used in a message header, like "name ". -func parseAddress(msghdr string) (nameAddress, error) { +func parseAddress(msghdr string) (message.NameAddress, error) { a, err := mail.ParseAddress(msghdr) if err != nil { - return nameAddress{}, nil + return message.NameAddress{}, nil } // todo: parse more fully according to ../rfc/5322:959 path, err := smtp.ParseAddress(a.Address) if err != nil { - return nameAddress{}, err + return message.NameAddress{}, err } - return nameAddress{a.Name, path}, nil + return message.NameAddress{DisplayName: a.Name, Address: path}, nil } func xmailboxID(ctx context.Context, tx *bstore.Tx, mailboxID int64) store.Mailbox { @@ -278,7 +248,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { fromAddr, err := parseAddress(m.From) xcheckuserf(ctx, err, "parsing From address") - var replyTo *nameAddress + var replyTo *message.NameAddress if m.ReplyTo != "" { a, err := parseAddress(m.ReplyTo) xcheckuserf(ctx, err, "parsing Reply-To address") @@ -287,7 +257,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { var recipients []smtp.Address - var toAddrs []nameAddress + var toAddrs []message.NameAddress for _, s := range m.To { addr, err := parseAddress(s) xcheckuserf(ctx, err, "parsing To address") @@ -295,7 +265,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { recipients = append(recipients, addr.Address) } - var ccAddrs []nameAddress + var ccAddrs []message.NameAddress for _, s := range m.Cc { addr, err := parseAddress(s) xcheckuserf(ctx, err, "parsing Cc address") @@ -362,74 +332,19 @@ 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. - xmsgw := &xerrWriter{ctx, bufio.NewWriter(dataFile), 0, w.maxMessageSize} - - isASCII := func(s string) bool { - for _, c := range s { - if c >= 0x80 { - return false - } - } - return true - } - - header := func(k, v string) { - fmt.Fprintf(xmsgw, "%s: %s\r\n", k, v) - } - - headerAddrs := func(k string, l []nameAddress) { - if len(l) == 0 { + xc := message.NewComposer(dataFile, w.maxMessageSize) + defer func() { + x := recover() + if x == nil { return } - v := "" - linelen := len(k) + len(": ") - for _, a := range l { - if v != "" { - v += "," - linelen++ - } - addr := mail.Address{Name: a.Name, Address: a.Address.Pack(smtputf8)} - s := addr.String() - if v != "" && linelen+1+len(s) > 77 { - v += "\r\n\t" - linelen = 1 - } else if v != "" { - v += " " - linelen++ - } - v += s - linelen += len(s) + if err, ok := x.(error); ok && errors.Is(err, message.ErrMessageSize) { + xcheckuserf(ctx, err, "making message") + } else if ok && errors.Is(err, message.ErrCompose) { + xcheckf(ctx, err, "making message") } - fmt.Fprintf(xmsgw, "%s: %s\r\n", k, v) - } - - line := func(w io.Writer) { - _, _ = w.Write([]byte("\r\n")) - } - - text := m.TextBody - if !strings.HasSuffix(text, "\n") { - text += "\n" - } - text = strings.ReplaceAll(text, "\n", "\r\n") - - charset := "us-ascii" - if !isASCII(text) { - charset = "utf-8" - } - - var cte string - if message.NeedsQuotedPrintable(text) { - var sb strings.Builder - _, err := io.Copy(quotedprintable.NewWriter(&sb), strings.NewReader(text)) - xcheckf(ctx, err, "converting text to quoted printable") - text = sb.String() - cte = "quoted-printable" - } else if has8bit || charset == "utf-8" { - cte = "8bit" - } else { - cte = "7bit" - } + panic(x) + }() // todo spec: can we add an Authentication-Results header that indicates this is an authenticated message? the "auth" method is for SMTP AUTH, which this isn't. ../rfc/8601 https://www.iana.org/assignments/email-auth/email-auth.xhtml @@ -454,39 +369,19 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { } // Outer message headers. - headerAddrs("From", []nameAddress{fromAddr}) + xc.HeaderAddrs("From", []message.NameAddress{fromAddr}) if replyTo != nil { - headerAddrs("Reply-To", []nameAddress{*replyTo}) + xc.HeaderAddrs("Reply-To", []message.NameAddress{*replyTo}) } - headerAddrs("To", toAddrs) - headerAddrs("Cc", ccAddrs) - - var subjectValue string - subjectLineLen := len("Subject: ") - subjectWord := false - for i, word := range strings.Split(m.Subject, " ") { - if !smtputf8 && !isASCII(word) { - word = mime.QEncoding.Encode("utf-8", word) - } - if i > 0 { - subjectValue += " " - subjectLineLen++ - } - if subjectWord && subjectLineLen+len(word) > 77 { - subjectValue += "\r\n\t" - subjectLineLen = 1 - } - subjectValue += word - subjectLineLen += len(word) - subjectWord = true - } - if subjectValue != "" { - header("Subject", subjectValue) + xc.HeaderAddrs("To", toAddrs) + xc.HeaderAddrs("Cc", ccAddrs) + if m.Subject != "" { + xc.Subject(m.Subject) } messageID := fmt.Sprintf("<%s>", mox.MessageIDGen(smtputf8)) - header("Message-Id", messageID) - header("Date", time.Now().Format(message.RFC5322Z)) + xc.Header("Message-Id", messageID) + xc.Header("Date", time.Now().Format(message.RFC5322Z)) // Add In-Reply-To and References headers. if m.ResponseMessageID > 0 { xdbread(ctx, acc, func(tx *bstore.Tx) { @@ -504,39 +399,39 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { if rp.Envelope == nil { return } - header("In-Reply-To", rp.Envelope.MessageID) + xc.Header("In-Reply-To", rp.Envelope.MessageID) ref := h.Get("References") if ref == "" { ref = h.Get("In-Reply-To") } if ref != "" { - header("References", ref+"\r\n\t"+rp.Envelope.MessageID) + xc.Header("References", ref+"\r\n\t"+rp.Envelope.MessageID) } else { - header("References", rp.Envelope.MessageID) + xc.Header("References", rp.Envelope.MessageID) } }) } if m.UserAgent != "" { - header("User-Agent", m.UserAgent) + xc.Header("User-Agent", m.UserAgent) } if m.RequireTLS != nil && !*m.RequireTLS { - header("TLS-Required", "No") + xc.Header("TLS-Required", "No") } - header("MIME-Version", "1.0") + xc.Header("MIME-Version", "1.0") if len(m.Attachments) > 0 || len(m.ForwardAttachments.Paths) > 0 { - mp := multipart.NewWriter(xmsgw) - header("Content-Type", fmt.Sprintf(`multipart/mixed; boundary="%s"`, mp.Boundary())) - line(xmsgw) + mp := multipart.NewWriter(xc) + xc.Header("Content-Type", fmt.Sprintf(`multipart/mixed; boundary="%s"`, mp.Boundary())) + xc.Line() - ct := mime.FormatMediaType("text/plain", map[string]string{"charset": charset}) + textBody, ct, cte := xc.TextPart(m.TextBody) textHdr := textproto.MIMEHeader{} textHdr.Set("Content-Type", ct) textHdr.Set("Content-Transfer-Encoding", cte) textp, err := mp.CreatePart(textHdr) xcheckf(ctx, err, "adding text part to message") - _, err = textp.Write([]byte(text)) + _, err = textp.Write(textBody) xcheckf(ctx, err, "writing text part") xaddPart := func(ct, filename string) io.Writer { @@ -650,15 +545,14 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { err = mp.Close() xcheckf(ctx, err, "writing mime multipart") } else { - ct := mime.FormatMediaType("text/plain", map[string]string{"charset": charset}) - header("Content-Type", ct) - header("Content-Transfer-Encoding", cte) - line(xmsgw) - xmsgw.Write([]byte(text)) + textBody, ct, cte := xc.TextPart(m.TextBody) + xc.Header("Content-Type", ct) + xc.Header("Content-Transfer-Encoding", cte) + xc.Line() + xc.Write([]byte(textBody)) } - err = xmsgw.w.Flush() - xcheckf(ctx, err, "writing message") + xc.Flush() // Add DKIM-Signature headers. var msgPrefix string @@ -680,7 +574,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { } for _, rcpt := range recipients { rcptMsgPrefix := recvHdrFor(rcpt.Pack(smtputf8)) + msgPrefix - msgSize := int64(len(rcptMsgPrefix)) + xmsgw.size + msgSize := int64(len(rcptMsgPrefix)) + xc.Size toPath := smtp.Path{ Localpart: rcpt.Localpart, IPDomain: dns.IPDomain{Domain: rcpt.Domain}, @@ -752,7 +646,7 @@ func (w Webmail) MessageSubmit(ctx context.Context, m SubmitMessage) { MailboxID: sentmb.ID, MailboxOrigID: sentmb.ID, Flags: store.Flags{Notjunk: true, Seen: true}, - Size: int64(len(msgPrefix)) + xmsgw.size, + Size: int64(len(msgPrefix)) + xc.Size, MsgPrefix: []byte(msgPrefix), }