From d2fc783c6e36dc5df74d3b1bf6d0147a10b2bab0 Mon Sep 17 00:00:00 2001 From: butterrobot Date: Wed, 9 Sep 2026 20:58:52 +0000 Subject: [PATCH 1/3] feat: add a per-recipient Format option to convert HTML bodies (FMG-9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Targets that render no HTML — a Mattermost direct message, for one — used to receive the markup of an HTML-only message verbatim. `Format` on a recipient (or on `[CatchAll]`) now renders an HTML body as `text` or `markdown` before it is forwarded; the default `raw` keeps forwarding the message unchanged. Only an HTML body is ever rewritten: a message that arrived as plain text is what the sender chose to write, and is forwarded untouched whatever the recipient asked for. Converting a body first required reading it correctly, which fixes three faults that were also spoiling raw delivery: - Content-Transfer-Encoding was never undone outside multipart parts, so a quoted-printable or base64 body reached the target as "=E2=80=99". - A non-UTF-8 charset was forwarded as its raw bytes, turning every accented character into mojibake. - The multipart walk did not descend into nested containers, so the multipart/mixed wrapping a multipart/alternative that every forwarded message is produced an empty notification. Attachments are now skipped when picking the body, an empty text/plain alternative no longer wins over the HTML the sender actually wrote, and a multipart Content-Type without a boundary is refused as malformed rather than silently yielding nothing. The renderer is built on golang.org/x/net/html, already an indirect dependency, and aims at chat and push notifications rather than at reproducing the document: links keep their destination, images without alt text and hidden preheaders are dropped, and layout tables become one line per row. A body too deeply nested for the parser is forwarded unchanged, since reformatting is a courtesy to the target rather than a condition of delivery. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 51 ++++ backend.go | 3 +- backend_test.go | 66 +++++ config.go | 35 ++- config_test.go | 52 ++++ email.go | 378 +++++++++++++++++++----- email_test.go | 217 ++++++++++++++ format.go | 46 +++ go.mod | 3 +- html.go | 760 ++++++++++++++++++++++++++++++++++++++++++++++++ html_test.go | 228 +++++++++++++++ 11 files changed, 1764 insertions(+), 75 deletions(-) create mode 100644 format.go create mode 100644 html.go create mode 100644 html_test.go diff --git a/README.md b/README.md index 057a7a8..f0730a7 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,11 @@ Targets = [ "discord://token@id", "slack://token@channel" ] +# Optional: how an HTML message body should reach these targets. +# "raw" forward the message unchanged (default) +# "text" render an HTML body as plain text +# "markdown" render an HTML body as Markdown +Format = "markdown" # Single Target (Deprecated) # The Target field is still supported for backward compatibility @@ -42,6 +47,8 @@ Target = "ntfy://ntfy.sh/legacy-topic?tags=thing" # Will show deprecation warni [CatchAll] # Shoutrrr services to forward unmatched emails to Targets = ["ntfy://ntfy.sh/catch-all-topic?tags=unmatched"] +# Format applies to the catch-all as well +Format = "text" ``` The server refuses to start if the configuration cannot forward anything — no @@ -63,6 +70,50 @@ malformed `Content-Type`, for instance — which no retry could fix. A partial failure is logged but still accepted, since a retry would redeliver to every target and duplicate the notification on the ones that already have it. +### Converting HTML messages + +Plenty of mail carries nothing but HTML, and plenty of notification services +render none of it — a Mattermost direct message shows the markup verbatim. +Setting `Format` on a recipient (or on `[CatchAll]`) rewrites an HTML body +before it is forwarded: + +| `Format` | Effect | +| ---------- | ------------------------------------------------- | +| `raw` | Forward the body exactly as it arrived. (default) | +| `text` | Render an HTML body as plain text. | +| `markdown` | Render an HTML body as Markdown. | + +Only an HTML body is ever rewritten. A message that arrives as plain text — +including the `text/plain` alternative of a message that carries both — is what +the sender chose to write and is forwarded untouched whatever `Format` says. + +The output is aimed at chat and push notifications rather than at reproducing +the document, so: + +- Links keep their destination: `[label](url)` in Markdown, `label (url)` in + plain text. Destinations a reader cannot open (`#anchors`, `javascript:`, + inline `cid:` attachments) are dropped and their text is kept. +- Images are dropped unless they have alt text, which removes tracking pixels + and sliced-up banners. +- Tables become one line per row, since mail lays itself out in tables far more + often than it tabulates anything. +- Hidden elements — the preheader line written for the inbox preview — are left + out. +- A `
` becomes a single newline rather than a Markdown hard break, because + every service this forwards to renders one as a line break. + +Message text is escaped in Markdown mode so a subject line's `*` or a leading +`-` cannot turn into formatting. + +If a body is too deeply nested for the HTML parser it is forwarded unchanged +and a warning is logged: reformatting is a courtesy to the target, not a +condition of delivery. + +Bodies are decoded before they are rendered, so `quoted-printable` and `base64` +transfer encodings and non-UTF-8 character sets (`iso-8859-1`, +`windows-1252`, …) reach the target as readable text in every format, +including `raw`. + ### From releases - Grab the latest release from the [releases page](https://git.nakama.town/fmartingr/smtp2shoutrrr/releases) diff --git a/backend.go b/backend.go index fd05096..50ae9cf 100644 --- a/backend.go +++ b/backend.go @@ -66,7 +66,7 @@ func (bkd *Backend) sendNotifications(recipient ConfigRecipient, email ReceivedE } // Prepare email body once (reuse for all targets) - body, err := email.Body() + body, err := email.FormattedBody(recipient.Format) if err != nil { slog.Error("Error getting email body", slog.String("err", err.Error())) return fmt.Errorf("failed to get email body: %w", err) @@ -74,6 +74,7 @@ func (bkd *Backend) sendNotifications(recipient ConfigRecipient, email ReceivedE slog.Debug("Prepared email body", slog.Int("body_length", len(body)), + slog.String("format", string(recipient.Format)), slog.String("subject", email.Msg.Header.Get("Subject")), slog.String("content_type", email.Msg.Header.Get("Content-Type"))) diff --git a/backend_test.go b/backend_test.go index e4c5eb4..feea93c 100644 --- a/backend_test.go +++ b/backend_test.go @@ -699,3 +699,69 @@ func TestRecipientWithoutTargetsIsRejectedPermanently(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "550", "a retry cannot add targets to a configuration") } + +// The case the option exists for: a target that renders no HTML, receiving +// mail that carries nothing else. +func TestHTMLBodyIsConvertedForTheRecipientsFormat(t *testing.T) { + notifications := make(chan string, 4) + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + + return + } + + notifications <- string(body) + w.WriteHeader(http.StatusOK) + })) + defer mockServer.Close() + + config := &Config{ + Port: 0, + Username: "testuser", + Password: "testpass", + Recipients: []ConfigRecipient{ + { + Addresses: []string{"markdown@example.com"}, + Targets: []string{"generic+" + mockServer.URL + "/?template=json"}, + Format: FormatMarkdown, + }, + { + Addresses: []string{"raw@example.com"}, + Targets: []string{"generic+" + mockServer.URL + "/?template=json"}, + }, + }, + } + + addr, _, _ := startServer(t, NewSMTPServer(config)) + auth := smtp.PlainAuth("", config.Username, config.Password, "localhost") + + message := []byte(strings.Join([]string{ + "Subject: Deploy failed", + `Content-Type: multipart/mixed; boundary="b"`, + "", + "--b", + "Content-Type: text/html; charset=utf-8", + "Content-Transfer-Encoding: quoted-printable", + "", + "

The staging deploy =E2=80=94 run 4=", + "2 =E2=80=94 failed.

", + "--b--", + "", + }, "\r\n")) + + for _, recipient := range []string{"markdown@example.com", "raw@example.com"} { + require.NoError(t, smtp.SendMail(addr, auth, "sender@example.com", []string{recipient}, message)) + } + + converted := <-notifications + require.Contains(t, converted, + `The **staging** deploy — [run 42](https://ci.example.com/42) — failed.`) + require.NotContains(t, converted, "") + require.NotContains(t, converted, "=E2=80=94", "the transfer encoding is undone before the markup is") + + unconverted := <-notifications + require.Contains(t, unconverted, `staging`, + "a recipient that asked for nothing still receives the message as it arrived") +} diff --git a/config.go b/config.go index 61411e8..d6a8c63 100644 --- a/config.go +++ b/config.go @@ -67,6 +67,14 @@ func (c *Config) SetDefaults() { if c.Port == 0 { c.Port = 11125 } + + for i := range c.Recipients { + c.Recipients[i].Format = c.Recipients[i].Format.normalize() + } + + if c.CatchAll != nil { + c.CatchAll.Format = c.CatchAll.Format.normalize() + } } // Validate rejects configuration that would leave the server relaying for @@ -95,10 +103,20 @@ func (c *Config) Validate() error { return fmt.Errorf("recipient %d (%s) has no usable Targets", i, strings.Join(r.Addresses, ",")) } + if !r.Format.valid() { + return fmt.Errorf("recipient %d (%s) has an unknown Format %q, expected one of: %s", + i, strings.Join(r.Addresses, ","), r.Format, formatNames()) + } } - if c.CatchAll != nil && len(c.CatchAll.GetTargetURLs()) == 0 { - return errors.New("CatchAll has no usable Targets") + if c.CatchAll != nil { + if len(c.CatchAll.GetTargetURLs()) == 0 { + return errors.New("CatchAll has no usable Targets") + } + if !c.CatchAll.Format.valid() { + return fmt.Errorf("CatchAll has an unknown Format %q, expected one of: %s", + c.CatchAll.Format, formatNames()) + } } if len(c.Recipients) == 0 && c.CatchAll == nil { @@ -109,9 +127,16 @@ func (c *Config) Validate() error { } type ConfigRecipient struct { - Addresses []string // email addresses - Target string // deprecated: use Targets instead - Targets []string // shoutrrr addresses (supports multiple) + Addresses []string // email addresses + Target string // deprecated: use Targets instead + Targets []string // shoutrrr addresses (supports multiple) + + // Format is the representation these targets want an HTML message body + // in: raw (the default, forwarding the message unchanged), text or + // markdown. Targets that render neither HTML nor Markdown — a Mattermost + // direct message, for one — otherwise receive the markup verbatim. + Format BodyFormat + targetURLs []*url.URL // cached parsed URLs } diff --git a/config_test.go b/config_test.go index 81d898c..e02263e 100644 --- a/config_test.go +++ b/config_test.go @@ -191,3 +191,55 @@ Targets = ["ntfy://ntfy.sh/catch-all"] require.Contains(t, logged.String(), "ignoring unknown key in config file") require.Contains(t, logged.String(), "Recipient") } + +func TestLoadConfigReadsFormat(t *testing.T) { + config, err := LoadConfig(writeConfig(t, credentials+` +[[Recipients]] +Addresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] +Format = "Markdown" + +[[Recipients]] +Addresses = ["plain@example.com"] +Targets = ["ntfy://ntfy.sh/plain"] +Format = "text" + +[[Recipients]] +Addresses = ["asis@example.com"] +Targets = ["ntfy://ntfy.sh/asis"] + +[CatchAll] +Targets = ["ntfy://ntfy.sh/catch-all"] +Format = "markdown" +`)) + require.NoError(t, err) + + require.Equal(t, FormatMarkdown, config.Recipients[0].Format, "the option is not case sensitive") + require.Equal(t, FormatText, config.Recipients[1].Format) + require.Equal(t, FormatRaw, config.Recipients[2].Format, "an unset Format forwards the message unchanged") + require.Equal(t, FormatMarkdown, config.CatchAll.Format) +} + +// A typo here is silent otherwise: the server starts and forwards raw HTML to +// a target that cannot render it. +func TestLoadConfigRejectsUnknownFormat(t *testing.T) { + for name, contents := range map[string]string{ + "on a recipient": ` +[[Recipients]] +Addresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] +Format = "md" +`, + "on the catch-all": ` +[CatchAll] +Targets = ["ntfy://ntfy.sh/catch-all"] +Format = "plaintext" +`, + } { + t.Run(name, func(t *testing.T) { + _, err := LoadConfig(writeConfig(t, credentials+contents)) + require.Error(t, err) + require.Contains(t, err.Error(), "raw, text, markdown") + }) + } +} diff --git a/email.go b/email.go index 3612bb5..4f07958 100644 --- a/email.go +++ b/email.go @@ -1,15 +1,19 @@ package smtp2shoutrrr import ( - "bytes" + "encoding/base64" "errors" "fmt" "io" "log/slog" "mime" "mime/multipart" + "mime/quotedprintable" "net/mail" + "net/textproto" "strings" + + "golang.org/x/net/html/charset" ) // errMalformedMessage marks a message this server can never turn into a @@ -17,81 +21,319 @@ import ( // sender is told to give up rather than retry forever. var errMalformedMessage = errors.New("malformed message") +// maxMultipartDepth bounds how deep a message can make this server walk. Real +// mail nests three levels at most (mixed → related → alternative); anything +// past that is a generated or hostile message rather than one with a body +// worth finding. +const maxMultipartDepth = 8 + type ReceivedEmail struct { Recipients []string Msg *mail.Message + + bodyRead bool body string + bodyIsHTML bool } +// Body returns the message body as the message carried it: its text/plain part +// when there is one, its text/html part otherwise. func (re *ReceivedEmail) Body() (string, error) { - if re.body == "" { - contentType := re.Msg.Header.Get("Content-Type") - - if contentType == "" { - body, err := io.ReadAll(re.Msg.Body) - if err != nil { - return "", fmt.Errorf("failed to read email body: %w", err) - } - re.body = string(body) - } else { - mediaType, params, err := mime.ParseMediaType(contentType) - if err != nil { - return "", fmt.Errorf("%w: parsing Content-Type %q: %w", errMalformedMessage, contentType, err) - } - - if strings.HasPrefix(mediaType, "multipart/") { - // Handle any multipart type (alternative, mixed, related, etc.) - mr := multipart.NewReader(re.Msg.Body, params["boundary"]) - var htmlBody string - - for { - part, err := mr.NextPart() - if err != nil { - break - } - - partContentType := part.Header.Get("Content-Type") - slog.Debug("Processing email part", slog.String("content_type", partContentType)) - - // Prefer text/plain, but keep HTML as fallback - if strings.HasPrefix(partContentType, "text/plain") { - body := new(bytes.Buffer) - _, err = body.ReadFrom(part) - _ = part.Close() - if err != nil { - slog.Error("Failed to read part body", slog.String("err", err.Error())) - return "", fmt.Errorf("failed to read part body: %w", err) - } - - re.body = body.String() - break // Found text/plain, use it - } else if strings.HasPrefix(partContentType, "text/html") && htmlBody == "" { - // Store HTML as fallback if no text/plain found - body := new(bytes.Buffer) - _, err = body.ReadFrom(part) - _ = part.Close() - if err == nil { - htmlBody = body.String() - } - } else { - _ = part.Close() - } - } - - // If no text/plain was found, use HTML - if re.body == "" && htmlBody != "" { - re.body = htmlBody - } - } else if strings.HasPrefix(mediaType, "text/") { - // Handle non-multipart text content (text/plain, text/html, etc.) - body, err := io.ReadAll(re.Msg.Body) - if err != nil { - return "", fmt.Errorf("failed to read email body: %w", err) - } - re.body = string(body) - } - } + if err := re.readBody(); err != nil { + return "", err } return re.body, nil } + +// FormattedBody returns the body in the requested format. Only an HTML body is +// ever rewritten: a message that already arrived as plain text is what the +// sender chose to write, and no target is better served by a round trip +// through a renderer. +func (re *ReceivedEmail) FormattedBody(format BodyFormat) (string, error) { + if err := re.readBody(); err != nil { + return "", err + } + + if format == FormatRaw || format == "" || !re.bodyIsHTML { + return re.body, nil + } + + rendered, err := renderHTML(re.body, format) + if err != nil { + // Reformatting is a courtesy to the target, not a condition of + // delivery. A body the renderer cannot take apart is forwarded as it + // arrived, which is what the target would have received anyway had + // the recipient not asked for a format. + slog.Warn("failed to render HTML body, forwarding it unchanged", + slog.String("format", string(format)), + slog.String("err", err.Error())) + + return re.body, nil + } + + if rendered == "" && strings.TrimSpace(re.body) != "" { + // Mail whose whole message is an image or a tracking beacon renders + // to nothing. The subject still goes out as the notification title, + // so this is worth a line in the log rather than a rejection. + slog.Warn("HTML body rendered to nothing", + slog.String("format", string(format)), + slog.Int("html_length", len(re.body))) + } + + return rendered, nil +} + +func (re *ReceivedEmail) readBody() error { + if re.bodyRead { + return nil + } + + body, isHTML, err := readMIMEBody(textproto.MIMEHeader(re.Msg.Header), re.Msg.Body, 0) + if err != nil { + return err + } + + re.body, re.bodyIsHTML, re.bodyRead = body, isHTML, true + + return nil +} + +// readMIMEBody picks the body out of one MIME entity, descending into +// multipart containers, and reports whether what it found is HTML. +func readMIMEBody(header textproto.MIMEHeader, body io.Reader, depth int) (string, bool, error) { + contentType := header.Get("Content-Type") + if contentType == "" { + content, err := decodeContent(header, body, "") + + return content, false, err + } + + mediaType, params, err := mime.ParseMediaType(contentType) + if err != nil { + return "", false, fmt.Errorf("%w: parsing Content-Type %q: %w", errMalformedMessage, contentType, err) + } + + switch { + case strings.HasPrefix(mediaType, "multipart/"): + var selected bodySelector + if err := walkMultipart(&selected, body, params["boundary"], depth); err != nil { + return "", false, err + } + content, isHTML := selected.result() + + return content, isHTML, nil + + case strings.HasPrefix(mediaType, "text/"): + content, err := decodeContent(header, body, params["charset"]) + + return content, mediaType == "text/html", err + + default: + // An image or an application/* payload on its own carries no text to + // notify with; the subject still reaches the target as the title. + return "", false, nil + } +} + +func walkMultipart(selected *bodySelector, body io.Reader, boundary string, depth int) error { + if boundary == "" { + return fmt.Errorf("%w: multipart message without a boundary", errMalformedMessage) + } + + if depth >= maxMultipartDepth { + slog.Warn("stopped descending into a deeply nested message", + slog.Int("depth", depth)) + + return nil + } + + reader := multipart.NewReader(body, boundary) + for { + part, err := reader.NextPart() + if errors.Is(err, io.EOF) { + return nil + } + + if err != nil { + // A truncated container still hands over the parts that were + // readable. Only a message that yielded nothing at all is worth + // refusing, since there is nothing left to notify with. + slog.Warn("stopped reading a multipart message early", + slog.String("err", err.Error())) + + if selected.empty() { + return fmt.Errorf("%w: reading multipart body: %w", errMalformedMessage, err) + } + + return nil + } + + err = readPart(selected, part, depth) + _ = part.Close() + + if err != nil { + return err + } + + if selected.done() { + return nil + } + } +} + +func readPart(selected *bodySelector, part *multipart.Part, depth int) error { + if disposition, _, err := mime.ParseMediaType(part.Header.Get("Content-Disposition")); err == nil && + disposition == "attachment" { + // An attached .txt or .html file is something the sender enclosed, + // not what they wrote. + return nil + } + + contentType := part.Header.Get("Content-Type") + slog.Debug("Processing email part", slog.String("content_type", contentType)) + + // RFC 2045 makes a part without a Content-Type plain US-ASCII text. + mediaType, params := "text/plain", map[string]string{} + + if contentType != "" { + var err error + + mediaType, params, err = mime.ParseMediaType(contentType) + if err != nil { + // One unreadable part among several is not the whole message; + // the rest of the tree may still hold a body. + slog.Warn("ignoring email part with an unparseable Content-Type", + slog.String("content_type", contentType), + slog.String("err", err.Error())) + + return nil + } + } + + switch { + case strings.HasPrefix(mediaType, "multipart/"): + return walkMultipart(selected, part, params["boundary"], depth+1) + + case mediaType == "text/plain" || mediaType == "text/html": + if selected.has(mediaType) { + return nil + } + + content, err := decodeContent(part.Header, part, params["charset"]) + if err != nil { + return err + } + + selected.offer(mediaType, content) + } + + return nil +} + +// decodeContent turns one entity's bytes into a UTF-8 string, undoing the +// transfer encoding and the character set the message declared. Without this a +// quoted-printable body reaches the target as "=E2=80=99" and a Latin-1 one as +// mojibake — and neither survives a trip through an HTML renderer. +func decodeContent(header textproto.MIMEHeader, body io.Reader, charsetLabel string) (string, error) { + raw, err := io.ReadAll(decodeTransferEncoding(body, header.Get("Content-Transfer-Encoding"))) + if err != nil { + return "", fmt.Errorf("%w: decoding message body: %w", errMalformedMessage, err) + } + + return decodeCharset(raw, charsetLabel), nil +} + +func decodeTransferEncoding(body io.Reader, encoding string) io.Reader { + switch strings.ToLower(strings.TrimSpace(encoding)) { + case "base64": + return base64.NewDecoder(base64.StdEncoding, body) + case "quoted-printable": + // mime/multipart already unwraps this for the parts it hands out and + // drops the header with it, so only a single-part message arrives + // here still encoded. + return quotedprintable.NewReader(body) + default: + // 7bit, 8bit and binary are the identity encoding; anything else is + // something this server has no way to undo, and passing it through + // beats replacing the body with an error. + return body + } +} + +func decodeCharset(raw []byte, label string) string { + switch strings.ToLower(strings.TrimSpace(label)) { + case "", "utf-8", "utf8", "us-ascii", "ascii": + return string(raw) + } + + encoding, _ := charset.Lookup(label) + if encoding == nil { + slog.Warn("unknown message charset, forwarding the body undecoded", + slog.String("charset", label)) + + return string(raw) + } + + decoded, err := encoding.NewDecoder().Bytes(raw) + if err != nil { + slog.Warn("failed to decode message charset, forwarding the body undecoded", + slog.String("charset", label), + slog.String("err", err.Error())) + + return string(raw) + } + + return string(decoded) +} + +// bodySelector keeps the best body found while walking a message. A text/plain +// part beats a text/html one, and the first of each kind beats the rest, which +// is the order a multipart/alternative puts its own preference in. +type bodySelector struct { + plain string + html string + hasPlain bool + hasHTML bool +} + +func (s *bodySelector) has(mediaType string) bool { + if mediaType == "text/html" { + return s.hasHTML + } + + return s.done() +} + +func (s *bodySelector) offer(mediaType, content string) { + if mediaType == "text/html" { + if !s.hasHTML { + s.html, s.hasHTML = content, true + } + + return + } + + if !s.done() { + s.plain, s.hasPlain = content, true + } +} + +func (s *bodySelector) empty() bool { + return !s.hasPlain && !s.hasHTML +} + +// done reports that nothing later in the message can improve on what has been +// found. An empty text/plain alternative does not count: senders emit those +// beside the HTML they actually wrote. +func (s *bodySelector) done() bool { + return s.hasPlain && strings.TrimSpace(s.plain) != "" +} + +func (s *bodySelector) result() (string, bool) { + if s.done() || !s.hasHTML { + return s.plain, false + } + + return s.html, true +} diff --git a/email_test.go b/email_test.go index 9a4c9db..2b61d95 100644 --- a/email_test.go +++ b/email_test.go @@ -1,9 +1,11 @@ package smtp2shoutrrr import ( + "encoding/base64" "net/mail" "strings" "testing" + "unicode/utf8" "github.com/stretchr/testify/require" ) @@ -88,3 +90,218 @@ func TestBodyFallsBackToHTMLPart(t *testing.T) { require.NoError(t, err) require.Equal(t, "

html body

", body) } + +func TestBodyDecodesQuotedPrintable(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + "Content-Type: text/plain; charset=utf-8", + "Content-Transfer-Encoding: quoted-printable", + "", + "caf=C3=A9 =E2=80=94 a very long line that the sender wrapped =", + "here", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "café — a very long line that the sender wrapped here\r\n", body) +} + +func TestBodyDecodesBase64(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + "Content-Type: text/plain; charset=utf-8", + "Content-Transfer-Encoding: base64", + "", + base64.StdEncoding.EncodeToString([]byte("café — encoded")), + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "café — encoded", body) +} + +// Windows-1252 and Latin-1 are still what a good deal of automated mail is +// written in, and their bytes are not valid UTF-8. +func TestBodyDecodesNonUTF8Charset(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/plain; charset=iso-8859-1\r\n\r\ncaf\xe9\r\n")} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "café\r\n", body) + require.True(t, utf8.ValidString(body)) +} + +func TestBodyUnknownCharsetIsForwardedUndecoded(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/plain; charset=not-a-charset\r\n\r\nbody\r\n")} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "body\r\n", body) +} + +// A forwarded message is a multipart/mixed wrapping the multipart/alternative +// that holds what the sender actually wrote. Stopping at the outer container +// left the notification empty. +func TestBodyDescendsIntoNestedMultipart(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/mixed; boundary="outer"`, + "", + "--outer", + `Content-Type: multipart/alternative; boundary="inner"`, + "", + "--inner", + "Content-Type: text/html; charset=utf-8", + "", + "

html body

", + "--inner", + "Content-Type: text/plain; charset=utf-8", + "", + "plain body", + "--inner--", + "--outer--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "plain body", body) +} + +func TestBodySkipsAttachments(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/mixed; boundary="b"`, + "", + "--b", + "Content-Type: text/plain; charset=utf-8", + `Content-Disposition: attachment; filename="notes.txt"`, + "", + "an enclosed file, not the message", + "--b", + "Content-Type: text/plain; charset=utf-8", + "", + "the message itself", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "the message itself", body) +} + +// Senders that build the HTML alternative from a template routinely emit an +// empty text/plain beside it. +func TestBodyPrefersHTMLOverAnEmptyPlainTextPart(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/alternative; boundary="b"`, + "", + "--b", + "Content-Type: text/plain; charset=utf-8", + "", + " ", + "--b", + "Content-Type: text/html; charset=utf-8", + "", + "

html body

", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "

html body

", body) +} + +func TestBodyRejectsMultipartWithoutBoundary(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: multipart/alternative\r\n\r\nbody\r\n")} + + _, err := email.Body() + require.ErrorIs(t, err, errMalformedMessage) +} + +func TestFormattedBodyConvertsHTML(t *testing.T) { + raw := strings.Join([]string{ + "Subject: Test", + "Content-Type: text/html; charset=utf-8", + "Content-Transfer-Encoding: quoted-printable", + "", + "

Build failed: run 42

", + "", + }, "\r\n") + + for format, want := range map[BodyFormat]string{ + FormatMarkdown: "Build **failed**: [run 42](https://ci.example.com/42)", + FormatText: "Build failed: run 42 (https://ci.example.com/42)", + FormatRaw: "

Build failed: run 42

\r\n", + } { + t.Run(string(format), func(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, raw)} + + body, err := email.FormattedBody(format) + require.NoError(t, err) + require.Equal(t, want, body) + }) + } +} + +// A recipient asking for Markdown is asking for HTML to stop reaching it, not +// for the plain text a sender wrote to be run through a renderer. +func TestFormattedBodyLeavesPlainTextAlone(t *testing.T) { + raw := "Subject: Test\r\nContent-Type: text/plain; charset=utf-8\r\n\r\n2 * 3 = 6 \r\n" + + for _, format := range []BodyFormat{FormatRaw, FormatText, FormatMarkdown} { + t.Run(string(format), func(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, raw)} + + body, err := email.FormattedBody(format) + require.NoError(t, err) + require.Equal(t, "2 * 3 = 6 \r\n", body) + }) + } +} + +// The zero value of the option means the same as raw, so a recipient built in +// code rather than loaded from a file still forwards what it received. +func TestFormattedBodyTreatsUnsetFormatAsRaw(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/html\r\n\r\n

html body

\r\n")} + + body, err := email.FormattedBody("") + require.NoError(t, err) + require.Equal(t, "

html body

\r\n", body) +} + +// Reformatting is a courtesy, so a body the renderer chokes on is still +// forwarded rather than costing the reader the notification. +func TestFormattedBodyFallsBackToTheRawHTML(t *testing.T) { + // The parser refuses a document with more than 512 elements open at once. + source := strings.Repeat("
", 600) + "deep" + strings.Repeat("
", 600) + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/html\r\n\r\n"+source)} + + body, err := email.FormattedBody(FormatMarkdown) + require.NoError(t, err) + require.Equal(t, source, body) +} + +func TestBodyIsReadOnlyOnce(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/html\r\n\r\n

body

\r\n")} + + first, err := email.FormattedBody(FormatMarkdown) + require.NoError(t, err) + + second, err := email.FormattedBody(FormatMarkdown) + require.NoError(t, err) + + require.Equal(t, "body", first) + require.Equal(t, first, second, "the message body is consumed as it is read") +} diff --git a/format.go b/format.go new file mode 100644 index 0000000..fc9aff1 --- /dev/null +++ b/format.go @@ -0,0 +1,46 @@ +package smtp2shoutrrr + +import ( + "slices" + "strings" +) + +// BodyFormat names the representation a recipient's targets want the message +// body in. Only an HTML body is ever rewritten: a message that already arrived +// as plain text is forwarded untouched whatever the recipient asked for. +type BodyFormat string + +const ( + // FormatRaw forwards the body exactly as the message carried it. + FormatRaw BodyFormat = "raw" + // FormatText renders an HTML body as plain text. + FormatText BodyFormat = "text" + // FormatMarkdown renders an HTML body as Markdown. + FormatMarkdown BodyFormat = "markdown" +) + +var bodyFormats = []BodyFormat{FormatRaw, FormatText, FormatMarkdown} + +// normalize maps an unset Format to the one that changes nothing, and accepts +// the casing a hand-written configuration file is likely to use. +func (f BodyFormat) normalize() BodyFormat { + normalized := BodyFormat(strings.ToLower(strings.TrimSpace(string(f)))) + if normalized == "" { + return FormatRaw + } + + return normalized +} + +func (f BodyFormat) valid() bool { + return slices.Contains(bodyFormats, f) +} + +func formatNames() string { + names := make([]string, 0, len(bodyFormats)) + for _, format := range bodyFormats { + names = append(names, string(format)) + } + + return strings.Join(names, ", ") +} diff --git a/go.mod b/go.mod index 0892ec1..fed4744 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/pelletier/go-toml/v2 v2.4.3 github.com/stretchr/testify v1.12.1 golang.org/x/crypto/x509roots/fallback v0.0.0-20260902180247-86efde54dc70 + golang.org/x/net v0.58.0 ) require ( @@ -21,7 +22,7 @@ require ( github.com/mattn/go-colorable v0.1.15 // indirect github.com/mattn/go-isatty v0.0.24 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect - golang.org/x/net v0.58.0 // indirect golang.org/x/sys v0.47.0 // indirect + golang.org/x/text v0.41.0 // indirect golang.org/x/tools v0.49.0 // indirect ) diff --git a/html.go b/html.go new file mode 100644 index 0000000..0fbb27c --- /dev/null +++ b/html.go @@ -0,0 +1,760 @@ +package smtp2shoutrrr + +import ( + "fmt" + "strconv" + "strings" + "unicode" + + "golang.org/x/net/html" + "golang.org/x/net/html/atom" +) + +// maxHTMLDepth bounds how deep a message can make the renderer recurse. Mail +// templates nest a few dozen elements at most; anything past this is a +// generated document, and walking it would only trade stack for output nobody +// reads. +const maxHTMLDepth = 256 + +// renderHTML rewrites an HTML body as plain text or Markdown. The output is +// aimed at chat and push notification services, so it favours a short readable +// message over a faithful reproduction of the document: layout tables become +// lines, images without alt text disappear, and a single newline is used where +// a strict CommonMark writer would need a hard break, because every service +// this forwards to renders one as a line break. +func renderHTML(source string, format BodyFormat) (string, error) { + document, err := html.Parse(strings.NewReader(source)) + if err != nil { + // The parser recovers from any markup it can hold, so this is a + // document too deeply nested for it rather than an invalid one. + return "", fmt.Errorf("parsing HTML body: %w", err) + } + + renderer := &htmlRenderer{markdown: format == FormatMarkdown, atLineStart: true} + renderer.walk(document, 0) + + return renderer.result(), nil +} + +type htmlRenderer struct { + markdown bool + + out strings.Builder + + // prefix opens every line of the current context: the quote markers of + // the blockquotes it sits in and the indentation of the list items. + prefix string + // marker replaces prefix on the next line only, which is where a list + // item's bullet or number goes. + marker string + + // pending holds markup that has been opened but not yet paid for. Email + // HTML is full of empty and wrappers, and writing their markers + // eagerly would scatter stray asterisks and brackets through the output, + // so a marker is only written once something follows it. + pending []string + + // pendingNewlines is the separation the next content owes what precedes + // it: one for a new line, two for a new block. Requests coalesce, so a + // paragraph closing inside a div does not open a hole. + pendingNewlines int + // breakPrefix is the prefix in force when the pending separation was + // asked for, which is not always the one in force when it is written: a + // blockquote opens and closes between the two. + breakPrefix string + // pendingSpace records collapsed whitespace that only becomes a space if + // another word follows it on the same line. + pendingSpace bool + + atLineStart bool + + // preformatted counts the
 elements in scope, where whitespace is
+	// content rather than layout.
+	preformatted int
+
+	listStack []listLevel
+}
+
+type listLevel struct {
+	ordered bool
+	index   int
+}
+
+func (r *htmlRenderer) result() string {
+	lines := strings.Split(r.out.String(), "\n")
+	for i := range lines {
+		lines[i] = strings.TrimRight(lines[i], " \t")
+	}
+
+	return strings.TrimSpace(strings.Join(lines, "\n"))
+}
+
+// write appends to the output. strings.Builder never fails, and every caller
+// has already decided the string belongs there.
+func (r *htmlRenderer) write(s string) {
+	_, _ = r.out.WriteString(s)
+}
+
+// content settles everything the output owes — line breaks, indentation, a
+// collapsed space, markup opened earlier — and then writes s.
+func (r *htmlRenderer) content(s string) {
+	if s == "" {
+		return
+	}
+
+	r.flushNewlines()
+	r.startLine()
+	r.flushSpace()
+	r.flushPending()
+	r.write(s)
+}
+
+// text writes message text, which is escaped so its punctuation cannot be
+// mistaken for the formatting this renderer emits itself.
+func (r *htmlRenderer) text(s string) {
+	if s == "" {
+		return
+	}
+
+	if r.markdown && r.preformatted == 0 {
+		s = markdownEscaper.Replace(s)
+		if r.startsLine() {
+			s = escapeLineStart(s)
+		}
+	}
+
+	r.content(s)
+}
+
+// startsLine reports whether the next content will open a line, where the
+// markers that only mean something there have to be defused.
+func (r *htmlRenderer) startsLine() bool {
+	return len(r.pending) == 0 && (r.atLineStart || r.pendingNewlines > 0)
+}
+
+func (r *htmlRenderer) flushNewlines() {
+	if r.pendingNewlines == 0 {
+		return
+	}
+
+	blank := r.pendingNewlines > 1
+	r.pendingNewlines = 0
+
+	r.write("\n")
+	if blank {
+		// A truly blank line closes a blockquote or a list item. The
+		// separator carries the prefix the blocks on either side of it share,
+		// so it keeps the contexts they are both inside open and lets the
+		// ones only one of them is inside end.
+		r.write(strings.TrimRight(commonPrefix(r.breakPrefix, r.prefix), " "))
+		r.write("\n")
+	}
+
+	r.atLineStart = true
+	r.pendingSpace = false
+}
+
+func (r *htmlRenderer) startLine() {
+	if !r.atLineStart {
+		return
+	}
+
+	r.atLineStart = false
+	r.pendingSpace = false
+
+	if r.marker != "" {
+		r.write(r.marker)
+		r.marker = ""
+
+		return
+	}
+
+	r.write(r.prefix)
+}
+
+func (r *htmlRenderer) flushSpace() {
+	if !r.pendingSpace {
+		return
+	}
+
+	r.pendingSpace = false
+	if r.out.Len() > 0 {
+		r.write(" ")
+	}
+}
+
+func (r *htmlRenderer) flushPending() {
+	for _, markup := range r.pending {
+		r.write(markup)
+	}
+
+	r.pending = r.pending[:0]
+}
+
+// open queues markup and returns a closer that reports whether any content
+// followed it. A closer that reports false has already withdrawn the markup.
+func (r *htmlRenderer) open(markup string) func() bool {
+	r.pending = append(r.pending, markup)
+	depth := len(r.pending)
+
+	return func() bool {
+		if len(r.pending) >= depth && r.pending[depth-1] == markup {
+			r.pending = r.pending[:depth-1]
+
+			return false
+		}
+
+		return true
+	}
+}
+
+func (r *htmlRenderer) breakLine()  { r.separate(1) }
+func (r *htmlRenderer) breakBlock() { r.separate(2) }
+
+func (r *htmlRenderer) separate(newlines int) {
+	if r.out.Len() == 0 {
+		// Nothing has been written, so there is nothing to separate from and
+		// the document would only gain a blank first line.
+		return
+	}
+
+	if r.pendingNewlines == 0 {
+		r.breakPrefix = r.prefix
+	}
+
+	if newlines > r.pendingNewlines {
+		r.pendingNewlines = newlines
+	}
+
+	r.pendingSpace = false
+}
+
+func (r *htmlRenderer) walk(node *html.Node, depth int) {
+	if depth > maxHTMLDepth {
+		return
+	}
+
+	switch node.Type {
+	case html.TextNode:
+		r.writeTextNode(node.Data)
+	case html.DocumentNode:
+		r.walkChildren(node, depth)
+	case html.ElementNode:
+		r.walkElement(node, depth)
+	default:
+		// Comments, doctypes and raw nodes carry nothing a reader wants.
+	}
+}
+
+func (r *htmlRenderer) walkChildren(node *html.Node, depth int) {
+	for child := node.FirstChild; child != nil; child = child.NextSibling {
+		r.walk(child, depth+1)
+	}
+}
+
+func (r *htmlRenderer) writeTextNode(data string) {
+	data = sanitizeText(data)
+	if data == "" {
+		return
+	}
+
+	if r.preformatted > 0 {
+		for i, line := range strings.Split(data, "\n") {
+			if i > 0 {
+				r.breakLine()
+			}
+			r.text(line)
+		}
+
+		return
+	}
+
+	if strings.TrimSpace(data) == "" {
+		// Whitespace between elements still separates the words on either
+		// side of it.
+		r.pendingSpace = true
+
+		return
+	}
+
+	if strings.TrimLeftFunc(data, unicode.IsSpace) != data {
+		r.pendingSpace = true
+	}
+
+	for i, word := range strings.Fields(data) {
+		if i > 0 {
+			r.pendingSpace = true
+		}
+		r.text(word)
+	}
+
+	if strings.TrimRightFunc(data, unicode.IsSpace) != data {
+		r.pendingSpace = true
+	}
+}
+
+func (r *htmlRenderer) walkElement(node *html.Node, depth int) {
+	if isHidden(node) {
+		return
+	}
+
+	switch node.DataAtom {
+	case atom.Script, atom.Style, atom.Head, atom.Title, atom.Noscript,
+		atom.Template, atom.Iframe, atom.Object, atom.Svg, atom.Map:
+		return
+
+	case atom.Br:
+		if r.pendingNewlines > 0 {
+			// A run of 
is how mail asks for a blank line, and a browser + // gives it one; coalescing them all into a single break would + // glue the greeting to the paragraph under it. + r.breakBlock() + + break + } + + r.breakLine() + + case atom.Hr: + r.breakBlock() + r.content("---") + r.breakBlock() + + case atom.H1, atom.H2, atom.H3, atom.H4, atom.H5, atom.H6: + r.renderHeading(node, depth) + + case atom.Ul, atom.Ol: + r.renderList(node, depth) + + case atom.Li: + r.renderListItem(node, depth) + + case atom.Blockquote: + r.renderBlockquote(node, depth) + + case atom.Pre: + r.renderPreformatted(node, depth) + + case atom.A: + r.renderLink(node, depth) + + case atom.Img: + r.renderImage(node) + + case atom.B, atom.Strong: + r.renderInline("**", node, depth) + + case atom.I, atom.Em: + r.renderInline("*", node, depth) + + case atom.Del, atom.S, atom.Strike: + r.renderInline("~~", node, depth) + + case atom.Code, atom.Kbd, atom.Samp: + if r.preformatted > 0 { + r.walkChildren(node, depth) + + return + } + r.renderInline("`", node, depth) + + case atom.Td, atom.Th: + // Mail is laid out in tables far more often than it tabulates + // anything, so a cell reads as another run of words rather than as a + // column that a Markdown table would have to line up. + r.walkChildren(node, depth) + r.pendingSpace = true + + default: + switch { + case blockElements[node.DataAtom]: + r.breakBlock() + r.walkChildren(node, depth) + r.breakBlock() + case lineElements[node.DataAtom]: + r.breakLine() + r.walkChildren(node, depth) + r.breakLine() + default: + r.walkChildren(node, depth) + } + } +} + +func (r *htmlRenderer) renderHeading(node *html.Node, depth int) { + r.breakBlock() + + if r.markdown { + closeHeading := r.open(strings.Repeat("#", headingLevels[node.DataAtom]) + " ") + r.walkChildren(node, depth) + closeHeading() + } else { + r.walkChildren(node, depth) + } + + r.breakBlock() +} + +func (r *htmlRenderer) renderList(node *html.Node, depth int) { + // A list nested in another one belongs to the item that holds it, so it + // only starts a new line; a blank one would make the whole outer list + // loose and double-space every item in it. + nested := len(r.listStack) > 0 + r.separateList(nested) + + r.listStack = append(r.listStack, listLevel{ + ordered: node.DataAtom == atom.Ol, + index: listStart(node), + }) + r.walkChildren(node, depth) + r.listStack = r.listStack[:len(r.listStack)-1] + + r.separateList(nested) +} + +func (r *htmlRenderer) separateList(nested bool) { + if nested { + r.breakLine() + + return + } + + r.breakBlock() +} + +func (r *htmlRenderer) renderListItem(node *html.Node, depth int) { + r.breakLine() + + bullet := "- " + if len(r.listStack) > 0 { + level := &r.listStack[len(r.listStack)-1] + if level.ordered { + bullet = strconv.Itoa(level.index) + ". " + level.index++ + } + } + + outer := r.prefix + r.marker = outer + bullet + r.prefix = outer + strings.Repeat(" ", len(bullet)) + + r.walkChildren(node, depth) + + r.breakLine() + r.prefix = outer + r.marker = "" +} + +func (r *htmlRenderer) renderBlockquote(node *html.Node, depth int) { + r.breakBlock() + + outer := r.prefix + r.prefix = outer + "> " + r.walkChildren(node, depth) + r.prefix = outer + + r.breakBlock() +} + +func (r *htmlRenderer) renderPreformatted(node *html.Node, depth int) { + r.breakBlock() + + if r.markdown { + r.content("```") + r.breakLine() + } + + r.preformatted++ + r.walkChildren(node, depth) + r.preformatted-- + + if r.markdown { + r.breakLine() + r.content("```") + } + + r.breakBlock() +} + +func (r *htmlRenderer) renderInline(marker string, node *html.Node, depth int) { + if !r.markdown { + r.walkChildren(node, depth) + + return + } + + closeInline := r.open(marker) + r.walkChildren(node, depth) + if closeInline() { + // Written straight out so a trailing space stays outside the marker, + // where Markdown still recognises it as the end of the span. + r.write(marker) + } +} + +func (r *htmlRenderer) renderLink(node *html.Node, depth int) { + href := strings.TrimSpace(attrValue(node, "href")) + if !isUsableURL(href) { + r.walkChildren(node, depth) + + return + } + + if r.markdown { + closeLink := r.open("[") + r.walkChildren(node, depth) + if closeLink() { + r.write("](" + markdownURL(href) + ")") + } else { + // A link with nothing to click on is still worth forwarding. + r.text(href) + } + + return + } + + start := r.out.Len() + r.walkChildren(node, depth) + + label := strings.TrimSpace(r.out.String()[start:]) + switch { + case label == "": + r.text(href) + case label != href: + // A bare label leaves a notification's reader with nothing to open. + r.write(" (" + href + ")") + } +} + +func (r *htmlRenderer) renderImage(node *html.Node) { + alt := strings.TrimSpace(sanitizeText(attrValue(node, "alt"))) + if alt == "" { + // Tracking pixels, spacers and sliced-up banners have no alt text and + // nothing to say. + return + } + + source := strings.TrimSpace(attrValue(node, "src")) + if r.markdown && isUsableURL(source) { + r.content("![" + markdownEscaper.Replace(alt) + "](" + markdownURL(source) + ")") + + return + } + + r.text(alt) +} + +// blockElements are separated from their surroundings by a blank line. +// lineElements only start a new line, which is how a browser renders the +//
-per-line that mail templates are built from. +var ( + blockElements = map[atom.Atom]bool{ + atom.Article: true, + atom.Dl: true, + atom.Fieldset: true, + atom.Form: true, + atom.P: true, + atom.Section: true, + atom.Table: true, + } + + lineElements = map[atom.Atom]bool{ + atom.Address: true, + atom.Aside: true, + atom.Caption: true, + atom.Center: true, + atom.Dd: true, + atom.Div: true, + atom.Dt: true, + atom.Figcaption: true, + atom.Figure: true, + atom.Footer: true, + atom.Header: true, + atom.Legend: true, + atom.Main: true, + atom.Nav: true, + atom.Tr: true, + } +) + +// The characters mail uses for layout rather than for words: padding that +// hides a preheader from the inbox preview, and the spaces that hold table +// cells apart without letting them wrap. +const ( + softHyphen = '\u00ad' + noBreakSpace = '\u00a0' + zeroWidthSpace = '\u200b' + zeroWidthNonJoiner = '\u200c' + zeroWidthJoiner = '\u200d' + narrowNoBreakSpace = '\u202f' + wordJoiner = '\u2060' + figureSpace = '\u2007' + byteOrderMark = '\ufeff' +) + +var headingLevels = map[atom.Atom]int{ + atom.H1: 1, + atom.H2: 2, + atom.H3: 3, + atom.H4: 4, + atom.H5: 5, + atom.H6: 6, +} + +// markdownEscaper defuses the punctuation that would otherwise turn message +// text into formatting. Underscores are left alone: CommonMark ignores them +// inside a word, which is where mail almost always puts them, and escaping +// every one turns file names and identifiers into noise. +var markdownEscaper = strings.NewReplacer( + `\`, `\\`, + "`", "\\`", + `*`, `\*`, + `[`, `\[`, + `]`, `\]`, + `<`, `\<`, +) + +// escapeLineStart defuses the markers that only mean something at the start of +// a line, so a message opening with "- " or "# " reads as the sender wrote it. +func escapeLineStart(s string) string { + if s == "" { + return s + } + + switch s[0] { + case '#', '>', '-', '+', '=', '|': + return `\` + s + } + + digits := 0 + for digits < len(s) && s[digits] >= '0' && s[digits] <= '9' { + digits++ + } + + if digits > 0 && digits < len(s) && (s[digits] == '.' || s[digits] == ')') { + return s[:digits] + `\` + s[digits:] + } + + return s +} + +// markdownURL fits a URL into a link destination. Spaces and parentheses would +// end the destination early, so those URLs take the angle bracket form. +func markdownURL(raw string) string { + if !strings.ContainsAny(raw, " ()<>") { + return raw + } + + return "<" + strings.NewReplacer(" ", "%20", "<", "%3C", ">", "%3E").Replace(raw) + ">" +} + +// isUsableURL rejects the destinations a notification's reader cannot act on: +// in-page anchors, inline data, script handlers and the message's own inline +// attachments. +func isUsableURL(raw string) bool { + if raw == "" || strings.HasPrefix(raw, "#") { + return false + } + + scheme, _, found := strings.Cut(raw, ":") + if !found || !isScheme(scheme) { + return true + } + + switch strings.ToLower(scheme) { + case "javascript", "data", "cid", "about", "blob": + return false + } + + return true +} + +func isScheme(s string) bool { + if s == "" || !unicode.IsLetter(rune(s[0])) { + return false + } + + return strings.IndexFunc(s, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && + r != '+' && r != '-' && r != '.' + }) < 0 +} + +// sanitizeText turns the characters mail uses for layout into ones a +// notification can show: the non-breaking spaces that hold table cells apart +// become ordinary spaces, and the zero-width padding that hides a preheader +// from the inbox preview is dropped rather than forwarded invisibly. +func sanitizeText(s string) string { + return strings.Map(func(r rune) rune { + switch r { + case zeroWidthSpace, zeroWidthNonJoiner, zeroWidthJoiner, + wordJoiner, byteOrderMark, softHyphen: + return -1 + case noBreakSpace, narrowNoBreakSpace, figureSpace: + return ' ' + } + + if unicode.IsSpace(r) { + return r + } + + // Control characters have no rendering of their own and only muddle + // the payloads the targets are sent as. + if unicode.IsControl(r) { + return -1 + } + + return r + }, s) +} + +// isHidden reports whether a browser would leave the element out of the page. +// Mail templates open with a hidden block holding the preview line the inbox +// shows, which is written for the list view and reads as noise anywhere else. +func isHidden(node *html.Node) bool { + for _, attr := range node.Attr { + switch attr.Key { + case "hidden": + return true + case "style": + if strings.Contains(spaceStripper.Replace(strings.ToLower(attr.Val)), "display:none") { + return true + } + } + } + + return false +} + +var spaceStripper = strings.NewReplacer(" ", "", "\t", "", "\n", "", "\r", "") + +// commonPrefix returns the leading run the two prefixes agree on. +func commonPrefix(a, b string) string { + limit := min(len(a), len(b)) + + shared := 0 + for shared < limit && a[shared] == b[shared] { + shared++ + } + + return a[:shared] +} + +func attrValue(node *html.Node, key string) string { + for _, attr := range node.Attr { + if attr.Key == key { + return attr.Val + } + } + + return "" +} + +func listStart(node *html.Node) int { + start, err := strconv.Atoi(strings.TrimSpace(attrValue(node, "start"))) + if err != nil { + return 1 + } + + return start +} diff --git a/html_test.go b/html_test.go new file mode 100644 index 0000000..05ad025 --- /dev/null +++ b/html_test.go @@ -0,0 +1,228 @@ +package smtp2shoutrrr + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func render(t *testing.T, format BodyFormat, source string) string { + t.Helper() + + out, err := renderHTML(source, format) + require.NoError(t, err) + + return out +} + +func TestRenderHTMLAsMarkdown(t *testing.T) { + for name, tc := range map[string]struct{ source, want string }{ + "paragraphs are separated by a blank line": { + source: "

first

second

", + want: "first\n\nsecond", + }, + "divs are separated by a single line, as a browser lays them out": { + source: "
first
second
", + want: "first\nsecond", + }, + "headings keep their level": { + source: "

one

three

", + want: "# one\n\n### three", + }, + "emphasis": { + source: "

bold italic gone x=1

", + want: "**bold** *italic* ~~gone~~ `x=1`", + }, + "a trailing space stays outside the emphasis it would otherwise break": { + source: "

bold after

", + want: "**bold** after", + }, + "empty inline wrappers leave no markers behind": { + source: "

text

", + want: "text", + }, + "links keep their destination": { + source: `

see the docs

`, + want: "see [the docs](https://example.com/x)", + }, + "a link with nothing to click on falls back to its destination": { + source: `

`, + want: "https://example.com/x", + }, + "a destination with spaces or parentheses is bracketed": { + source: `x`, + want: "[x]()", + }, + "unusable destinations are dropped but their text is kept": { + source: `up click img`, + want: "up click img", + }, + "unordered lists": { + source: "
  • one
  • two
", + want: "- one\n- two", + }, + "ordered lists count from their start attribute": { + source: `
  1. three
  2. four
`, + want: "3. three\n4. four", + }, + "nested lists stay tight and indented": { + source: "
  • outer
    • inner
", + want: "- outer\n - inner", + }, + "a wrapped list item is indented under its bullet": { + source: "
  • first line
    second line
", + want: "- first line\n second line", + }, + "blockquotes mark every line, including the blank ones": { + source: "

one

two

", + want: "> one\n>\n> two", + }, + "a blockquote does not leak its marker into what surrounds it": { + source: "

before

quoted

after

", + want: "before\n\n> quoted\n\nafter", + }, + "preformatted text is fenced and left alone": { + source: "
if (a < b) {\n  *x = 1;\n}
", + want: "```\nif (a < b) {\n *x = 1;\n}\n```", + }, + "br starts a new line": { + source: "

one
two

", + want: "one\ntwo", + }, + "a run of br leaves a blank line, as a browser does": { + source: "

Hi,

the body

", + want: "Hi,\n\nthe body", + }, + "hr becomes a rule": { + source: "

a


b

", + want: "a\n\n---\n\nb", + }, + "table rows become lines and cells become words": { + source: "
Total4
Failed0
", + want: "Total 4\nFailed 0", + }, + "images without alt text are dropped": { + source: `

ab

`, + want: "ab", + }, + "images with alt text are kept": { + source: `Logo`, + want: "![Logo](https://x.example/logo.png)", + }, + "an inline attachment keeps its alt text without a destination": { + source: `Chart`, + want: "Chart", + }, + "script and style content never reaches the reader": { + source: "

body

", + want: "body", + }, + "hidden preheaders are left out": { + source: `
inbox preview

real body

`, + want: "real body", + }, + "whitespace between elements collapses to a single space": { + source: "

one\n\t two three

", + want: "one two three", + }, + "entities and non-breaking spaces become ordinary text": { + source: "

4m 12s & counting — done

", + want: "4m 12s & counting — done", + }, + "zero width padding is dropped": { + source: "

​a​b​

", + want: "ab", + }, + "text that looks like markup is escaped": { + source: "

2 * 3, a [b] c, `tick`, back\\slash, <tag>

", + want: "2 \\* 3, a \\[b\\] c, \\`tick\\`, back\\\\slash, \\", + }, + "markers are only defused where they would take effect": { + source: "

- not a bullet

# not a heading

1. not a list

a - b

", + want: "\\- not a bullet\n\n\\# not a heading\n\n1\\. not a list\n\na - b", + }, + "underscores are left alone, since a word is where mail puts them": { + source: "

see config_test.go

", + want: "see config_test.go", + }, + "a document with nothing to say renders to nothing": { + source: ``, + want: "", + }, + } { + t.Run(name, func(t *testing.T) { + require.Equal(t, tc.want, render(t, FormatMarkdown, tc.source)) + }) + } +} + +func TestRenderHTMLAsText(t *testing.T) { + for name, tc := range map[string]struct{ source, want string }{ + "no emphasis markers are introduced": { + source: "

bold and italic and code

", + want: "bold and italic and code", + }, + "a link keeps its destination beside its label": { + source: `

see the docs

`, + want: "see the docs (https://example.com/x)", + }, + "a link whose label is already the destination is not repeated": { + source: `https://example.com/x`, + want: "https://example.com/x", + }, + "headings are plain lines": { + source: "

Title

body

", + want: "Title\n\nbody", + }, + "images keep their alt text without a destination": { + source: `Logo`, + want: "Logo", + }, + "preformatted text keeps its shape without a fence": { + source: "
a\n  b
", + want: "a\n b", + }, + "text that looks like markup is left as written": { + source: "

2 * 3 = 6 [see notes]

", + want: "2 * 3 = 6 [see notes]", + }, + "lists stay readable": { + source: "
  • one
  • two
", + want: "- one\n- two", + }, + } { + t.Run(name, func(t *testing.T) { + require.Equal(t, tc.want, render(t, FormatText, tc.source)) + }) + } +} + +// The parser recovers from anything, so the renderer has to as well: a +// notification is worth more than a report that the markup was invalid. +func TestRenderHTMLSurvivesBrokenMarkup(t *testing.T) { + for name, source := range map[string]string{ + "unclosed tags": "

boldboth

next", + "stray closing tags": "

text", + "no markup at all": "just some words", + "an empty document": "", + "an unterminated tag": "

text<", + } { + t.Run(name, func(t *testing.T) { + for _, format := range []BodyFormat{FormatText, FormatMarkdown} { + _, err := renderHTML(source, format) + require.NoError(t, err) + } + }) + } +} + +// A generated document should cost bounded work rather than the whole stack. +func TestRenderHTMLStopsAtNestingLimit(t *testing.T) { + depth := maxHTMLDepth + 50 + source := strings.Repeat("

", depth) + "too deep" + strings.Repeat("
", depth) + + out, err := renderHTML(source, FormatMarkdown) + require.NoError(t, err) + require.NotContains(t, out, "too deep") +} -- 2.52.0 From c5942c3d8ffe8e0deaaf2e4675042b7a371840a8 Mon Sep 17 00:00:00 2001 From: butterrobot Date: Thu, 10 Sep 2026 07:28:45 +0000 Subject: [PATCH 2/3] fix: address review of the HTML conversion feature (FMG-9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blockers - Cap the rendered output at 64 KiB. Block prefixes are re-emitted on every line, so nesting multiplied against line count: one message at the server's own 1 MB limit rendered to 125 MB and 894 MiB of allocation, which OOM-kills the process in any modest container. Now 65 KB and 30 MiB, with a "…" so a cut message says so. - Stop turning an undecodable transfer encoding into a permanent 550. Real mailers emit unpadded base64, and a 550 tells the sender to stop retrying, so mail that main delivered was lost for good. base64 and quoted-printable now degrade to whatever decoded — and to the bytes as they arrived — exactly as the charset path already did. One unreadable part no longer fails a message that has a perfectly good alternative in hand. - Keep the media type mime.ParseMediaType returns alongside its error. A part with "charset=" or an unquoted attachment file name was dropped whole, which delivered an empty notification with a 250 for mail main forwarded, and made the new attachment guard fail open on the malformed forms older MUAs emit. Renderer - Merge two emphasis spans that meet with nothing between them, and drop a marker nested inside itself. Splitting a bolded label across two runs is what every Word and Outlook export does, and it put a literal "**" in front of the reader — the exact thing this feature exists to prevent. - Render code spans and fenced blocks from their text, so escapes stay literal where Markdown makes them literal, and fence past the longest backtick run inside. A sender could otherwise close the fence and have the rest of the message render as live Markdown — a heading or a link inside what the reader trusts as a forwarded notification. - Strip line breaks from href and src and collapse them in alt text. A newline in a URL is invisible in the document and a fabricated line in the message, and it destroyed the destination as well. - Keep the blank lines inside
, which a diff and a stack trace are shaped
  by; recognise visibility:hidden, mso-hide:all and font-size:0 preheaders;
  de-duplicate a bare URL label on a line that opens with a bullet.
- Raise the depth guard above the parser's own limit on open elements, so
  content is never dropped silently between the two.

Elsewhere

- Forward the raw HTML when a body renders to nothing, as the render-error path
  already did. An image-only newsletter handed shoutrrr "", which Mattermost
  and Discord reject, so every target failed and the sender retried forever.
- Scope body selection to its container per RFC 2046: the parts of a
  multipart/alternative are one content in several forms, the parts of any
  other multipart are cumulative. "Plain beats HTML" applied across a mixed
  container picked a list's unsubscribe footer over the newsletter itself.
- Pass the message by pointer. Its body is a single-use stream cached on the
  value, so a copy would forward an empty notification to every recipient after
  the first.
- Normalize in BodyFormat.valid(), so a Config assembled in Go rather than
  loaded from a file does not fail on a Format nobody set.

Tests and docs

- A realistic transactional message pinned end to end in both formats. Every
  fault above lived in a combination of features rather than in one of them,
  which is why statement coverage did not catch any of them.
- TestRenderHTMLSurvivesBrokenMarkup asserts output rather than only err == nil,
  and the nesting-limit test asserts the cap rather than NotContains, which
  passed on empty output.
- README: the conversion section is its own, the startup rejection and the
  case-insensitivity are documented, the copy-paste example no longer enables a
  non-default, and "exactly as it arrived" is corrected — raw is decoded first.

Co-Authored-By: Claude Opus 5 (1M context) 
---
 README.md       | 109 +++++++-------
 backend.go      |  11 +-
 backend_test.go |  36 +++++
 config_test.go  |  12 ++
 email.go        | 248 ++++++++++++++++++++------------
 email_test.go   | 216 +++++++++++++++++++++++++++-
 format.go       |   4 +-
 html.go         | 370 ++++++++++++++++++++++++++++++++++++++----------
 html_test.go    | 168 ++++++++++++++++++++--
 9 files changed, 935 insertions(+), 239 deletions(-)

diff --git a/README.md b/README.md
index f0730a7..a71cb0e 100644
--- a/README.md
+++ b/README.md
@@ -33,7 +33,8 @@ Targets = [
 #   "raw"      forward the message unchanged (default)
 #   "text"     render an HTML body as plain text
 #   "markdown" render an HTML body as Markdown
-Format = "markdown"
+# See "Converting HTML messages" below.
+# Format = "markdown"
 
 # Single Target (Deprecated)
 # The Target field is still supported for backward compatibility
@@ -48,15 +49,16 @@ Target = "ntfy://ntfy.sh/legacy-topic?tags=thing"  # Will show deprecation warni
 # Shoutrrr services to forward unmatched emails to
 Targets = ["ntfy://ntfy.sh/catch-all-topic?tags=unmatched"]
 # Format applies to the catch-all as well
-Format = "text"
+# Format = "text"
 ```
 
 The server refuses to start if the configuration cannot forward anything — no
 recipients and no catch-all, a recipient without addresses or usable targets, or
 a mistyped table name such as `[[Recipient]]`, which TOML would otherwise accept
-silently. It also refuses to start without a `Username` and a `Password`: these
-used to fall back to `username`/`password`, which left the authentication gate
-open to the most obvious guess there is.
+silently, or a `Format` that names none of the supported conversions. It also
+refuses to start without a `Username` and a `Password`: these used to fall back
+to `username`/`password`, which left the authentication gate open to the most
+obvious guess there is.
 
 Clients must authenticate with the configured `Username` and `Password` before
 starting a mail transaction; an unauthenticated client is refused rather than
@@ -70,50 +72,6 @@ malformed `Content-Type`, for instance — which no retry could fix. A partial
 failure is logged but still accepted, since a retry would redeliver to every
 target and duplicate the notification on the ones that already have it.
 
-### Converting HTML messages
-
-Plenty of mail carries nothing but HTML, and plenty of notification services
-render none of it — a Mattermost direct message shows the markup verbatim.
-Setting `Format` on a recipient (or on `[CatchAll]`) rewrites an HTML body
-before it is forwarded:
-
-| `Format`   | Effect                                            |
-| ---------- | ------------------------------------------------- |
-| `raw`      | Forward the body exactly as it arrived. (default) |
-| `text`     | Render an HTML body as plain text.                |
-| `markdown` | Render an HTML body as Markdown.                  |
-
-Only an HTML body is ever rewritten. A message that arrives as plain text —
-including the `text/plain` alternative of a message that carries both — is what
-the sender chose to write and is forwarded untouched whatever `Format` says.
-
-The output is aimed at chat and push notifications rather than at reproducing
-the document, so:
-
-- Links keep their destination: `[label](url)` in Markdown, `label (url)` in
-  plain text. Destinations a reader cannot open (`#anchors`, `javascript:`,
-  inline `cid:` attachments) are dropped and their text is kept.
-- Images are dropped unless they have alt text, which removes tracking pixels
-  and sliced-up banners.
-- Tables become one line per row, since mail lays itself out in tables far more
-  often than it tabulates anything.
-- Hidden elements — the preheader line written for the inbox preview — are left
-  out.
-- A `
` becomes a single newline rather than a Markdown hard break, because - every service this forwards to renders one as a line break. - -Message text is escaped in Markdown mode so a subject line's `*` or a leading -`-` cannot turn into formatting. - -If a body is too deeply nested for the HTML parser it is forwarded unchanged -and a warning is logged: reformatting is a courtesy to the target, not a -condition of delivery. - -Bodies are decoded before they are rendered, so `quoted-printable` and `base64` -transfer encodings and non-UTF-8 character sets (`iso-8859-1`, -`windows-1252`, …) reach the target as readable text in every format, -including `raw`. - ### From releases - Grab the latest release from the [releases page](https://git.nakama.town/fmartingr/smtp2shoutrrr/releases) @@ -137,6 +95,59 @@ docker run -v /path/to/config.toml:/config.toml \ git.nakama.town/fmartingr/smtp2shoutrrr:latest ``` +## Converting HTML messages + +Plenty of mail carries nothing but HTML, and plenty of notification services +render none of it — a Mattermost direct message shows the markup verbatim. +Setting `Format` on a recipient (or on `[CatchAll]`) rewrites an HTML body +before it is forwarded: + +| `Format` | Effect | +| ---------- | --------------------------------------------------- | +| `raw` | Forward the body as the message wrote it. (default) | +| `text` | Render an HTML body as plain text. | +| `markdown` | Render an HTML body as Markdown. | + +The value is not case sensitive, and an unrecognised one is refused when the +server starts rather than silently forwarding raw HTML. + +Only an HTML body is ever rewritten. A message that arrives as plain text is +what the sender chose to write and is forwarded untouched whatever `Format` +says. Which part of a multipart message that is follows RFC 2046: the parts of +a `multipart/alternative` are the same content in several forms, so the +`text/plain` one is preferred where it is not empty; the parts of any other +multipart are cumulative, so the first one carrying a body is the message and +the footers, signatures and attachments after it are not. + +The output is aimed at chat and push notifications rather than at reproducing +the document, so: + +- Links keep their destination: `[label](url)` in Markdown, `label (url)` in + plain text. Destinations a reader cannot open (`#anchors`, `javascript:`, + inline `cid:` attachments) are dropped and their text is kept. +- Images are dropped unless they have alt text, which removes tracking pixels + and sliced-up banners. +- Tables become one line per row, since mail lays itself out in tables far more + often than it tabulates anything. +- Hidden elements — the preheader line written for the inbox preview — are left + out. +- A `
` becomes a single newline rather than a Markdown hard break, because + every service this forwards to renders one as a line break. + +Message text is escaped in Markdown mode so a subject line's `*` or a leading +`-` cannot turn into formatting. + +If a body is too deeply nested for the HTML parser, or renders to nothing at +all because it was images and tracking pixels, it is forwarded unchanged and a +warning is logged: reformatting is a courtesy to the target, not a condition of +delivery. Output past 64 KiB is cut short with a `…`, which no chat target +would have displayed anyway. + +Bodies are decoded before they are rendered, so `quoted-printable` and `base64` +transfer encodings and non-UTF-8 character sets (`iso-8859-1`, `windows-1252`, +…) reach the target as readable text in every format, `raw` included — which is +the one way `raw` is not quite the bytes that arrived. + ## Development Run the server with: diff --git a/backend.go b/backend.go index 50ae9cf..36f11a6 100644 --- a/backend.go +++ b/backend.go @@ -56,7 +56,7 @@ type Backend struct { config *Config } -func (bkd *Backend) sendNotifications(recipient ConfigRecipient, email ReceivedEmail) error { +func (bkd *Backend) sendNotifications(recipient ConfigRecipient, email *ReceivedEmail) error { // Get all target URLs (handles merging and caching) targetURLs := recipient.GetTargetURLs() @@ -135,7 +135,7 @@ func (bkd *Backend) NewSession(c *smtp.Conn) (smtp.Session, error) { }, nil } -func (bkd *Backend) forwardEmail(email ReceivedEmail) error { +func (bkd *Backend) forwardEmail(email *ReceivedEmail) error { slog.Info("forwading message", slog.String("to", strings.Join(email.Recipients, ","))) // Try to match configured recipients first @@ -182,7 +182,10 @@ type Session struct { config *Config - forwarderFunc func(ReceivedEmail) error + // forwarderFunc takes the message by pointer: the body is read from a + // single-use stream and cached on it, so a copy would forward an empty + // notification to every recipient after the first. + forwarderFunc func(*ReceivedEmail) error } func (s *Session) isAuthenticated() bool { @@ -293,7 +296,7 @@ func (s *Session) Data(r io.Reader) error { slog.Info("Received email", slog.String("destination", strings.Join(recipients, ","))) - if err := s.forwarderFunc(ReceivedEmail{ + if err := s.forwarderFunc(&ReceivedEmail{ Recipients: recipients, Msg: msg, }); err != nil { diff --git a/backend_test.go b/backend_test.go index feea93c..b3fe864 100644 --- a/backend_test.go +++ b/backend_test.go @@ -765,3 +765,39 @@ func TestHTMLBodyIsConvertedForTheRecipientsFormat(t *testing.T) { require.Contains(t, unconverted, `staging`, "a recipient that asked for nothing still receives the message as it arrived") } + +// The message body is read from a single-use stream and cached on the value +// that holds it, so passing it by value hands every recipient after the first +// an empty notification with a 250. +func TestMessageBodySurvivesASecondRecipient(t *testing.T) { + notifications := make(chan string, 4) + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + + return + } + + notifications <- string(body) + w.WriteHeader(http.StatusOK) + })) + defer mockServer.Close() + + recipient := ConfigRecipient{ + Addresses: []string{"test@example.com"}, + Targets: []string{"generic+" + mockServer.URL + "/?template=json"}, + } + + backend := &Backend{config: &Config{Username: "u", Password: "p"}} + email := &ReceivedEmail{ + Recipients: []string{"test@example.com"}, + Msg: readMessage(t, "Subject: Test\r\n\r\nthe body"), + } + + require.NoError(t, backend.sendNotifications(recipient, email)) + require.NoError(t, backend.sendNotifications(recipient, email)) + + require.Contains(t, <-notifications, "the body") + require.Contains(t, <-notifications, "the body") +} diff --git a/config_test.go b/config_test.go index e02263e..f25b472 100644 --- a/config_test.go +++ b/config_test.go @@ -243,3 +243,15 @@ Format = "plaintext" }) } } + +// Validate used to depend on SetDefaults having run, so a Config assembled in +// Go failed on a Format nobody had set. +func TestValidateAcceptsAConfigBuiltWithoutDefaults(t *testing.T) { + config := &Config{ + Username: "user", + Password: "secret", + Recipients: []ConfigRecipient{{Addresses: []string{"user@example.com"}, Targets: []string{"ntfy://ntfy.sh/topic"}}}, + } + + require.NoError(t, config.Validate()) +} diff --git a/email.go b/email.go index 4f07958..659c452 100644 --- a/email.go +++ b/email.go @@ -1,6 +1,7 @@ package smtp2shoutrrr import ( + "bytes" "encoding/base64" "errors" "fmt" @@ -12,6 +13,7 @@ import ( "net/mail" "net/textproto" "strings" + "unicode" "golang.org/x/net/html/charset" ) @@ -55,16 +57,17 @@ func (re *ReceivedEmail) FormattedBody(format BodyFormat) (string, error) { return "", err } - if format == FormatRaw || format == "" || !re.bodyIsHTML { + if format.normalize() == FormatRaw || !re.bodyIsHTML { return re.body, nil } + // Reformatting is a courtesy to the target, not a condition of delivery. + // A body the renderer cannot take apart, or one that renders to nothing + // because it was all images and tracking pixels, is forwarded as it + // arrived: an empty notification is refused by most targets, which puts + // the message into a retry loop it can never leave. rendered, err := renderHTML(re.body, format) if err != nil { - // Reformatting is a courtesy to the target, not a condition of - // delivery. A body the renderer cannot take apart is forwarded as it - // arrived, which is what the target would have received anyway had - // the recipient not asked for a format. slog.Warn("failed to render HTML body, forwarding it unchanged", slog.String("format", string(format)), slog.String("err", err.Error())) @@ -73,12 +76,11 @@ func (re *ReceivedEmail) FormattedBody(format BodyFormat) (string, error) { } if rendered == "" && strings.TrimSpace(re.body) != "" { - // Mail whose whole message is an image or a tracking beacon renders - // to nothing. The subject still goes out as the notification title, - // so this is worth a line in the log rather than a rejection. - slog.Warn("HTML body rendered to nothing", + slog.Warn("HTML body rendered to nothing, forwarding it unchanged", slog.String("format", string(format)), slog.Int("html_length", len(re.body))) + + return re.body, nil } return rendered, nil @@ -89,7 +91,7 @@ func (re *ReceivedEmail) readBody() error { return nil } - body, isHTML, err := readMIMEBody(textproto.MIMEHeader(re.Msg.Header), re.Msg.Body, 0) + body, isHTML, err := readEntity(textproto.MIMEHeader(re.Msg.Header), re.Msg.Body, 0) if err != nil { return err } @@ -99,9 +101,9 @@ func (re *ReceivedEmail) readBody() error { return nil } -// readMIMEBody picks the body out of one MIME entity, descending into -// multipart containers, and reports whether what it found is HTML. -func readMIMEBody(header textproto.MIMEHeader, body io.Reader, depth int) (string, bool, error) { +// readEntity reads one MIME entity — the message itself or a part of it — and +// reports the body it contributes and whether that body is HTML. +func readEntity(header textproto.MIMEHeader, body io.Reader, depth int) (string, bool, error) { contentType := header.Get("Content-Type") if contentType == "" { content, err := decodeContent(header, body, "") @@ -114,15 +116,19 @@ func readMIMEBody(header textproto.MIMEHeader, body io.Reader, depth int) (strin return "", false, fmt.Errorf("%w: parsing Content-Type %q: %w", errMalformedMessage, contentType, err) } + return readMediaType(header, body, mediaType, params, depth) +} + +func readMediaType( + header textproto.MIMEHeader, + body io.Reader, + mediaType string, + params map[string]string, + depth int, +) (string, bool, error) { switch { case strings.HasPrefix(mediaType, "multipart/"): - var selected bodySelector - if err := walkMultipart(&selected, body, params["boundary"], depth); err != nil { - return "", false, err - } - content, isHTML := selected.result() - - return content, isHTML, nil + return readMultipart(body, mediaType, params["boundary"], depth) case strings.HasPrefix(mediaType, "text/"): content, err := decodeContent(header, body, params["charset"]) @@ -136,99 +142,121 @@ func readMIMEBody(header textproto.MIMEHeader, body io.Reader, depth int) (strin } } -func walkMultipart(selected *bodySelector, body io.Reader, boundary string, depth int) error { +func readMultipart(body io.Reader, mediaType, boundary string, depth int) (string, bool, error) { if boundary == "" { - return fmt.Errorf("%w: multipart message without a boundary", errMalformedMessage) + return "", false, fmt.Errorf("%w: multipart message without a boundary", errMalformedMessage) } if depth >= maxMultipartDepth { - slog.Warn("stopped descending into a deeply nested message", - slog.Int("depth", depth)) + slog.Warn("stopped descending into a deeply nested message", slog.Int("depth", depth)) - return nil + return "", false, nil } + // RFC 2046 §5.1.4: the parts of a multipart/alternative are one content in + // several forms, so exactly one of them is chosen. The parts of any other + // multipart are cumulative (§5.1.3): the first one carrying a body is the + // message, and what follows it is footers, signatures and enclosures. + alternatives := mediaType == "multipart/alternative" + + var chosen bodySelector + reader := multipart.NewReader(body, boundary) + for { part, err := reader.NextPart() if errors.Is(err, io.EOF) { - return nil + break } if err != nil { // A truncated container still hands over the parts that were // readable. Only a message that yielded nothing at all is worth // refusing, since there is nothing left to notify with. - slog.Warn("stopped reading a multipart message early", - slog.String("err", err.Error())) + slog.Warn("stopped reading a multipart message early", slog.String("err", err.Error())) - if selected.empty() { - return fmt.Errorf("%w: reading multipart body: %w", errMalformedMessage, err) + if chosen.empty() { + return "", false, fmt.Errorf("%w: reading multipart body: %w", errMalformedMessage, err) } - return nil + break } - err = readPart(selected, part, depth) + content, isHTML, err := readPart(part, depth) _ = part.Close() if err != nil { - return err + // One unreadable part is not the whole message; the rest of the + // tree may still hold a body. + slog.Warn("ignoring an unreadable email part", slog.String("err", err.Error())) + + continue } - if selected.done() { - return nil + if !alternatives { + if strings.TrimSpace(content) != "" { + return content, isHTML, nil + } + + continue + } + + chosen.offer(content, isHTML) + + if chosen.done() { + break } } + + content, isHTML := chosen.result() + + return content, isHTML, nil } -func readPart(selected *bodySelector, part *multipart.Part, depth int) error { - if disposition, _, err := mime.ParseMediaType(part.Header.Get("Content-Disposition")); err == nil && - disposition == "attachment" { - // An attached .txt or .html file is something the sender enclosed, - // not what they wrote. - return nil - } - +func readPart(part *multipart.Part, depth int) (string, bool, error) { contentType := part.Header.Get("Content-Type") slog.Debug("Processing email part", slog.String("content_type", contentType)) - // RFC 2045 makes a part without a Content-Type plain US-ASCII text. + // mime.ParseMediaType hands back a usable media type alongside the error + // it reports for a malformed parameter — an empty charset, an unquoted + // file name — which older mailers emit often enough that dropping those + // parts would lose the message body itself. + if disposition, _, err := mime.ParseMediaType(part.Header.Get("Content-Disposition")); err == nil || + errors.Is(err, mime.ErrInvalidMediaParameter) { + if disposition == "attachment" { + // An attached .txt or .html file is something the sender enclosed, + // not what they wrote. + return "", false, nil + } + } + + // RFC 2045 §5.2 makes a part without a Content-Type plain US-ASCII text. mediaType, params := "text/plain", map[string]string{} if contentType != "" { var err error mediaType, params, err = mime.ParseMediaType(contentType) - if err != nil { - // One unreadable part among several is not the whole message; - // the rest of the tree may still hold a body. + if err != nil && !errors.Is(err, mime.ErrInvalidMediaParameter) { slog.Warn("ignoring email part with an unparseable Content-Type", slog.String("content_type", contentType), slog.String("err", err.Error())) - return nil + return "", false, nil + } + + if params == nil { + params = map[string]string{} } } - switch { - case strings.HasPrefix(mediaType, "multipart/"): - return walkMultipart(selected, part, params["boundary"], depth+1) - - case mediaType == "text/plain" || mediaType == "text/html": - if selected.has(mediaType) { - return nil - } - - content, err := decodeContent(part.Header, part, params["charset"]) - if err != nil { - return err - } - - selected.offer(mediaType, content) + if !strings.HasPrefix(mediaType, "multipart/") && + mediaType != "text/plain" && mediaType != "text/html" { + // text/calendar, an inline image, a signature: nothing to notify with. + return "", false, nil } - return nil + return readMediaType(part.Header, part, mediaType, params, depth+1) } // decodeContent turns one entity's bytes into a UTF-8 string, undoing the @@ -236,31 +264,76 @@ func readPart(selected *bodySelector, part *multipart.Part, depth int) error { // quoted-printable body reaches the target as "=E2=80=99" and a Latin-1 one as // mojibake — and neither survives a trip through an HTML renderer. func decodeContent(header textproto.MIMEHeader, body io.Reader, charsetLabel string) (string, error) { - raw, err := io.ReadAll(decodeTransferEncoding(body, header.Get("Content-Transfer-Encoding"))) + raw, err := io.ReadAll(body) if err != nil { - return "", fmt.Errorf("%w: decoding message body: %w", errMalformedMessage, err) + return "", fmt.Errorf("reading message body: %w", err) } - return decodeCharset(raw, charsetLabel), nil + return decodeCharset(decodeTransferEncoding(raw, header.Get("Content-Transfer-Encoding")), charsetLabel), nil } -func decodeTransferEncoding(body io.Reader, encoding string) io.Reader { +// decodeTransferEncoding is deliberately forgiving. A body that will not decode +// is still a body: forwarding the bytes as they arrived shows the reader +// something, where refusing the message tells the sender to stop retrying and +// loses the notification for good. +func decodeTransferEncoding(raw []byte, encoding string) []byte { switch strings.ToLower(strings.TrimSpace(encoding)) { case "base64": - return base64.NewDecoder(base64.StdEncoding, body) + return decodeBase64(raw) + case "quoted-printable": // mime/multipart already unwraps this for the parts it hands out and // drops the header with it, so only a single-part message arrives // here still encoded. - return quotedprintable.NewReader(body) + decoded, err := io.ReadAll(quotedprintable.NewReader(bytes.NewReader(raw))) + if err != nil { + slog.Warn("failed to decode a quoted-printable body, forwarding what could be decoded", + slog.String("err", err.Error())) + } + + if len(decoded) == 0 { + return raw + } + + return decoded + default: // 7bit, 8bit and binary are the identity encoding; anything else is - // something this server has no way to undo, and passing it through - // beats replacing the body with an error. - return body + // something this server has no way to undo. + return raw } } +// decodeBase64 tolerates the two liberties real mailers take with a base64 +// body: they wrap it across lines, and they leave the padding off the last +// group. +func decodeBase64(raw []byte) []byte { + compact := bytes.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + + return r + }, raw) + + encoding := base64.StdEncoding + if len(compact)%4 != 0 { + encoding = base64.RawStdEncoding + } + + decoded, err := encoding.DecodeString(string(compact)) + if err != nil { + slog.Warn("failed to decode a base64 body, forwarding what could be decoded", + slog.String("err", err.Error())) + } + + if len(decoded) == 0 { + return raw + } + + return decoded +} + func decodeCharset(raw []byte, label string) string { switch strings.ToLower(strings.TrimSpace(label)) { case "", "utf-8", "utf8", "us-ascii", "ascii": @@ -287,9 +360,11 @@ func decodeCharset(raw []byte, label string) string { return string(decoded) } -// bodySelector keeps the best body found while walking a message. A text/plain -// part beats a text/html one, and the first of each kind beats the rest, which -// is the order a multipart/alternative puts its own preference in. +// bodySelector picks between the forms of a multipart/alternative. Plain text +// beats HTML, and the first of each kind beats the rest, which is the order +// the message puts its own preference in. A blank part is held rather than +// chosen: senders that build the HTML from a template routinely emit an empty +// alternative beside it. type bodySelector struct { plain string html string @@ -297,24 +372,16 @@ type bodySelector struct { hasHTML bool } -func (s *bodySelector) has(mediaType string) bool { - if mediaType == "text/html" { - return s.hasHTML - } - - return s.done() -} - -func (s *bodySelector) offer(mediaType, content string) { - if mediaType == "text/html" { - if !s.hasHTML { +func (s *bodySelector) offer(content string, isHTML bool) { + if isHTML { + if !s.hasHTML || strings.TrimSpace(s.html) == "" { s.html, s.hasHTML = content, true } return } - if !s.done() { + if !s.hasPlain || strings.TrimSpace(s.plain) == "" { s.plain, s.hasPlain = content, true } } @@ -323,9 +390,8 @@ func (s *bodySelector) empty() bool { return !s.hasPlain && !s.hasHTML } -// done reports that nothing later in the message can improve on what has been -// found. An empty text/plain alternative does not count: senders emit those -// beside the HTML they actually wrote. +// done reports that nothing later in the container can improve on what has +// been found. func (s *bodySelector) done() bool { return s.hasPlain && strings.TrimSpace(s.plain) != "" } diff --git a/email_test.go b/email_test.go index 2b61d95..f806f85 100644 --- a/email_test.go +++ b/email_test.go @@ -279,17 +279,28 @@ func TestFormattedBodyTreatsUnsetFormatAsRaw(t *testing.T) { require.Equal(t, "

html body

\r\n", body) } -// Reformatting is a courtesy, so a body the renderer chokes on is still +// Reformatting is a courtesy, so a body the renderer cannot use is still // forwarded rather than costing the reader the notification. func TestFormattedBodyFallsBackToTheRawHTML(t *testing.T) { // The parser refuses a document with more than 512 elements open at once. - source := strings.Repeat("
", 600) + "deep" + strings.Repeat("
", 600) - email := ReceivedEmail{Msg: readMessage(t, - "Subject: Test\r\nContent-Type: text/html\r\n\r\n"+source)} + tooDeep := strings.Repeat("
", 600) + "deep" + strings.Repeat("
", 600) - body, err := email.FormattedBody(FormatMarkdown) - require.NoError(t, err) - require.Equal(t, source, body) + for name, source := range map[string]string{ + "a body the parser refuses": tooDeep, + // Otherwise shoutrrr is handed "", which Mattermost and Discord + // reject, so every target fails and the sender retries the same + // message forever. + "a body that renders to nothing": ``, + } { + t.Run(name, func(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/html\r\n\r\n"+source)} + + body, err := email.FormattedBody(FormatMarkdown) + require.NoError(t, err) + require.Equal(t, source, body) + }) + } } func TestBodyIsReadOnlyOnce(t *testing.T) { @@ -305,3 +316,194 @@ func TestBodyIsReadOnlyOnce(t *testing.T) { require.Equal(t, "body", first) require.Equal(t, first, second, "the message body is consumed as it is read") } + +// Regression: an unreadable transfer encoding used to be reported as a +// permanent 550, which tells the sender to stop retrying and loses a +// notification that main delivered. Real mailers emit unpadded base64. +func TestBodyToleratesAnUndecodableTransferEncoding(t *testing.T) { + for name, tc := range map[string]struct{ raw, want string }{ + "unpadded base64": { + raw: base64.RawStdEncoding.EncodeToString([]byte("hello world")), + want: "hello world", + }, + "base64 that is not base64 at all": { + raw: "!!!! not base64 !!!!", + want: "!!!! not base64 !!!!", + }, + } { + t.Run(name, func(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, + "Subject: Test\r\nContent-Type: text/plain\r\n"+ + "Content-Transfer-Encoding: base64\r\n\r\n"+tc.raw+"\r\n")} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, tc.want, strings.TrimSpace(body)) + }) + } +} + +// Regression: one part that would not decode used to fail the whole message, +// even with a perfectly good alternative already in hand. +func TestBodyKeepsAGoodPartBesideAnUnreadableOne(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/alternative; boundary="b"`, + "", + "--b", + "Content-Type: text/html", + "Content-Transfer-Encoding: base64", + "", + "!!!!not base64!!!!", + "--b", + "Content-Type: text/plain", + "", + "the real body", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "the real body", body) +} + +// Regression: mime.ParseMediaType returns a usable media type alongside the +// error it reports for a malformed parameter. Discarding both dropped the part +// that held the message and delivered an empty notification with a 250. +func TestBodyReadsPartsWithAMalformedContentType(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/alternative; boundary="b"`, + "", + "--b", + "Content-Type: text/plain; charset=", + "", + "still text", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "still text", body) +} + +// The same fault the other way round: the attachment guard used to fail open +// on the unquoted file names older mailers emit, so the enclosure won. +func TestBodySkipsAttachmentsWithAMalformedDisposition(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/mixed; boundary="b"`, + "", + "--b", + "Content-Type: text/plain", + "Content-Disposition: attachment; filename=log file.txt", + "", + "an enclosed file, not the message", + "--b", + "Content-Type: text/plain", + "", + "the message itself", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "the message itself", body) +} + +// RFC 2046 §5.1.3: the parts of a multipart/mixed are cumulative, so the first +// one carrying a body is the message and the rest are footers and enclosures. +// Applying "plain beats HTML" across them handed list mail its unsubscribe +// footer as the notification. +func TestBodyPrefersTheFirstPartOfACumulativeMultipart(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/mixed; boundary="b"`, + "", + "--b", + "Content-Type: text/html", + "", + "

the newsletter

", + "--b", + "Content-Type: text/plain", + "", + "You are receiving this because you subscribed.", + "--b--", + "", + }, "\r\n"))} + + body, err := email.FormattedBody(FormatMarkdown) + require.NoError(t, err) + require.Equal(t, "the newsletter", body, + "the HTML body is the message, and asking for Markdown must reach it") +} + +func TestBodyPassesOverAnEmptyHTMLAlternative(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/alternative; boundary="b"`, + "", + "--b", + "Content-Type: text/html", + "", + " ", + "--b", + "Content-Type: text/html", + "", + "

the real one

", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "

the real one

", body) +} + +// A container this server cannot finish reading still hands over the parts it +// read. Only a message that yielded nothing at all is refused, since there is +// then nothing left to notify with and the same bytes fail the same way on +// every redelivery. +func TestBodyKeepsWhatItReadFromABrokenContainer(t *testing.T) { + t.Run("a good part before the break is delivered", func(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/mixed; boundary="b"`, + "", + "--b", + "Content-Type: text/plain", + "", + "good body", + "--b", + "this is not a header", + "", + "x", + "--b--", + "", + }, "\r\n"))} + + body, err := email.Body() + require.NoError(t, err) + require.Equal(t, "good body", body) + }) + + t.Run("a break before anything readable is permanent", func(t *testing.T) { + email := ReceivedEmail{Msg: readMessage(t, strings.Join([]string{ + "Subject: Test", + `Content-Type: multipart/alternative; boundary="b"`, + "", + "--b", + "this is not a header", + "", + "body", + "--b--", + "", + }, "\r\n"))} + + _, err := email.Body() + require.ErrorIs(t, err, errMalformedMessage) + }) +} diff --git a/format.go b/format.go index fc9aff1..abb6c65 100644 --- a/format.go +++ b/format.go @@ -32,8 +32,10 @@ func (f BodyFormat) normalize() BodyFormat { return normalized } +// valid normalizes first, so a Config assembled in Go rather than loaded from +// a file does not fail validation on a Format nobody set. func (f BodyFormat) valid() bool { - return slices.Contains(bodyFormats, f) + return slices.Contains(bodyFormats, f.normalize()) } func formatNames() string { diff --git a/html.go b/html.go index 0fbb27c..417d3e1 100644 --- a/html.go +++ b/html.go @@ -1,7 +1,9 @@ package smtp2shoutrrr import ( + "bytes" "fmt" + "slices" "strconv" "strings" "unicode" @@ -10,11 +12,23 @@ import ( "golang.org/x/net/html/atom" ) -// maxHTMLDepth bounds how deep a message can make the renderer recurse. Mail -// templates nest a few dozen elements at most; anything past this is a -// generated document, and walking it would only trade stack for output nobody -// reads. -const maxHTMLDepth = 256 +const ( + // maxHTMLDepth bounds how deep a message can make the renderer recurse. It + // sits above the parser's own limit on open elements, so in practice the + // parser refuses a document first and this only matters if that limit ever + // moves. + maxHTMLDepth = 600 + + // maxRenderedBytes caps the output. Block prefixes are re-emitted on every + // line, so nesting multiplies against line count: without a cap, one + // message at the server's 1 MB limit renders to 125 MB and takes the + // process down with it. + maxRenderedBytes = 64 << 10 + + // truncationMarker tells the reader the message goes on, rather than + // letting a cap end it mid-sentence and look like the whole of it. + truncationMarker = "…" +) // renderHTML rewrites an HTML body as plain text or Markdown. The output is // aimed at chat and push notification services, so it favours a short readable @@ -30,7 +44,7 @@ func renderHTML(source string, format BodyFormat) (string, error) { return "", fmt.Errorf("parsing HTML body: %w", err) } - renderer := &htmlRenderer{markdown: format == FormatMarkdown, atLineStart: true} + renderer := newHTMLRenderer(format == FormatMarkdown, maxRenderedBytes) renderer.walk(document, 0) return renderer.result(), nil @@ -39,7 +53,11 @@ func renderHTML(source string, format BodyFormat) (string, error) { type htmlRenderer struct { markdown bool - out strings.Builder + out bytes.Buffer + // budget is the output this renderer may still produce; truncated records + // that it ran out, or that the document went deeper than it will follow. + budget int + truncated bool // prefix opens every line of the current context: the quote markers of // the blockquotes it sits in and the indentation of the list items. @@ -52,7 +70,16 @@ type htmlRenderer struct { // HTML is full of empty and wrappers, and writing their markers // eagerly would scatter stray asterisks and brackets through the output, // so a marker is only written once something follows it. - pending []string + pending []pendingMarkup + markupSeq int + // activeMarkers are the inline markers already open around the cursor. A + // marker nested inside itself would close the outer span early. + activeMarkers []string + + // lastClosed and closedAt locate the inline marker most recently written, + // so a span opening immediately against it can be merged into it. + lastClosed string + closedAt int // pendingNewlines is the separation the next content owes what precedes // it: one for a new line, two for a new block. Requests coalesce, so a @@ -75,26 +102,61 @@ type htmlRenderer struct { listStack []listLevel } +type pendingMarkup struct { + seq int + markup string + // mergeable marks an inline span that may be joined to the one it opens + // against, rather than closing and reopening the same emphasis. + mergeable bool +} + type listLevel struct { ordered bool index int } +func newHTMLRenderer(markdown bool, budget int) *htmlRenderer { + return &htmlRenderer{markdown: markdown, budget: budget, atLineStart: true} +} + func (r *htmlRenderer) result() string { lines := strings.Split(r.out.String(), "\n") for i := range lines { lines[i] = strings.TrimRight(lines[i], " \t") } - return strings.TrimSpace(strings.Join(lines, "\n")) + rendered := strings.TrimSpace(strings.Join(lines, "\n")) + if r.truncated && rendered != "" { + rendered += "\n" + truncationMarker + } + + return rendered } -// write appends to the output. strings.Builder never fails, and every caller -// has already decided the string belongs there. +// write appends to the output, up to the budget. bytes.Buffer never fails, and +// every caller has already decided the string belongs there. func (r *htmlRenderer) write(s string) { + if r.truncated { + return + } + + if len(s) > r.budget { + // Dropped whole rather than clipped: a cut inside a multi-byte rune + // would put invalid UTF-8 into the payload the target is sent as. + r.truncated = true + + return + } + + r.budget -= len(s) _, _ = r.out.WriteString(s) } +func (r *htmlRenderer) unwrite(n int) { + r.out.Truncate(r.out.Len() - n) + r.budget += n +} + // content settles everything the output owes — line breaks, indentation, a // collapsed space, markup opened earlier — and then writes s. func (r *htmlRenderer) content(s string) { @@ -137,16 +199,18 @@ func (r *htmlRenderer) flushNewlines() { return } - blank := r.pendingNewlines > 1 + newlines := r.pendingNewlines r.pendingNewlines = 0 + // A truly blank line closes a blockquote or a list item. The separator + // carries the prefix the blocks on either side of it share, so it keeps + // the contexts they are both inside open and lets the ones only one of + // them is inside end. + separator := strings.TrimRight(commonPrefix(r.breakPrefix, r.prefix), " ") + r.write("\n") - if blank { - // A truly blank line closes a blockquote or a list item. The - // separator carries the prefix the blocks on either side of it share, - // so it keeps the contexts they are both inside open and lets the - // ones only one of them is inside end. - r.write(strings.TrimRight(commonPrefix(r.breakPrefix, r.prefix), " ")) + for range newlines - 1 { + r.write(separator) r.write("\n") } @@ -184,8 +248,20 @@ func (r *htmlRenderer) flushSpace() { } func (r *htmlRenderer) flushPending() { - for _, markup := range r.pending { - r.write(markup) + for _, opened := range r.pending { + if opened.mergeable && r.lastClosed == opened.markup && r.closedAt == r.out.Len() { + // Two spans of the same kind meeting with nothing between them are + // one span. Writing both markers instead leaves a run of four + // asterisks, which Markdown reads as text rather than as emphasis + // — and splitting a bolded label into exactly that shape is what + // every Word and Outlook export does. + r.unwrite(len(opened.markup)) + r.lastClosed = "" + + continue + } + + r.write(opened.markup) } r.pending = r.pending[:0] @@ -193,13 +269,16 @@ func (r *htmlRenderer) flushPending() { // open queues markup and returns a closer that reports whether any content // followed it. A closer that reports false has already withdrawn the markup. -func (r *htmlRenderer) open(markup string) func() bool { - r.pending = append(r.pending, markup) - depth := len(r.pending) +func (r *htmlRenderer) open(markup string, mergeable bool) func() bool { + r.markupSeq++ + seq := r.markupSeq + r.pending = append(r.pending, pendingMarkup{seq: seq, markup: markup, mergeable: mergeable}) return func() bool { - if len(r.pending) >= depth && r.pending[depth-1] == markup { - r.pending = r.pending[:depth-1] + // Keyed on the sequence number rather than on the position, which + // repeats every time the queue drains. + if last := len(r.pending) - 1; last >= 0 && r.pending[last].seq == seq { + r.pending = r.pending[:last] return false } @@ -208,6 +287,14 @@ func (r *htmlRenderer) open(markup string) func() bool { } } +// closeMarkup writes a marker straight out, so a trailing space stays outside +// it where Markdown still recognises it as the end of the span. +func (r *htmlRenderer) closeMarkup(markup string) { + r.write(markup) + r.lastClosed = markup + r.closedAt = r.out.Len() +} + func (r *htmlRenderer) breakLine() { r.separate(1) } func (r *htmlRenderer) breakBlock() { r.separate(2) } @@ -222,7 +309,12 @@ func (r *htmlRenderer) separate(newlines int) { r.breakPrefix = r.prefix } - if newlines > r.pendingNewlines { + switch { + case r.preformatted > 0: + // A blank line inside preformatted text is content, so breaks add up + // there rather than merging into one. + r.pendingNewlines += newlines + case newlines > r.pendingNewlines: r.pendingNewlines = newlines } @@ -230,7 +322,13 @@ func (r *htmlRenderer) separate(newlines int) { } func (r *htmlRenderer) walk(node *html.Node, depth int) { + if r.truncated { + return + } + if depth > maxHTMLDepth { + r.truncated = true + return } @@ -247,11 +345,26 @@ func (r *htmlRenderer) walk(node *html.Node, depth int) { } func (r *htmlRenderer) walkChildren(node *html.Node, depth int) { - for child := node.FirstChild; child != nil; child = child.NextSibling { + for child := node.FirstChild; child != nil && !r.truncated; child = child.NextSibling { r.walk(child, depth+1) } } +// renderPlain renders a subtree as text alone. A code span and a fenced block +// hold no markup to escape and no markers to introduce, and a link's label has +// to be known before it is written to decide whether its destination adds +// anything to it. +func (r *htmlRenderer) renderPlain(node *html.Node, depth int, preformatted bool) string { + sub := newHTMLRenderer(false, r.budget) + if preformatted { + sub.preformatted = 1 + } + + sub.walkChildren(node, depth) + + return sub.result() +} + func (r *htmlRenderer) writeTextNode(data string) { data = sanitizeText(data) if data == "" { @@ -351,12 +464,7 @@ func (r *htmlRenderer) walkElement(node *html.Node, depth int) { r.renderInline("~~", node, depth) case atom.Code, atom.Kbd, atom.Samp: - if r.preformatted > 0 { - r.walkChildren(node, depth) - - return - } - r.renderInline("`", node, depth) + r.renderCode(node, depth) case atom.Td, atom.Th: // Mail is laid out in tables far more often than it tabulates @@ -385,7 +493,7 @@ func (r *htmlRenderer) renderHeading(node *html.Node, depth int) { r.breakBlock() if r.markdown { - closeHeading := r.open(strings.Repeat("#", headingLevels[node.DataAtom]) + " ") + closeHeading := r.open(strings.Repeat("#", headingLevels[node.DataAtom])+" ", false) r.walkChildren(node, depth) closeHeading() } else { @@ -457,43 +565,93 @@ func (r *htmlRenderer) renderBlockquote(node *html.Node, depth int) { } func (r *htmlRenderer) renderPreformatted(node *html.Node, depth int) { - r.breakBlock() - - if r.markdown { - r.content("```") - r.breakLine() - } - - r.preformatted++ - r.walkChildren(node, depth) - r.preformatted-- - - if r.markdown { - r.breakLine() - r.content("```") - } - - r.breakBlock() -} - -func (r *htmlRenderer) renderInline(marker string, node *html.Node, depth int) { - if !r.markdown { + if r.preformatted > 0 { + // Already inside preformatted text, where a second
 changes
+		// nothing — and rendering it separately would walk the subtree twice
+		// per level of nesting.
 		r.walkChildren(node, depth)
 
 		return
 	}
 
-	closeInline := r.open(marker)
-	r.walkChildren(node, depth)
-	if closeInline() {
-		// Written straight out so a trailing space stays outside the marker,
-		// where Markdown still recognises it as the end of the span.
-		r.write(marker)
+	r.breakBlock()
+
+	code := r.renderPlain(node, depth, true)
+
+	fence := ""
+	if r.markdown {
+		fence = backtickFence(code, 3)
+		r.content(fence)
+		r.breakLine()
 	}
+
+	// Emitted inside the preformatted count so the blank lines a diff or a
+	// stack trace is shaped by survive: outside it, consecutive breaks are one
+	// break.
+	r.preformatted++
+	for i, line := range strings.Split(code, "\n") {
+		if i > 0 {
+			r.breakLine()
+		}
+		r.content(line)
+	}
+	r.preformatted--
+
+	if r.markdown {
+		r.breakLine()
+		r.content(fence)
+	}
+
+	r.breakBlock()
+}
+
+func (r *htmlRenderer) renderCode(node *html.Node, depth int) {
+	if !r.markdown || r.preformatted > 0 {
+		r.walkChildren(node, depth)
+
+		return
+	}
+
+	// Rendered from its text rather than written through, for the two things
+	// Markdown does differently inside a code span: backslash escapes are
+	// literal there, and the span ends at the first backtick run as long as
+	// the one that opened it.
+	code := strings.ReplaceAll(r.renderPlain(node, depth, false), "\n", " ")
+	if code == "" {
+		return
+	}
+
+	padding := ""
+	if strings.HasPrefix(code, "`") || strings.HasSuffix(code, "`") {
+		padding = " "
+	}
+
+	fence := backtickFence(code, 1)
+	r.content(fence + padding + code + padding + fence)
+}
+
+func (r *htmlRenderer) renderInline(marker string, node *html.Node, depth int) {
+	if !r.markdown || slices.Contains(r.activeMarkers, marker) {
+		// A marker nested inside itself closes the outer span at the inner
+		// one's start and leaves the surplus markers in the text.
+		r.walkChildren(node, depth)
+
+		return
+	}
+
+	r.activeMarkers = append(r.activeMarkers, marker)
+	closeInline := r.open(marker, true)
+	r.walkChildren(node, depth)
+
+	if closeInline() {
+		r.closeMarkup(marker)
+	}
+
+	r.activeMarkers = r.activeMarkers[:len(r.activeMarkers)-1]
 }
 
 func (r *htmlRenderer) renderLink(node *html.Node, depth int) {
-	href := strings.TrimSpace(attrValue(node, "href"))
+	href := sanitizeURL(attrValue(node, "href"))
 	if !isUsableURL(href) {
 		r.walkChildren(node, depth)
 
@@ -501,22 +659,26 @@ func (r *htmlRenderer) renderLink(node *html.Node, depth int) {
 	}
 
 	if r.markdown {
-		closeLink := r.open("[")
+		closeLink := r.open("[", false)
 		r.walkChildren(node, depth)
+
 		if closeLink() {
-			r.write("](" + markdownURL(href) + ")")
-		} else {
-			// A link with nothing to click on is still worth forwarding.
-			r.text(href)
+			r.closeMarkup("](" + markdownURL(href) + ")")
+
+			return
 		}
 
+		// A link with nothing to click on is still worth forwarding.
+		r.text(href)
+
 		return
 	}
 
-	start := r.out.Len()
+	// Rendered once on its own first: a label that is already the destination
+	// does not want it repeated, and one that is empty wants it instead.
+	label := r.renderPlain(node, depth, false)
 	r.walkChildren(node, depth)
 
-	label := strings.TrimSpace(r.out.String()[start:])
 	switch {
 	case label == "":
 		r.text(href)
@@ -527,14 +689,14 @@ func (r *htmlRenderer) renderLink(node *html.Node, depth int) {
 }
 
 func (r *htmlRenderer) renderImage(node *html.Node) {
-	alt := strings.TrimSpace(sanitizeText(attrValue(node, "alt")))
+	alt := collapseSpace(sanitizeText(attrValue(node, "alt")))
 	if alt == "" {
 		// Tracking pixels, spacers and sliced-up banners have no alt text and
 		// nothing to say.
 		return
 	}
 
-	source := strings.TrimSpace(attrValue(node, "src"))
+	source := sanitizeURL(attrValue(node, "src"))
 	if r.markdown && isUsableURL(source) {
 		r.content("![" + markdownEscaper.Replace(alt) + "](" + markdownURL(source) + ")")
 
@@ -638,6 +800,26 @@ func escapeLineStart(s string) string {
 	return s
 }
 
+// backtickFence returns a run of backticks longer than any run in the content,
+// so the content cannot close the span or block that holds it and spill the
+// rest of the message out as live Markdown.
+func backtickFence(content string, minimum int) string {
+	longest, run := 0, 0
+
+	for _, char := range content {
+		if char != '`' {
+			run = 0
+
+			continue
+		}
+
+		run++
+		longest = max(longest, run)
+	}
+
+	return strings.Repeat("`", max(longest+1, minimum))
+}
+
 // markdownURL fits a URL into a link destination. Spaces and parentheses would
 // end the destination early, so those URLs take the angle bracket form.
 func markdownURL(raw string) string {
@@ -680,6 +862,31 @@ func isScheme(s string) bool {
 	}) < 0
 }
 
+// sanitizeURL applies to a URL attribute what a browser applies before it
+// fetches one: the tabs and line breaks that may sit inside the value are
+// removed. Left in, they end the line the notification is on, which is a place
+// to write a sentence the reader will take for the sender's.
+func sanitizeURL(raw string) string {
+	return strings.Map(func(r rune) rune {
+		if r == ' ' {
+			return ' '
+		}
+
+		if unicode.IsSpace(r) || unicode.IsControl(r) {
+			return -1
+		}
+
+		return r
+	}, strings.TrimSpace(raw))
+}
+
+// collapseSpace squeezes the runs of whitespace an attribute value may hold
+// into single spaces, for the same reason: alt text spans lines in the source
+// and must not span them in the message.
+func collapseSpace(s string) string {
+	return strings.Join(strings.Fields(s), " ")
+}
+
 // sanitizeText turns the characters mail uses for layout into ones a
 // notification can show: the non-breaking spaces that hold table cells apart
 // become ordinary spaces, and the zero-width padding that hides a preheader
@@ -717,7 +924,7 @@ func isHidden(node *html.Node) bool {
 		case "hidden":
 			return true
 		case "style":
-			if strings.Contains(spaceStripper.Replace(strings.ToLower(attr.Val)), "display:none") {
+			if isHidingStyle(spaceStripper.Replace(strings.ToLower(attr.Val))) {
 				return true
 			}
 		}
@@ -726,6 +933,25 @@ func isHidden(node *html.Node) bool {
 	return false
 }
 
+func isHidingStyle(style string) bool {
+	for _, declaration := range []string{"display:none", "visibility:hidden", "mso-hide:all"} {
+		if strings.Contains(style, declaration) {
+			return true
+		}
+	}
+
+	// font-size:0 hides a preheader too, but only when the zero is the whole
+	// value: font-size:0.9em is an ordinary line of text.
+	if index := strings.Index(style, "font-size:0"); index >= 0 {
+		rest := strings.TrimPrefix(style[index+len("font-size:0"):], "px")
+		rest = strings.TrimPrefix(rest, "pt")
+
+		return rest == "" || strings.HasPrefix(rest, ";")
+	}
+
+	return false
+}
+
 var spaceStripper = strings.NewReplacer(" ", "", "\t", "", "\n", "", "\r", "")
 
 // commonPrefix returns the leading run the two prefixes agree on.
diff --git a/html_test.go b/html_test.go
index 05ad025..c8a6a53 100644
--- a/html_test.go
+++ b/html_test.go
@@ -3,6 +3,7 @@ package smtp2shoutrrr
 import (
 	"strings"
 	"testing"
+	"unicode/utf8"
 
 	"github.com/stretchr/testify/require"
 )
@@ -146,6 +147,53 @@ func TestRenderHTMLAsMarkdown(t *testing.T) {
 			source: "

see config_test.go

", want: "see config_test.go", }, + "two spans of the same emphasis meeting head on become one": { + // What every Word and Outlook export does to a bolded label. + source: "

Total: 49.99

", + want: "**Total: 49.99**", + }, + "emphasis nested inside itself does not double its markers": { + source: "

ab

", + want: "**ab**", + }, + "spans with a space between them stay apart": { + source: "

a b

", + want: "**a** **b**", + }, + "a code span is literal, so its text is not escaped": { + source: "

retry_after=30*60

", + want: "`retry_after=30*60`", + }, + "a code span is fenced past the backticks it holds": { + source: "

a`b

", + want: "``a`b``", + }, + "a code block is fenced past the backticks it holds": { + // Otherwise the sender closes the block and everything after it — + // a heading, a link — renders as live Markdown in a message the + // reader takes for a forwarded notification. + source: "
ok\n```\n## Your account is locked\n
", + want: "````\nok\n```\n## Your account is locked\n````", + }, + "a code block keeps the blank lines it is shaped by": { + source: "
ok\n\n\nnext
", + want: "```\nok\n\n\nnext\n```", + }, + "line breaks inside a URL are removed rather than forwarded": { + // A newline in an href is invisible in the document and a + // fabricated line in the notification. + source: "
open report", + want: "[open report]()", + }, + "alt text is collapsed onto one line": { + source: "\"a\n", + want: "![a long alt](https://x.example/i.png)", + }, + "the other ways a preheader hides are recognised too": { + source: `
a
b
` + + `
c
d

body

`, + want: "d\n\nbody", + }, "a document with nothing to say renders to nothing": { source: ``, want: "", @@ -191,6 +239,14 @@ func TestRenderHTMLAsText(t *testing.T) { source: "
  • one
  • two
", want: "- one\n- two", }, + "a bare URL is not repeated even when its line opens with a bullet": { + source: ``, + want: "- https://e.com/x", + }, + "line breaks inside a URL are removed rather than forwarded": { + source: "open report", + want: "open report (https://ok.example/rURGENT: wire funds)", + }, } { t.Run(name, func(t *testing.T) { require.Equal(t, tc.want, render(t, FormatText, tc.source)) @@ -201,28 +257,110 @@ func TestRenderHTMLAsText(t *testing.T) { // The parser recovers from anything, so the renderer has to as well: a // notification is worth more than a report that the markup was invalid. func TestRenderHTMLSurvivesBrokenMarkup(t *testing.T) { - for name, source := range map[string]string{ - "unclosed tags": "

boldboth

next", - "stray closing tags": "

text", - "no markup at all": "just some words", - "an empty document": "", - "an unterminated tag": "

text<", + for name, tc := range map[string]struct{ source, want string }{ + "unclosed tags": {"

boldboth

next", "**bold*both***\n\n***next***"}, + "stray closing tags": {"

text", "text"}, + "no markup at all": {"just some words", "just some words"}, + "an empty document": {"", ""}, + "an unterminated tag": {"

text<", "text\\<"}, } { t.Run(name, func(t *testing.T) { - for _, format := range []BodyFormat{FormatText, FormatMarkdown} { - _, err := renderHTML(source, format) - require.NoError(t, err) - } + require.Equal(t, tc.want, render(t, FormatMarkdown, tc.source)) }) } } -// A generated document should cost bounded work rather than the whole stack. -func TestRenderHTMLStopsAtNestingLimit(t *testing.T) { - depth := maxHTMLDepth + 50 - source := strings.Repeat("

", depth) + "too deep" + strings.Repeat("
", depth) +// Regression: block prefixes are re-emitted on every line, so nesting +// multiplies against line count. One message at the server's own 1 MB limit +// rendered to 125 MB and took the process down with it — the same class of +// fault as the remote DoS closed in 7565618. +func TestRenderHTMLCapsItsOutput(t *testing.T) { + source := strings.Repeat("
", 250) + strings.Repeat("

x

", 131000) out, err := renderHTML(source, FormatMarkdown) require.NoError(t, err) - require.NotContains(t, out, "too deep") + require.LessOrEqual(t, len(out), maxRenderedBytes+len(truncationMarker)+1) + require.True(t, strings.HasSuffix(out, truncationMarker), + "a capped message says so rather than ending mid-sentence") + require.True(t, utf8.ValidString(out), "a cap must not fall inside a rune") } + +// The renderer's own depth guard sits above the parser's limit on open +// elements, so the parser refuses a document first and email.go forwards it +// unchanged. Content is never dropped silently in between. +func TestRenderHTMLDepthGuardSitsAboveTheParsersOwn(t *testing.T) { + require.Greater(t, maxHTMLDepth, 512) + + depth := 600 + _, err := renderHTML(strings.Repeat("
", depth)+"deep"+strings.Repeat("
", depth), + FormatMarkdown) + require.Error(t, err) +} + +// A realistic transactional message, pinned end to end. Every finding this +// renderer has had lived in a combination of features rather than in one of +// them, which is what a fixture catches and a unit test does not. +func TestRenderHTMLOnARealisticMessage(t *testing.T) { + require.Equal(t, strings.Join([]string{ + "[![Example Billing](https://cdn.example.com/logo.png)](https://billing.example.com)", + "", + "## Invoice #2026-0231", + "", + "Hi Ferran,", + "", + "Your invoice for **February 2026** **(30 days)** is ready. The total is " + + "**€42.00**, charged to the card ending 4242 on *1 March*.", + "", + "Description Qty Amount", + "Hosting — small 1 €30.00", + "Backups 2 €12.00", + "", + "Reference: `inv_2026*0231`", + "[View invoice](https://billing.example.com/invoices/2026-0231?utm_source=email&utm_medium=cta)", + "", + "You are receiving this because you have an account. " + + "[Unsubscribe](https://billing.example.com/unsubscribe?t=abc) · Back to top", + }, "\n"), render(t, FormatMarkdown, marketingMessage)) + + require.Equal(t, strings.Join([]string{ + "Example Billing (https://billing.example.com)", + "", + "Invoice #2026-0231", + "", + "Hi Ferran,", + "", + "Your invoice for February 2026 (30 days) is ready. The total is €42.00, " + + "charged to the card ending 4242 on 1 March.", + "", + "Description Qty Amount", + "Hosting — small 1 €30.00", + "Backups 2 €12.00", + "", + "Reference: inv_2026*0231", + "View invoice (https://billing.example.com/invoices/2026-0231?utm_source=email&utm_medium=cta)", + "", + "You are receiving this because you have an account. " + + "Unsubscribe (https://billing.example.com/unsubscribe?t=abc) · Back to top", + }, "\n"), render(t, FormatText, marketingMessage)) +} + +// Shaped like the transactional mail this feature exists for: a hidden +// preheader, a layout table, a logo wrapped in a link, an Outlook-style split +// bold run, a tracking pixel and an in-page anchor. +const marketingMessage = `Invoice + +Your February invoice is ready​​ +
+ + + + + + + +
Example Billing

Invoice #2026-0231

Hi Ferran,

Your invoice for February 2026 (30 days) is ready. +The total is €42.00, charged to the card ending 4242 on 1 March.
+ +
DescriptionQtyAmount
Hosting — small1€30.00
Backups2€12.00
Reference: inv_2026*0231
View invoice

You are receiving this because you have an account. +Unsubscribe · Back to top

+
` -- 2.52.0 From 06de333118fe1a67e747a8daad0376e7d34e3b89 Mon Sep 17 00:00:00 2001 From: butterrobot Date: Thu, 10 Sep 2026 09:09:46 +0000 Subject: [PATCH 3/3] refactor: convert HTML with html-to-markdown instead of by hand (FMG-9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-written renderer is replaced by github.com/JohannesKaufmann/html-to-markdown/v2 plus the mail-specific policy it has no opinion about. 986 lines of html.go become 372; the conversion itself — CommonMark escaping, delimiter runs, fencing a code block past the backticks inside it — is now a maintained library's problem rather than ours. The original justification for writing it by hand was that the library would drag in goquery and its dependencies. That was true of v1 and wrong for v2, which dropped it: the measured cost is two modules, html-to-markdown/v2 and JohannesKaufmann/dom, on top of the golang.org/x/net this already used. What the library does not know is mail, because it is written for documents. The parsed message is prepared before conversion: - Hidden preheaders, written for the inbox list, are removed. - Images without alt text go, which takes the tracking pixels, spacers and sliced-up banners with them. An inline cid: attachment leaves its alt text behind as ordinary words. - Destinations a reader cannot open are dropped and the link text kept; tabs and line breaks are stripped from the rest, since a line break inside an href is invisible in the document and a fabricated line in the message. - Table rows become lines and their cells stay apart, which the converter has no rule for: "Total4Failed0" otherwise. - A link left holding nothing but a pixel falls back to its own destination rather than rendering as an invisible "[](url)". - Quote and list nesting is flattened past six levels, and the output is capped at 64 KiB with a marker. That last one is not something any of the candidates solved. html-to-markdown amplifies exactly as the hand-written renderer did before it was capped, from the same cause — a line prefix re-emitted per line and per level. Measured on one message at the server's own 1 MB limit, nested 250 deep: 131 MB of output over 2m16s, against 64 KiB in 1.5s and 85 MiB of peak heap with the flattening in place. Format = "text" is dropped, leaving raw and markdown. Markdown reads as plain text wherever nothing renders it, so a second conversion would only have been a worse copy of this one, and the plain-text libraries surveyed were the weak half of the field. Nothing has shipped with "text", so no released configuration names it; an unknown Format is still refused at startup. The test suite carries over almost unchanged, because it asserts output rather than internals — which is what made the swap safe to judge. Every mail-policy and injection case still holds, and the pathological-input test is sized from the constants now so the suite stays quick. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 47 +-- config_test.go | 14 +- email.go | 2 +- email_test.go | 3 +- format.go | 9 +- go.mod | 2 + go.sum | 12 + html.go | 954 ++++++++++--------------------------------------- html_test.go | 298 +++++---------- 9 files changed, 326 insertions(+), 1015 deletions(-) diff --git a/README.md b/README.md index a71cb0e..538800e 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,6 @@ Targets = [ ] # Optional: how an HTML message body should reach these targets. # "raw" forward the message unchanged (default) -# "text" render an HTML body as plain text # "markdown" render an HTML body as Markdown # See "Converting HTML messages" below. # Format = "markdown" @@ -49,7 +48,7 @@ Target = "ntfy://ntfy.sh/legacy-topic?tags=thing" # Will show deprecation warni # Shoutrrr services to forward unmatched emails to Targets = ["ntfy://ntfy.sh/catch-all-topic?tags=unmatched"] # Format applies to the catch-all as well -# Format = "text" +# Format = "markdown" ``` The server refuses to start if the configuration cannot forward anything — no @@ -105,9 +104,12 @@ before it is forwarded: | `Format` | Effect | | ---------- | --------------------------------------------------- | | `raw` | Forward the body as the message wrote it. (default) | -| `text` | Render an HTML body as plain text. | | `markdown` | Render an HTML body as Markdown. | +There is deliberately no plain-text format beside it. Markdown reads as plain +text wherever nothing renders it — that is rather the point of Markdown — so a +second conversion would only be a worse copy of this one. + The value is not case sensitive, and an unrecognised one is refused when the server starts rather than silently forwarding raw HTML. @@ -119,23 +121,28 @@ a `multipart/alternative` are the same content in several forms, so the multipart are cumulative, so the first one carrying a body is the message and the footers, signatures and attachments after it are not. -The output is aimed at chat and push notifications rather than at reproducing -the document, so: +The Markdown itself is produced by +[html-to-markdown](https://github.com/JohannesKaufmann/html-to-markdown), which +knows CommonMark — escaping, delimiter runs, fencing a code block past the +backticks inside it. What it does not know is mail, because it is written for +documents. Before the conversion runs, the parsed message is stripped down to +what a notification should carry: -- Links keep their destination: `[label](url)` in Markdown, `label (url)` in - plain text. Destinations a reader cannot open (`#anchors`, `javascript:`, - inline `cid:` attachments) are dropped and their text is kept. -- Images are dropped unless they have alt text, which removes tracking pixels - and sliced-up banners. -- Tables become one line per row, since mail lays itself out in tables far more - often than it tabulates anything. -- Hidden elements — the preheader line written for the inbox preview — are left - out. -- A `
` becomes a single newline rather than a Markdown hard break, because - every service this forwards to renders one as a line break. - -Message text is escaped in Markdown mode so a subject line's `*` or a leading -`-` cannot turn into formatting. +- Hidden elements go. A template opens with a preheader written for the inbox + list, which reads as noise anywhere else. +- Images without alt text go, which removes tracking pixels, spacers and + sliced-up banners. An inline `cid:` attachment leaves its alt text behind as + ordinary words. +- Destinations a reader cannot open — `#anchors`, `javascript:`, `cid:` — are + dropped and the text of the link is kept. Tabs and line breaks are removed + from the rest, since a line break inside an `href` is invisible in the + document and a fabricated line in the message. +- Table rows become lines and their cells stay apart, since mail lays itself + out in tables far more often than it tabulates anything. +- Quoting and list nesting is flattened past six levels. A line prefix is + re-emitted on every line of every level it nests, so depth multiplies against + line count: a message at the 1 MB limit nested 250 quotes deep otherwise + renders to 131 MB and takes minutes of CPU. If a body is too deeply nested for the HTML parser, or renders to nothing at all because it was images and tracking pixels, it is forwarded unchanged and a @@ -145,7 +152,7 @@ would have displayed anyway. Bodies are decoded before they are rendered, so `quoted-printable` and `base64` transfer encodings and non-UTF-8 character sets (`iso-8859-1`, `windows-1252`, -…) reach the target as readable text in every format, `raw` included — which is +…) reach the target as readable text in both formats, `raw` included — which is the one way `raw` is not quite the bytes that arrived. ## Development diff --git a/config_test.go b/config_test.go index f25b472..296856b 100644 --- a/config_test.go +++ b/config_test.go @@ -199,11 +199,6 @@ Addresses = ["user@example.com"] Targets = ["ntfy://ntfy.sh/topic"] Format = "Markdown" -[[Recipients]] -Addresses = ["plain@example.com"] -Targets = ["ntfy://ntfy.sh/plain"] -Format = "text" - [[Recipients]] Addresses = ["asis@example.com"] Targets = ["ntfy://ntfy.sh/asis"] @@ -215,8 +210,7 @@ Format = "markdown" require.NoError(t, err) require.Equal(t, FormatMarkdown, config.Recipients[0].Format, "the option is not case sensitive") - require.Equal(t, FormatText, config.Recipients[1].Format) - require.Equal(t, FormatRaw, config.Recipients[2].Format, "an unset Format forwards the message unchanged") + require.Equal(t, FormatRaw, config.Recipients[1].Format, "an unset Format forwards the message unchanged") require.Equal(t, FormatMarkdown, config.CatchAll.Format) } @@ -228,18 +222,18 @@ func TestLoadConfigRejectsUnknownFormat(t *testing.T) { [[Recipients]] Addresses = ["user@example.com"] Targets = ["ntfy://ntfy.sh/topic"] -Format = "md" +Format = "text" `, "on the catch-all": ` [CatchAll] Targets = ["ntfy://ntfy.sh/catch-all"] -Format = "plaintext" +Format = "md" `, } { t.Run(name, func(t *testing.T) { _, err := LoadConfig(writeConfig(t, credentials+contents)) require.Error(t, err) - require.Contains(t, err.Error(), "raw, text, markdown") + require.Contains(t, err.Error(), "raw, markdown") }) } } diff --git a/email.go b/email.go index 659c452..7d0e513 100644 --- a/email.go +++ b/email.go @@ -66,7 +66,7 @@ func (re *ReceivedEmail) FormattedBody(format BodyFormat) (string, error) { // because it was all images and tracking pixels, is forwarded as it // arrived: an empty notification is refused by most targets, which puts // the message into a retry loop it can never leave. - rendered, err := renderHTML(re.body, format) + rendered, err := renderMarkdown(re.body) if err != nil { slog.Warn("failed to render HTML body, forwarding it unchanged", slog.String("format", string(format)), diff --git a/email_test.go b/email_test.go index f806f85..805541d 100644 --- a/email_test.go +++ b/email_test.go @@ -239,7 +239,6 @@ func TestFormattedBodyConvertsHTML(t *testing.T) { for format, want := range map[BodyFormat]string{ FormatMarkdown: "Build **failed**: [run 42](https://ci.example.com/42)", - FormatText: "Build failed: run 42 (https://ci.example.com/42)", FormatRaw: "

Build failed: run 42

\r\n", } { t.Run(string(format), func(t *testing.T) { @@ -257,7 +256,7 @@ func TestFormattedBodyConvertsHTML(t *testing.T) { func TestFormattedBodyLeavesPlainTextAlone(t *testing.T) { raw := "Subject: Test\r\nContent-Type: text/plain; charset=utf-8\r\n\r\n2 * 3 = 6 \r\n" - for _, format := range []BodyFormat{FormatRaw, FormatText, FormatMarkdown} { + for _, format := range []BodyFormat{FormatRaw, FormatMarkdown} { t.Run(string(format), func(t *testing.T) { email := ReceivedEmail{Msg: readMessage(t, raw)} diff --git a/format.go b/format.go index abb6c65..f7ed018 100644 --- a/format.go +++ b/format.go @@ -13,13 +13,14 @@ type BodyFormat string const ( // FormatRaw forwards the body exactly as the message carried it. FormatRaw BodyFormat = "raw" - // FormatText renders an HTML body as plain text. - FormatText BodyFormat = "text" - // FormatMarkdown renders an HTML body as Markdown. + // FormatMarkdown renders an HTML body as Markdown. There is deliberately + // no plain-text format beside it: Markdown reads as plain text wherever + // nothing renders it, so a second conversion would only be a worse copy + // of this one. FormatMarkdown BodyFormat = "markdown" ) -var bodyFormats = []BodyFormat{FormatRaw, FormatText, FormatMarkdown} +var bodyFormats = []BodyFormat{FormatRaw, FormatMarkdown} // normalize maps an unset Format to the one that changes nothing, and accepts // the casing a hand-written configuration file is likely to use. diff --git a/go.mod b/go.mod index fed4744..5ce3be2 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.27 toolchain go1.27.1 require ( + github.com/JohannesKaufmann/html-to-markdown/v2 v2.5.2 github.com/containrrr/shoutrrr v0.8.0 github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 github.com/emersion/go-smtp v0.25.0 @@ -15,6 +16,7 @@ require ( ) require ( + github.com/JohannesKaufmann/dom v0.3.1 // indirect github.com/fatih/color v1.19.0 // indirect github.com/go-logr/logr v1.4.4 // indirect github.com/golang/protobuf v1.5.4 // indirect diff --git a/go.sum b/go.sum index 5b3ff23..d052506 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,7 @@ +github.com/JohannesKaufmann/dom v0.3.1 h1:J16l9JAHWgkFPR3VIPbQ1gvS0cWab6laK1q7PFL3qh0= +github.com/JohannesKaufmann/dom v0.3.1/go.mod h1:BZPkf8ZeYrBgABjwJn9iiKt8aiCtkxpHkevms+Yp2DE= +github.com/JohannesKaufmann/html-to-markdown/v2 v2.5.2 h1:XFJZFWESIWlUEHHjzBuv8RvrtCWnSGlimEX17ysSDb8= +github.com/JohannesKaufmann/html-to-markdown/v2 v2.5.2/go.mod h1:BHWO8lJzttJLqwuV8Rb1B3OG2OSzLbssZDI1FRg2eAA= github.com/containrrr/shoutrrr v0.8.0 h1:mfG2ATzIS7NR2Ec6XL+xyoHzN97H8WPjir8aYzJUSec= github.com/containrrr/shoutrrr v0.8.0/go.mod h1:ioyQAyu1LJY6sILuNyKaQaw+9Ttik5QePU8atnAdO2o= github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 h1:oP4q0fw+fOSWn3DfFi4EXdT+B+gTtzx8GC9xsc26Znk= @@ -28,8 +32,16 @@ github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/sebdah/goldie/v2 v2.8.0 h1:dZb9wR8q5++oplmEiJT+U/5KyotVD+HNGCAc5gNr8rc= +github.com/sebdah/goldie/v2 v2.8.0/go.mod h1:oZ9fp0+se1eapSRjfYbsV/0Hqhbuu3bJVvKI/NNtssI= +github.com/sergi/go-diff v1.4.0 h1:n/SP9D5ad1fORl+llWyN+D6qoUETXNZARKjyY2/KVCw= +github.com/sergi/go-diff v1.4.0/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= +github.com/yuin/goldmark v1.8.2 h1:kEGpgqJXdgbkhcOgBxkC0X0PmoPG1ZyoZ117rDVp4zE= +github.com/yuin/goldmark v1.8.2/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg= go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= golang.org/x/crypto/x509roots/fallback v0.0.0-20260902180247-86efde54dc70 h1:VwViOGcd7C8/Gs18efVlMxvUCS0un6KRKBPFEBvpUXg= diff --git a/html.go b/html.go index 417d3e1..0325b5c 100644 --- a/html.go +++ b/html.go @@ -1,42 +1,46 @@ package smtp2shoutrrr import ( - "bytes" "fmt" - "slices" - "strconv" "strings" "unicode" + "unicode/utf8" + "github.com/JohannesKaufmann/html-to-markdown/v2/converter" + "github.com/JohannesKaufmann/html-to-markdown/v2/plugin/base" + "github.com/JohannesKaufmann/html-to-markdown/v2/plugin/commonmark" + "github.com/JohannesKaufmann/html-to-markdown/v2/plugin/strikethrough" "golang.org/x/net/html" "golang.org/x/net/html/atom" ) const ( - // maxHTMLDepth bounds how deep a message can make the renderer recurse. It - // sits above the parser's own limit on open elements, so in practice the - // parser refuses a document first and this only matters if that limit ever - // moves. - maxHTMLDepth = 600 + // maxNestingDepth bounds the elements that give every line they contain a + // prefix. The converter re-emits that prefix per line and per level, so + // depth multiplies against line count: a message at the server's own 1 MB + // limit, nested 250 quotes deep, renders to 131 MB and takes minutes. + // Real mail quotes a handful of levels and indents fewer. + maxNestingDepth = 6 - // maxRenderedBytes caps the output. Block prefixes are re-emitted on every - // line, so nesting multiplies against line count: without a cap, one - // message at the server's 1 MB limit renders to 125 MB and takes the - // process down with it. + // maxRenderedBytes caps what is forwarded. No chat target displays this + // much, and a notification is not the place to find out. maxRenderedBytes = 64 << 10 // truncationMarker tells the reader the message goes on, rather than - // letting a cap end it mid-sentence and look like the whole of it. + // letting the cap end it mid-sentence and look like the whole of it. truncationMarker = "…" ) -// renderHTML rewrites an HTML body as plain text or Markdown. The output is -// aimed at chat and push notification services, so it favours a short readable -// message over a faithful reproduction of the document: layout tables become -// lines, images without alt text disappear, and a single newline is used where -// a strict CommonMark writer would need a hard break, because every service -// this forwards to renders one as a line break. -func renderHTML(source string, format BodyFormat) (string, error) { +// renderMarkdown rewrites an HTML body as Markdown. +// +// The conversion itself belongs to html-to-markdown, which knows CommonMark — +// escaping, delimiter runs, fencing a code block past the backticks inside it. +// What it has no opinion about is mail, since it is written for documents: a +// document has no preheader written for an inbox list, no tracking pixel, no +// cid: attachment, and lays nothing out in tables. That is what prepare does +// to the tree first, so the converter only ever sees what a notification +// should carry. +func renderMarkdown(source string) (string, error) { document, err := html.Parse(strings.NewReader(source)) if err != nil { // The parser recovers from any markup it can hold, so this is a @@ -44,790 +48,196 @@ func renderHTML(source string, format BodyFormat) (string, error) { return "", fmt.Errorf("parsing HTML body: %w", err) } - renderer := newHTMLRenderer(format == FormatMarkdown, maxRenderedBytes) - renderer.walk(document, 0) + prepare(document, 0) - return renderer.result(), nil + rendered, err := newConverter().ConvertNode(document) + if err != nil { + return "", fmt.Errorf("converting HTML body: %w", err) + } + + return truncate(strings.TrimSpace(string(rendered))), nil } -type htmlRenderer struct { - markdown bool - - out bytes.Buffer - // budget is the output this renderer may still produce; truncated records - // that it ran out, or that the document went deeper than it will follow. - budget int - truncated bool - - // prefix opens every line of the current context: the quote markers of - // the blockquotes it sits in and the indentation of the list items. - prefix string - // marker replaces prefix on the next line only, which is where a list - // item's bullet or number goes. - marker string - - // pending holds markup that has been opened but not yet paid for. Email - // HTML is full of empty and wrappers, and writing their markers - // eagerly would scatter stray asterisks and brackets through the output, - // so a marker is only written once something follows it. - pending []pendingMarkup - markupSeq int - // activeMarkers are the inline markers already open around the cursor. A - // marker nested inside itself would close the outer span early. - activeMarkers []string - - // lastClosed and closedAt locate the inline marker most recently written, - // so a span opening immediately against it can be merged into it. - lastClosed string - closedAt int - - // pendingNewlines is the separation the next content owes what precedes - // it: one for a new line, two for a new block. Requests coalesce, so a - // paragraph closing inside a div does not open a hole. - pendingNewlines int - // breakPrefix is the prefix in force when the pending separation was - // asked for, which is not always the one in force when it is written: a - // blockquote opens and closes between the two. - breakPrefix string - // pendingSpace records collapsed whitespace that only becomes a space if - // another word follows it on the same line. - pendingSpace bool - - atLineStart bool - - // preformatted counts the
 elements in scope, where whitespace is
-	// content rather than layout.
-	preformatted int
-
-	listStack []listLevel
+// newConverter builds a converter per message, which is what the library's own
+// entry points do: a Converter carries the error of the conversion it is
+// running, and go-smtp serves every connection on its own goroutine.
+func newConverter() *converter.Converter {
+	return converter.NewConverter(
+		converter.WithPlugins(
+			base.NewBasePlugin(),
+			commonmark.NewCommonmarkPlugin(
+				// The default is "* * *", which reads as a stray line of
+				// asterisks anywhere the Markdown is not rendered.
+				commonmark.WithHorizontalRule("---"),
+			),
+			strikethrough.NewStrikethroughPlugin(),
+		),
+	)
 }
 
-type pendingMarkup struct {
-	seq    int
-	markup string
-	// mergeable marks an inline span that may be joined to the one it opens
-	// against, rather than closing and reopening the same emphasis.
-	mergeable bool
-}
+func prepare(node *html.Node, depth int) {
+	child := node.FirstChild
 
-type listLevel struct {
-	ordered bool
-	index   int
-}
+	for child != nil {
+		next := child.NextSibling
 
-func newHTMLRenderer(markdown bool, budget int) *htmlRenderer {
-	return &htmlRenderer{markdown: markdown, budget: budget, atLineStart: true}
-}
+		switch child.Type {
+		case html.CommentNode:
+			node.RemoveChild(child)
 
-func (r *htmlRenderer) result() string {
-	lines := strings.Split(r.out.String(), "\n")
-	for i := range lines {
-		lines[i] = strings.TrimRight(lines[i], " \t")
-	}
+		case html.TextNode:
+			child.Data = sanitizeText(child.Data)
 
-	rendered := strings.TrimSpace(strings.Join(lines, "\n"))
-	if r.truncated && rendered != "" {
-		rendered += "\n" + truncationMarker
-	}
-
-	return rendered
-}
-
-// write appends to the output, up to the budget. bytes.Buffer never fails, and
-// every caller has already decided the string belongs there.
-func (r *htmlRenderer) write(s string) {
-	if r.truncated {
-		return
-	}
-
-	if len(s) > r.budget {
-		// Dropped whole rather than clipped: a cut inside a multi-byte rune
-		// would put invalid UTF-8 into the payload the target is sent as.
-		r.truncated = true
-
-		return
-	}
-
-	r.budget -= len(s)
-	_, _ = r.out.WriteString(s)
-}
-
-func (r *htmlRenderer) unwrite(n int) {
-	r.out.Truncate(r.out.Len() - n)
-	r.budget += n
-}
-
-// content settles everything the output owes — line breaks, indentation, a
-// collapsed space, markup opened earlier — and then writes s.
-func (r *htmlRenderer) content(s string) {
-	if s == "" {
-		return
-	}
-
-	r.flushNewlines()
-	r.startLine()
-	r.flushSpace()
-	r.flushPending()
-	r.write(s)
-}
-
-// text writes message text, which is escaped so its punctuation cannot be
-// mistaken for the formatting this renderer emits itself.
-func (r *htmlRenderer) text(s string) {
-	if s == "" {
-		return
-	}
-
-	if r.markdown && r.preformatted == 0 {
-		s = markdownEscaper.Replace(s)
-		if r.startsLine() {
-			s = escapeLineStart(s)
-		}
-	}
-
-	r.content(s)
-}
-
-// startsLine reports whether the next content will open a line, where the
-// markers that only mean something there have to be defused.
-func (r *htmlRenderer) startsLine() bool {
-	return len(r.pending) == 0 && (r.atLineStart || r.pendingNewlines > 0)
-}
-
-func (r *htmlRenderer) flushNewlines() {
-	if r.pendingNewlines == 0 {
-		return
-	}
-
-	newlines := r.pendingNewlines
-	r.pendingNewlines = 0
-
-	// A truly blank line closes a blockquote or a list item. The separator
-	// carries the prefix the blocks on either side of it share, so it keeps
-	// the contexts they are both inside open and lets the ones only one of
-	// them is inside end.
-	separator := strings.TrimRight(commonPrefix(r.breakPrefix, r.prefix), " ")
-
-	r.write("\n")
-	for range newlines - 1 {
-		r.write(separator)
-		r.write("\n")
-	}
-
-	r.atLineStart = true
-	r.pendingSpace = false
-}
-
-func (r *htmlRenderer) startLine() {
-	if !r.atLineStart {
-		return
-	}
-
-	r.atLineStart = false
-	r.pendingSpace = false
-
-	if r.marker != "" {
-		r.write(r.marker)
-		r.marker = ""
-
-		return
-	}
-
-	r.write(r.prefix)
-}
-
-func (r *htmlRenderer) flushSpace() {
-	if !r.pendingSpace {
-		return
-	}
-
-	r.pendingSpace = false
-	if r.out.Len() > 0 {
-		r.write(" ")
-	}
-}
-
-func (r *htmlRenderer) flushPending() {
-	for _, opened := range r.pending {
-		if opened.mergeable && r.lastClosed == opened.markup && r.closedAt == r.out.Len() {
-			// Two spans of the same kind meeting with nothing between them are
-			// one span. Writing both markers instead leaves a run of four
-			// asterisks, which Markdown reads as text rather than as emphasis
-			// — and splitting a bolded label into exactly that shape is what
-			// every Word and Outlook export does.
-			r.unwrite(len(opened.markup))
-			r.lastClosed = ""
-
-			continue
-		}
-
-		r.write(opened.markup)
-	}
-
-	r.pending = r.pending[:0]
-}
-
-// open queues markup and returns a closer that reports whether any content
-// followed it. A closer that reports false has already withdrawn the markup.
-func (r *htmlRenderer) open(markup string, mergeable bool) func() bool {
-	r.markupSeq++
-	seq := r.markupSeq
-	r.pending = append(r.pending, pendingMarkup{seq: seq, markup: markup, mergeable: mergeable})
-
-	return func() bool {
-		// Keyed on the sequence number rather than on the position, which
-		// repeats every time the queue drains.
-		if last := len(r.pending) - 1; last >= 0 && r.pending[last].seq == seq {
-			r.pending = r.pending[:last]
-
-			return false
-		}
-
-		return true
-	}
-}
-
-// closeMarkup writes a marker straight out, so a trailing space stays outside
-// it where Markdown still recognises it as the end of the span.
-func (r *htmlRenderer) closeMarkup(markup string) {
-	r.write(markup)
-	r.lastClosed = markup
-	r.closedAt = r.out.Len()
-}
-
-func (r *htmlRenderer) breakLine()  { r.separate(1) }
-func (r *htmlRenderer) breakBlock() { r.separate(2) }
-
-func (r *htmlRenderer) separate(newlines int) {
-	if r.out.Len() == 0 {
-		// Nothing has been written, so there is nothing to separate from and
-		// the document would only gain a blank first line.
-		return
-	}
-
-	if r.pendingNewlines == 0 {
-		r.breakPrefix = r.prefix
-	}
-
-	switch {
-	case r.preformatted > 0:
-		// A blank line inside preformatted text is content, so breaks add up
-		// there rather than merging into one.
-		r.pendingNewlines += newlines
-	case newlines > r.pendingNewlines:
-		r.pendingNewlines = newlines
-	}
-
-	r.pendingSpace = false
-}
-
-func (r *htmlRenderer) walk(node *html.Node, depth int) {
-	if r.truncated {
-		return
-	}
-
-	if depth > maxHTMLDepth {
-		r.truncated = true
-
-		return
-	}
-
-	switch node.Type {
-	case html.TextNode:
-		r.writeTextNode(node.Data)
-	case html.DocumentNode:
-		r.walkChildren(node, depth)
-	case html.ElementNode:
-		r.walkElement(node, depth)
-	default:
-		// Comments, doctypes and raw nodes carry nothing a reader wants.
-	}
-}
-
-func (r *htmlRenderer) walkChildren(node *html.Node, depth int) {
-	for child := node.FirstChild; child != nil && !r.truncated; child = child.NextSibling {
-		r.walk(child, depth+1)
-	}
-}
-
-// renderPlain renders a subtree as text alone. A code span and a fenced block
-// hold no markup to escape and no markers to introduce, and a link's label has
-// to be known before it is written to decide whether its destination adds
-// anything to it.
-func (r *htmlRenderer) renderPlain(node *html.Node, depth int, preformatted bool) string {
-	sub := newHTMLRenderer(false, r.budget)
-	if preformatted {
-		sub.preformatted = 1
-	}
-
-	sub.walkChildren(node, depth)
-
-	return sub.result()
-}
-
-func (r *htmlRenderer) writeTextNode(data string) {
-	data = sanitizeText(data)
-	if data == "" {
-		return
-	}
-
-	if r.preformatted > 0 {
-		for i, line := range strings.Split(data, "\n") {
-			if i > 0 {
-				r.breakLine()
+		case html.ElementNode:
+			if unwrapped := prepareElement(node, child, depth); unwrapped != nil {
+				// The element was replaced by its own children, which have
+				// not been looked at yet.
+				next = unwrapped
 			}
-			r.text(line)
 		}
 
-		return
-	}
-
-	if strings.TrimSpace(data) == "" {
-		// Whitespace between elements still separates the words on either
-		// side of it.
-		r.pendingSpace = true
-
-		return
-	}
-
-	if strings.TrimLeftFunc(data, unicode.IsSpace) != data {
-		r.pendingSpace = true
-	}
-
-	for i, word := range strings.Fields(data) {
-		if i > 0 {
-			r.pendingSpace = true
-		}
-		r.text(word)
-	}
-
-	if strings.TrimRightFunc(data, unicode.IsSpace) != data {
-		r.pendingSpace = true
+		child = next
 	}
 }
 
-func (r *htmlRenderer) walkElement(node *html.Node, depth int) {
+func prepareElement(parent, node *html.Node, depth int) *html.Node {
 	if isHidden(node) {
-		return
+		// A template opens with a hidden block holding the preview line the
+		// inbox shows, which is written for the list view and reads as noise
+		// anywhere else.
+		parent.RemoveChild(node)
+
+		return nil
 	}
 
 	switch node.DataAtom {
-	case atom.Script, atom.Style, atom.Head, atom.Title, atom.Noscript,
-		atom.Template, atom.Iframe, atom.Object, atom.Svg, atom.Map:
-		return
-
-	case atom.Br:
-		if r.pendingNewlines > 0 {
-			// A run of 
is how mail asks for a blank line, and a browser - // gives it one; coalescing them all into a single break would - // glue the greeting to the paragraph under it. - r.breakBlock() - - break - } - - r.breakLine() - - case atom.Hr: - r.breakBlock() - r.content("---") - r.breakBlock() - - case atom.H1, atom.H2, atom.H3, atom.H4, atom.H5, atom.H6: - r.renderHeading(node, depth) - - case atom.Ul, atom.Ol: - r.renderList(node, depth) - - case atom.Li: - r.renderListItem(node, depth) - - case atom.Blockquote: - r.renderBlockquote(node, depth) - - case atom.Pre: - r.renderPreformatted(node, depth) + case atom.Img: + return prepareImage(parent, node) case atom.A: - r.renderLink(node, depth) + href := sanitizeURL(attrValue(node, "href")) + if !isUsableURL(href) { + // In-page anchors, script handlers and the message's own inline + // attachments give a notification's reader nothing to open, but + // the text of the link is still part of the message. + return unwrap(parent, node) + } - case atom.Img: - r.renderImage(node) + setAttr(node, "href", href) + prepare(node, depth) - case atom.B, atom.Strong: - r.renderInline("**", node, depth) + if !hasContent(node) { + // A link wrapped around a tracking pixel, or around the spacer + // that mail templates use for one, would render as "[](url)" — + // invisible to the reader. Its destination is the only thing left + // worth forwarding. + node.AppendChild(&html.Node{Type: html.TextNode, Data: href}) + } - case atom.I, atom.Em: - r.renderInline("*", node, depth) - - case atom.Del, atom.S, atom.Strike: - r.renderInline("~~", node, depth) - - case atom.Code, atom.Kbd, atom.Samp: - r.renderCode(node, depth) + return nil case atom.Td, atom.Th: // Mail is laid out in tables far more often than it tabulates - // anything, so a cell reads as another run of words rather than as a - // column that a Markdown table would have to line up. - r.walkChildren(node, depth) - r.pendingSpace = true + // anything, and a converter with no table rules runs the cells of a + // row together: "Total4Failed0". + parent.InsertBefore(&html.Node{Type: html.TextNode, Data: " "}, node) - default: - switch { - case blockElements[node.DataAtom]: - r.breakBlock() - r.walkChildren(node, depth) - r.breakBlock() - case lineElements[node.DataAtom]: - r.breakLine() - r.walkChildren(node, depth) - r.breakLine() - default: - r.walkChildren(node, depth) + case atom.Tr: + if hasElementSibling(node) { + parent.InsertBefore(&html.Node{ + Type: html.ElementNode, DataAtom: atom.Br, Data: "br", + }, node) } - } -} -func (r *htmlRenderer) renderHeading(node *html.Node, depth int) { - r.breakBlock() - - if r.markdown { - closeHeading := r.open(strings.Repeat("#", headingLevels[node.DataAtom])+" ", false) - r.walkChildren(node, depth) - closeHeading() - } else { - r.walkChildren(node, depth) - } - - r.breakBlock() -} - -func (r *htmlRenderer) renderList(node *html.Node, depth int) { - // A list nested in another one belongs to the item that holds it, so it - // only starts a new line; a blank one would make the whole outer list - // loose and double-space every item in it. - nested := len(r.listStack) > 0 - r.separateList(nested) - - r.listStack = append(r.listStack, listLevel{ - ordered: node.DataAtom == atom.Ol, - index: listStart(node), - }) - r.walkChildren(node, depth) - r.listStack = r.listStack[:len(r.listStack)-1] - - r.separateList(nested) -} - -func (r *htmlRenderer) separateList(nested bool) { - if nested { - r.breakLine() - - return - } - - r.breakBlock() -} - -func (r *htmlRenderer) renderListItem(node *html.Node, depth int) { - r.breakLine() - - bullet := "- " - if len(r.listStack) > 0 { - level := &r.listStack[len(r.listStack)-1] - if level.ordered { - bullet = strconv.Itoa(level.index) + ". " - level.index++ + case atom.Blockquote, atom.Ul, atom.Ol: + depth++ + if depth > maxNestingDepth { + return unwrap(parent, node) } } - outer := r.prefix - r.marker = outer + bullet - r.prefix = outer + strings.Repeat(" ", len(bullet)) + prepare(node, depth) - r.walkChildren(node, depth) - - r.breakLine() - r.prefix = outer - r.marker = "" + return nil } -func (r *htmlRenderer) renderBlockquote(node *html.Node, depth int) { - r.breakBlock() - - outer := r.prefix - r.prefix = outer + "> " - r.walkChildren(node, depth) - r.prefix = outer - - r.breakBlock() -} - -func (r *htmlRenderer) renderPreformatted(node *html.Node, depth int) { - if r.preformatted > 0 { - // Already inside preformatted text, where a second
 changes
-		// nothing — and rendering it separately would walk the subtree twice
-		// per level of nesting.
-		r.walkChildren(node, depth)
-
-		return
-	}
-
-	r.breakBlock()
-
-	code := r.renderPlain(node, depth, true)
-
-	fence := ""
-	if r.markdown {
-		fence = backtickFence(code, 3)
-		r.content(fence)
-		r.breakLine()
-	}
-
-	// Emitted inside the preformatted count so the blank lines a diff or a
-	// stack trace is shaped by survive: outside it, consecutive breaks are one
-	// break.
-	r.preformatted++
-	for i, line := range strings.Split(code, "\n") {
-		if i > 0 {
-			r.breakLine()
-		}
-		r.content(line)
-	}
-	r.preformatted--
-
-	if r.markdown {
-		r.breakLine()
-		r.content(fence)
-	}
-
-	r.breakBlock()
-}
-
-func (r *htmlRenderer) renderCode(node *html.Node, depth int) {
-	if !r.markdown || r.preformatted > 0 {
-		r.walkChildren(node, depth)
-
-		return
-	}
-
-	// Rendered from its text rather than written through, for the two things
-	// Markdown does differently inside a code span: backslash escapes are
-	// literal there, and the span ends at the first backtick run as long as
-	// the one that opened it.
-	code := strings.ReplaceAll(r.renderPlain(node, depth, false), "\n", " ")
-	if code == "" {
-		return
-	}
-
-	padding := ""
-	if strings.HasPrefix(code, "`") || strings.HasSuffix(code, "`") {
-		padding = " "
-	}
-
-	fence := backtickFence(code, 1)
-	r.content(fence + padding + code + padding + fence)
-}
-
-func (r *htmlRenderer) renderInline(marker string, node *html.Node, depth int) {
-	if !r.markdown || slices.Contains(r.activeMarkers, marker) {
-		// A marker nested inside itself closes the outer span at the inner
-		// one's start and leaves the surplus markers in the text.
-		r.walkChildren(node, depth)
-
-		return
-	}
-
-	r.activeMarkers = append(r.activeMarkers, marker)
-	closeInline := r.open(marker, true)
-	r.walkChildren(node, depth)
-
-	if closeInline() {
-		r.closeMarkup(marker)
-	}
-
-	r.activeMarkers = r.activeMarkers[:len(r.activeMarkers)-1]
-}
-
-func (r *htmlRenderer) renderLink(node *html.Node, depth int) {
-	href := sanitizeURL(attrValue(node, "href"))
-	if !isUsableURL(href) {
-		r.walkChildren(node, depth)
-
-		return
-	}
-
-	if r.markdown {
-		closeLink := r.open("[", false)
-		r.walkChildren(node, depth)
-
-		if closeLink() {
-			r.closeMarkup("](" + markdownURL(href) + ")")
-
-			return
-		}
-
-		// A link with nothing to click on is still worth forwarding.
-		r.text(href)
-
-		return
-	}
-
-	// Rendered once on its own first: a label that is already the destination
-	// does not want it repeated, and one that is empty wants it instead.
-	label := r.renderPlain(node, depth, false)
-	r.walkChildren(node, depth)
+// prepareImage keeps of an image only what a notification can use. Tracking
+// pixels, spacers and sliced-up banners carry no alt text and go entirely; an
+// inline attachment has no address the reader can fetch, so its alt text
+// stays behind as ordinary words.
+func prepareImage(parent, node *html.Node) *html.Node {
+	alt := collapseSpace(sanitizeText(attrValue(node, "alt")))
+	source := sanitizeURL(attrValue(node, "src"))
 
 	switch {
-	case label == "":
-		r.text(href)
-	case label != href:
-		// A bare label leaves a notification's reader with nothing to open.
-		r.write(" (" + href + ")")
+	case alt == "":
+		parent.RemoveChild(node)
+	case !isUsableURL(source):
+		parent.InsertBefore(&html.Node{Type: html.TextNode, Data: alt}, node)
+		parent.RemoveChild(node)
+	default:
+		setAttr(node, "alt", alt)
+		setAttr(node, "src", source)
 	}
+
+	return nil
 }
 
-func (r *htmlRenderer) renderImage(node *html.Node) {
-	alt := collapseSpace(sanitizeText(attrValue(node, "alt")))
-	if alt == "" {
-		// Tracking pixels, spacers and sliced-up banners have no alt text and
-		// nothing to say.
-		return
+// unwrap replaces a node with its own children and returns the first of them.
+func unwrap(parent, node *html.Node) *html.Node {
+	first := node.FirstChild
+
+	for child := node.FirstChild; child != nil; child = node.FirstChild {
+		node.RemoveChild(child)
+		parent.InsertBefore(child, node)
 	}
 
-	source := sanitizeURL(attrValue(node, "src"))
-	if r.markdown && isUsableURL(source) {
-		r.content("![" + markdownEscaper.Replace(alt) + "](" + markdownURL(source) + ")")
+	parent.RemoveChild(node)
 
-		return
-	}
-
-	r.text(alt)
+	return first
 }
 
-// blockElements are separated from their surroundings by a blank line.
-// lineElements only start a new line, which is how a browser renders the
-// 
-per-line that mail templates are built from. -var ( - blockElements = map[atom.Atom]bool{ - atom.Article: true, - atom.Dl: true, - atom.Fieldset: true, - atom.Form: true, - atom.P: true, - atom.Section: true, - atom.Table: true, - } - - lineElements = map[atom.Atom]bool{ - atom.Address: true, - atom.Aside: true, - atom.Caption: true, - atom.Center: true, - atom.Dd: true, - atom.Div: true, - atom.Dt: true, - atom.Figcaption: true, - atom.Figure: true, - atom.Footer: true, - atom.Header: true, - atom.Legend: true, - atom.Main: true, - atom.Nav: true, - atom.Tr: true, - } -) - -// The characters mail uses for layout rather than for words: padding that -// hides a preheader from the inbox preview, and the spaces that hold table -// cells apart without letting them wrap. -const ( - softHyphen = '\u00ad' - noBreakSpace = '\u00a0' - zeroWidthSpace = '\u200b' - zeroWidthNonJoiner = '\u200c' - zeroWidthJoiner = '\u200d' - narrowNoBreakSpace = '\u202f' - wordJoiner = '\u2060' - figureSpace = '\u2007' - byteOrderMark = '\ufeff' -) - -var headingLevels = map[atom.Atom]int{ - atom.H1: 1, - atom.H2: 2, - atom.H3: 3, - atom.H4: 4, - atom.H5: 5, - atom.H6: 6, -} - -// markdownEscaper defuses the punctuation that would otherwise turn message -// text into formatting. Underscores are left alone: CommonMark ignores them -// inside a word, which is where mail almost always puts them, and escaping -// every one turns file names and identifiers into noise. -var markdownEscaper = strings.NewReplacer( - `\`, `\\`, - "`", "\\`", - `*`, `\*`, - `[`, `\[`, - `]`, `\]`, - `<`, `\<`, -) - -// escapeLineStart defuses the markers that only mean something at the start of -// a line, so a message opening with "- " or "# " reads as the sender wrote it. -func escapeLineStart(s string) string { - if s == "" { - return s - } - - switch s[0] { - case '#', '>', '-', '+', '=', '|': - return `\` + s - } - - digits := 0 - for digits < len(s) && s[digits] >= '0' && s[digits] <= '9' { - digits++ - } - - if digits > 0 && digits < len(s) && (s[digits] == '.' || s[digits] == ')') { - return s[:digits] + `\` + s[digits:] - } - - return s -} - -// backtickFence returns a run of backticks longer than any run in the content, -// so the content cannot close the span or block that holds it and spill the -// rest of the message out as live Markdown. -func backtickFence(content string, minimum int) string { - longest, run := 0, 0 - - for _, char := range content { - if char != '`' { - run = 0 - - continue +// hasContent reports whether anything left under a node would show. It runs +// after the subtree has been prepared, so an element that survived that is one +// the reader will see. +func hasContent(node *html.Node) bool { + for child := node.FirstChild; child != nil; child = child.NextSibling { + switch child.Type { + case html.ElementNode: + if child.DataAtom == atom.Img || hasContent(child) { + return true + } + case html.TextNode: + if strings.TrimSpace(child.Data) != "" { + return true + } } - - run++ - longest = max(longest, run) } - return strings.Repeat("`", max(longest+1, minimum)) + return false } -// markdownURL fits a URL into a link destination. Spaces and parentheses would -// end the destination early, so those URLs take the angle bracket form. -func markdownURL(raw string) string { - if !strings.ContainsAny(raw, " ()<>") { - return raw +func hasElementSibling(node *html.Node) bool { + for sibling := node.PrevSibling; sibling != nil; sibling = sibling.PrevSibling { + if sibling.Type == html.ElementNode { + return true + } } - return "<" + strings.NewReplacer(" ", "%20", "<", "%3C", ">", "%3E").Replace(raw) + ">" + return false +} + +func truncate(rendered string) string { + if len(rendered) <= maxRenderedBytes { + return rendered + } + + cut := maxRenderedBytes + for cut > 0 && !utf8.RuneStart(rendered[cut]) { + cut-- + } + + return strings.TrimRight(rendered[:cut], " \t\n") + "\n" + truncationMarker } // isUsableURL rejects the destinations a notification's reader cannot act on: @@ -915,9 +325,22 @@ func sanitizeText(s string) string { }, s) } +// The characters mail uses for layout rather than for words: padding that +// hides a preheader from the inbox preview, and the spaces that hold table +// cells apart without letting them wrap. +const ( + softHyphen = '\u00ad' + noBreakSpace = '\u00a0' + zeroWidthSpace = '\u200b' + zeroWidthNonJoiner = '\u200c' + zeroWidthJoiner = '\u200d' + narrowNoBreakSpace = '\u202f' + wordJoiner = '\u2060' + figureSpace = '\u2007' + byteOrderMark = '\ufeff' +) + // isHidden reports whether a browser would leave the element out of the page. -// Mail templates open with a hidden block holding the preview line the inbox -// shows, which is written for the list view and reads as noise anywhere else. func isHidden(node *html.Node) bool { for _, attr := range node.Attr { switch attr.Key { @@ -954,18 +377,6 @@ func isHidingStyle(style string) bool { var spaceStripper = strings.NewReplacer(" ", "", "\t", "", "\n", "", "\r", "") -// commonPrefix returns the leading run the two prefixes agree on. -func commonPrefix(a, b string) string { - limit := min(len(a), len(b)) - - shared := 0 - for shared < limit && a[shared] == b[shared] { - shared++ - } - - return a[:shared] -} - func attrValue(node *html.Node, key string) string { for _, attr := range node.Attr { if attr.Key == key { @@ -976,11 +387,14 @@ func attrValue(node *html.Node, key string) string { return "" } -func listStart(node *html.Node) int { - start, err := strconv.Atoi(strings.TrimSpace(attrValue(node, "start"))) - if err != nil { - return 1 +func setAttr(node *html.Node, key, value string) { + for i := range node.Attr { + if node.Attr[i].Key == key { + node.Attr[i].Val = value + + return + } } - return start + node.Attr = append(node.Attr, html.Attribute{Key: key, Val: value}) } diff --git a/html_test.go b/html_test.go index c8a6a53..4bbdaf9 100644 --- a/html_test.go +++ b/html_test.go @@ -8,25 +8,24 @@ import ( "github.com/stretchr/testify/require" ) -func render(t *testing.T, format BodyFormat, source string) string { +func render(t *testing.T, source string) string { t.Helper() - out, err := renderHTML(source, format) + out, err := renderMarkdown(source) require.NoError(t, err) return out } -func TestRenderHTMLAsMarkdown(t *testing.T) { +// The conversion itself belongs to html-to-markdown; what these pin is the +// contract this package keeps on top of it, which is what a notification +// should carry rather than what the document said. +func TestRenderMarkdown(t *testing.T) { for name, tc := range map[string]struct{ source, want string }{ "paragraphs are separated by a blank line": { source: "

first

second

", want: "first\n\nsecond", }, - "divs are separated by a single line, as a browser lays them out": { - source: "
first
second
", - want: "first\nsecond", - }, "headings keep their level": { source: "

one

three

", want: "# one\n\n### three", @@ -35,30 +34,39 @@ func TestRenderHTMLAsMarkdown(t *testing.T) { source: "

bold italic gone x=1

", want: "**bold** *italic* ~~gone~~ `x=1`", }, - "a trailing space stays outside the emphasis it would otherwise break": { - source: "

bold after

", - want: "**bold** after", + "two spans of the same emphasis meeting head on become one": { + // What every Word and Outlook export does to a bolded label. + source: "

Total: 49.99

", + want: "**Total: 49.99**", }, - "empty inline wrappers leave no markers behind": { - source: "

text

", - want: "text", + "emphasis nested inside itself does not double its markers": { + source: "

ab

", + want: "**ab**", }, "links keep their destination": { source: `

see the docs

`, want: "see [the docs](https://example.com/x)", }, "a link with nothing to click on falls back to its destination": { - source: `

`, - want: "https://example.com/x", + // Otherwise a link wrapped around a tracking pixel renders as + // "[](url)", which the reader cannot see at all. + source: `

`, + want: "[https://example.com/x](https://example.com/x)", }, - "a destination with spaces or parentheses is bracketed": { - source: `x`, - want: "[x]()", + "a link around nothing but a spacer falls back the same way": { + source: `

`, + want: "[https://example.com/x](https://example.com/x)", }, "unusable destinations are dropped but their text is kept": { source: `up click img`, want: "up click img", }, + "line breaks inside a URL are removed rather than forwarded": { + // A newline in an href is invisible in the document and a + // fabricated line in the notification. + source: "open report", + want: "[open report](https://ok.example/rURGENT:%20wire%20funds)", + }, "unordered lists": { source: "
  • one
  • two
", want: "- one\n- two", @@ -67,41 +75,23 @@ func TestRenderHTMLAsMarkdown(t *testing.T) { source: `
  1. three
  2. four
`, want: "3. three\n4. four", }, - "nested lists stay tight and indented": { - source: "
  • outer
    • inner
", - want: "- outer\n - inner", - }, - "a wrapped list item is indented under its bullet": { - source: "
  • first line
    second line
", - want: "- first line\n second line", - }, - "blockquotes mark every line, including the blank ones": { - source: "

one

two

", - want: "> one\n>\n> two", - }, - "a blockquote does not leak its marker into what surrounds it": { + "blockquotes mark every line": { source: "

before

quoted

after

", want: "before\n\n> quoted\n\nafter", }, - "preformatted text is fenced and left alone": { - source: "
if (a < b) {\n  *x = 1;\n}
", - want: "```\nif (a < b) {\n *x = 1;\n}\n```", + "a code block is fenced past the backticks it holds": { + // Otherwise the sender closes the block and everything after it — + // a heading, a link — renders as live Markdown in a message the + // reader takes for a forwarded notification. + source: "
ok\n```\n## Your account is locked\n
", + want: "````\nok\n```\n## Your account is locked\n````", }, - "br starts a new line": { - source: "

one
two

", - want: "one\ntwo", - }, - "a run of br leaves a blank line, as a browser does": { - source: "

Hi,

the body

", - want: "Hi,\n\nthe body", - }, - "hr becomes a rule": { - source: "

a


b

", - want: "a\n\n---\n\nb", - }, - "table rows become lines and cells become words": { + "table rows become lines and cells stay apart": { + // Mail lays itself out in tables far more often than it tabulates + // anything, and a converter with no table rules would otherwise + // run a row together as "Total4Failed0". source: "
Total4
Failed0
", - want: "Total 4\nFailed 0", + want: "Total 4 \nFailed 0", }, "images without alt text are dropped": { source: `

ab

`, @@ -115,6 +105,10 @@ func TestRenderHTMLAsMarkdown(t *testing.T) { source: `Chart`, want: "Chart", }, + "alt text is collapsed onto one line": { + source: "\"a\n", + want: "![a long alt](https://x.example/i.png)", + }, "script and style content never reaches the reader": { source: "

body

", want: "body", @@ -123,9 +117,10 @@ func TestRenderHTMLAsMarkdown(t *testing.T) { source: `
inbox preview

real body

`, want: "real body", }, - "whitespace between elements collapses to a single space": { - source: "

one\n\t two three

", - want: "one two three", + "the other ways a preheader hides are recognised too": { + source: `
a
b
` + + `
c
d

body

`, + want: "d\n\nbody", }, "entities and non-breaking spaces become ordinary text": { source: "

4m 12s & counting — done

", @@ -136,171 +131,79 @@ func TestRenderHTMLAsMarkdown(t *testing.T) { want: "ab", }, "text that looks like markup is escaped": { - source: "

2 * 3, a [b] c, `tick`, back\\slash, <tag>

", - want: "2 \\* 3, a \\[b\\] c, \\`tick\\`, back\\\\slash, \\", + source: "

2 * 3, a [b] c, `tick`

", + want: "2 * 3, a \\[b] c, \\`tick\\`", }, "markers are only defused where they would take effect": { source: "

- not a bullet

# not a heading

1. not a list

a - b

", want: "\\- not a bullet\n\n\\# not a heading\n\n1\\. not a list\n\na - b", }, - "underscores are left alone, since a word is where mail puts them": { - source: "

see config_test.go

", - want: "see config_test.go", - }, - "two spans of the same emphasis meeting head on become one": { - // What every Word and Outlook export does to a bolded label. - source: "

Total: 49.99

", - want: "**Total: 49.99**", - }, - "emphasis nested inside itself does not double its markers": { - source: "

ab

", - want: "**ab**", - }, - "spans with a space between them stay apart": { - source: "

a b

", - want: "**a** **b**", - }, - "a code span is literal, so its text is not escaped": { - source: "

retry_after=30*60

", - want: "`retry_after=30*60`", - }, - "a code span is fenced past the backticks it holds": { - source: "

a`b

", - want: "``a`b``", - }, - "a code block is fenced past the backticks it holds": { - // Otherwise the sender closes the block and everything after it — - // a heading, a link — renders as live Markdown in a message the - // reader takes for a forwarded notification. - source: "
ok\n```\n## Your account is locked\n
", - want: "````\nok\n```\n## Your account is locked\n````", - }, - "a code block keeps the blank lines it is shaped by": { - source: "
ok\n\n\nnext
", - want: "```\nok\n\n\nnext\n```", - }, - "line breaks inside a URL are removed rather than forwarded": { - // A newline in an href is invisible in the document and a - // fabricated line in the notification. - source: "open report", - want: "[open report]()", - }, - "alt text is collapsed onto one line": { - source: "\"a\n", - want: "![a long alt](https://x.example/i.png)", - }, - "the other ways a preheader hides are recognised too": { - source: `
a
b
` + - `
c
d

body

`, - want: "d\n\nbody", - }, "a document with nothing to say renders to nothing": { source: ``, want: "", }, } { t.Run(name, func(t *testing.T) { - require.Equal(t, tc.want, render(t, FormatMarkdown, tc.source)) + require.Equal(t, tc.want, render(t, tc.source)) }) } } -func TestRenderHTMLAsText(t *testing.T) { - for name, tc := range map[string]struct{ source, want string }{ - "no emphasis markers are introduced": { - source: "

bold and italic and code

", - want: "bold and italic and code", - }, - "a link keeps its destination beside its label": { - source: `

see the docs

`, - want: "see the docs (https://example.com/x)", - }, - "a link whose label is already the destination is not repeated": { - source: `https://example.com/x`, - want: "https://example.com/x", - }, - "headings are plain lines": { - source: "

Title

body

", - want: "Title\n\nbody", - }, - "images keep their alt text without a destination": { - source: `Logo`, - want: "Logo", - }, - "preformatted text keeps its shape without a fence": { - source: "
a\n  b
", - want: "a\n b", - }, - "text that looks like markup is left as written": { - source: "

2 * 3 = 6 [see notes]

", - want: "2 * 3 = 6 [see notes]", - }, - "lists stay readable": { - source: "
  • one
  • two
", - want: "- one\n- two", - }, - "a bare URL is not repeated even when its line opens with a bullet": { - source: ``, - want: "- https://e.com/x", - }, - "line breaks inside a URL are removed rather than forwarded": { - source: "open report", - want: "open report (https://ok.example/rURGENT: wire funds)", - }, - } { - t.Run(name, func(t *testing.T) { - require.Equal(t, tc.want, render(t, FormatText, tc.source)) - }) - } -} - -// The parser recovers from anything, so the renderer has to as well: a +// The parser recovers from anything, so the conversion has to as well: a // notification is worth more than a report that the markup was invalid. -func TestRenderHTMLSurvivesBrokenMarkup(t *testing.T) { +func TestRenderMarkdownSurvivesBrokenMarkup(t *testing.T) { for name, tc := range map[string]struct{ source, want string }{ "unclosed tags": {"

boldboth

next", "**bold*both***\n\n***next***"}, "stray closing tags": {"

text", "text"}, "no markup at all": {"just some words", "just some words"}, "an empty document": {"", ""}, - "an unterminated tag": {"

text<", "text\\<"}, + "an unterminated tag": {"

text<", "text<"}, } { t.Run(name, func(t *testing.T) { - require.Equal(t, tc.want, render(t, FormatMarkdown, tc.source)) + require.Equal(t, tc.want, render(t, tc.source)) }) } } -// Regression: block prefixes are re-emitted on every line, so nesting -// multiplies against line count. One message at the server's own 1 MB limit -// rendered to 125 MB and took the process down with it — the same class of -// fault as the remote DoS closed in 7565618. -func TestRenderHTMLCapsItsOutput(t *testing.T) { - source := strings.Repeat("

", 250) + strings.Repeat("

x

", 131000) +// Regression: a line prefix is re-emitted on every line of every level it +// nests, so depth multiplies against line count. One message at the server's +// own 1 MB limit rendered to 131 MB over two and a quarter minutes — the same +// class of fault as the remote DoS closed in 7565618. Neither the converter +// nor any of the alternatives bounds this on its own. +func TestRenderMarkdownBoundsPathologicalInput(t *testing.T) { + // Sized to overrun the cap several times over once the nesting is + // flattened, rather than to reproduce the original 1 MB message: the + // assertion is the same and the suite stays quick. + lines := strings.Repeat("

x

", maxRenderedBytes/maxNestingDepth) - out, err := renderHTML(source, FormatMarkdown) - require.NoError(t, err) - require.LessOrEqual(t, len(out), maxRenderedBytes+len(truncationMarker)+1) - require.True(t, strings.HasSuffix(out, truncationMarker), - "a capped message says so rather than ending mid-sentence") - require.True(t, utf8.ValidString(out), "a cap must not fall inside a rune") + for name, source := range map[string]string{ + "quotes nested far past anything real": strings.Repeat("
", 250) + lines, + "lists nested far past anything real": strings.Repeat("
  • ", 250) + lines, + } { + t.Run(name, func(t *testing.T) { + out, err := renderMarkdown(source) + require.NoError(t, err) + require.LessOrEqual(t, len(out), maxRenderedBytes+len(truncationMarker)+1) + require.True(t, strings.HasSuffix(out, truncationMarker), + "a capped message says so rather than ending mid-sentence") + require.True(t, utf8.ValidString(out), "the cap must not fall inside a rune") + }) + } } -// The renderer's own depth guard sits above the parser's limit on open -// elements, so the parser refuses a document first and email.go forwards it -// unchanged. Content is never dropped silently in between. -func TestRenderHTMLDepthGuardSitsAboveTheParsersOwn(t *testing.T) { - require.Greater(t, maxHTMLDepth, 512) +// Nesting under the limit is left alone, so an ordinary quoted reply still +// reads as one. +func TestRenderMarkdownKeepsNestingUnderTheLimit(t *testing.T) { + source := strings.Repeat("
    ", maxNestingDepth) + "

    deep

    " + + strings.Repeat("
    ", maxNestingDepth) - depth := 600 - _, err := renderHTML(strings.Repeat("
    ", depth)+"deep"+strings.Repeat("
    ", depth), - FormatMarkdown) - require.Error(t, err) + require.Equal(t, strings.Repeat("> ", maxNestingDepth)+"deep", render(t, source)) } -// A realistic transactional message, pinned end to end. Every finding this -// renderer has had lived in a combination of features rather than in one of +// A realistic transactional message, pinned end to end. The faults this +// feature has had lived in combinations of features rather than in one of // them, which is what a fixture catches and a unit test does not. -func TestRenderHTMLOnARealisticMessage(t *testing.T) { +func TestRenderMarkdownOnARealisticMessage(t *testing.T) { require.Equal(t, strings.Join([]string{ "[![Example Billing](https://cdn.example.com/logo.png)](https://billing.example.com)", "", @@ -308,40 +211,19 @@ func TestRenderHTMLOnARealisticMessage(t *testing.T) { "", "Hi Ferran,", "", - "Your invoice for **February 2026** **(30 days)** is ready. The total is " + + "Your invoice for **February 2026 (30 days)** is ready. The total is " + "**€42.00**, charged to the card ending 4242 on *1 March*.", "", - "Description Qty Amount", - "Hosting — small 1 €30.00", + "Description Qty Amount ", + "Hosting — small 1 €30.00 ", "Backups 2 €12.00", "", - "Reference: `inv_2026*0231`", + "Reference: `inv_2026*0231` ", "[View invoice](https://billing.example.com/invoices/2026-0231?utm_source=email&utm_medium=cta)", "", "You are receiving this because you have an account. " + "[Unsubscribe](https://billing.example.com/unsubscribe?t=abc) · Back to top", - }, "\n"), render(t, FormatMarkdown, marketingMessage)) - - require.Equal(t, strings.Join([]string{ - "Example Billing (https://billing.example.com)", - "", - "Invoice #2026-0231", - "", - "Hi Ferran,", - "", - "Your invoice for February 2026 (30 days) is ready. The total is €42.00, " + - "charged to the card ending 4242 on 1 March.", - "", - "Description Qty Amount", - "Hosting — small 1 €30.00", - "Backups 2 €12.00", - "", - "Reference: inv_2026*0231", - "View invoice (https://billing.example.com/invoices/2026-0231?utm_source=email&utm_medium=cta)", - "", - "You are receiving this because you have an account. " + - "Unsubscribe (https://billing.example.com/unsubscribe?t=abc) · Back to top", - }, "\n"), render(t, FormatText, marketingMessage)) + }, "\n"), render(t, marketingMessage)) } // Shaped like the transactional mail this feature exists for: a hidden -- 2.52.0