feat: add a per-recipient Format option to convert HTML bodies (FMG-9) #13

Open

butterrobot wants to merge 3 commits from agent/full-stack-developer/7fb9f472e93e into main

pull from: agent/full-stack-developer/7fb9f472e93e

merge into: fmartingr:main

fmartingr:main

fmartingr:agent/full-stack-developer/92dd9c7e35da

Conversation 0 Commits 3 Files changed 12 +1773 -77

butterrobot commented 2026-09-09 22:59:11 +02:00

Collaborator

Copy link

Closes FMG-9

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 Markdown before it is forwarded:

[[Recipients]]
Addresses = ["me@example.com"]
Targets = ["mattermost://token@mattermost.example.com/"]
Format = "markdown"   # raw (default) | markdown

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. An unknown Format is refused at startup rather than silently forwarding raw HTML.

There is deliberately no plain-text format beside markdown: Markdown reads as plain text wherever nothing renders it, so a second conversion would only be a worse copy of the first.

The conversion

The Markdown is produced by html-to-markdown, which knows CommonMark — escaping, delimiter runs, fencing a code block past the backticks inside it. Cost: two modules (html-to-markdown/v2 and JohannesKaufmann/dom) on top of the golang.org/x/net this already used.

What it does not know is mail, because it is written for documents. The parsed message is prepared before conversion — hidden preheaders removed, alt-less images dropped (with the tracking pixels and spacers), unusable destinations unwrapped while keeping the link text, tabs and line breaks stripped from URLs, table rows turned into lines with their cells kept apart, and a link left holding nothing but a pixel falls back to its own destination instead of rendering as an invisible [](url).

The DoS is ours either way. A line prefix is re-emitted per line and per level, so nesting depth multiplies against line count. html-to-markdown amplifies exactly as a hand-written renderer does. Measured on one message at the server's own 1 MB limit, nested 250 deep:

	output	time	peak heap
without flattening	131 MB	2 m 16 s	~1 GiB
with flattening + cap	64 KiB	1.5 s	85 MiB

So quote and list nesting is flattened past six levels and the output is capped at 64 KiB with a ….

Reading the body correctly

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.

Body selection now follows RFC 2046 per container: the parts of a multipart/alternative are one content in several forms, the parts of any other multipart are cumulative. Attachments are skipped, an empty alternative no longer wins over the HTML the sender actually wrote, and a decode that fails degrades to the bytes as they arrived rather than bouncing mail that was being delivered before.

What a real message looks like

[![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

[View invoice](https://billing.example.com/invoices/2026-0231?utm_source=email&utm_medium=cta)

Checks

make format, make ci-lint, make test (93.6% coverage), make check and make build all pass locally.

🤖 Generated with Claude Code

Closes FMG-9 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 Markdown before it is forwarded: ```toml [[Recipients]] Addresses = ["me@example.com"] Targets = ["mattermost://token@mattermost.example.com/"] Format = "markdown" # raw (default) | markdown ``` 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. An unknown `Format` is refused at startup rather than silently forwarding raw HTML. There is deliberately no plain-text format beside `markdown`: Markdown reads as plain text wherever nothing renders it, so a second conversion would only be a worse copy of the first. ### The conversion The Markdown 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. Cost: **two modules** (`html-to-markdown/v2` and `JohannesKaufmann/dom`) on top of the `golang.org/x/net` this already used. What it does not know is mail, because it is written for documents. The parsed message is prepared before conversion — hidden preheaders removed, alt-less images dropped (with the tracking pixels and spacers), unusable destinations unwrapped while keeping the link text, tabs and line breaks stripped from URLs, table rows turned into lines with their cells kept apart, and a link left holding nothing but a pixel falls back to its own destination instead of rendering as an invisible `[](url)`. **The DoS is ours either way.** A line prefix is re-emitted per line and per level, so nesting depth multiplies against line count. `html-to-markdown` amplifies exactly as a hand-written renderer does. Measured on one message at the server's own 1 MB limit, nested 250 deep: | | output | time | peak heap | | --- | --- | --- | --- | | without flattening | 131 MB | 2 m 16 s | ~1 GiB | | with flattening + cap | 64 KiB | 1.5 s | 85 MiB | So quote and list nesting is flattened past six levels and the output is capped at 64 KiB with a `…`. ### Reading the body correctly 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. Body selection now follows RFC 2046 per container: the parts of a `multipart/alternative` are one content in several forms, the parts of any other multipart are cumulative. Attachments are skipped, an empty alternative no longer wins over the HTML the sender actually wrote, and a decode that fails degrades to the bytes as they arrived rather than bouncing mail that was being delivered before. ### What a real message looks like ``` [](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 [View invoice](https://billing.example.com/invoices/2026-0231?utm_source=email&utm_medium=cta) ``` ### Checks `make format`, `make ci-lint`, `make test` (93.6% coverage), `make check` and `make build` all pass locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

butterrobot added 1 commit 2026-09-09 22:59:11 +02:00

feat: add a per-recipient Format option to convert HTML bodies (FMG-9) ... d2fc783c6e

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) <noreply@anthropic.com>

butterrobot added 1 commit 2026-09-10 09:28:46 +02:00

fix: address review of the HTML conversion feature (FMG-9) ... c5942c3d8f

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 <b> 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 <pre>, 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) <noreply@anthropic.com>

butterrobot added 1 commit 2026-09-10 11:09:47 +02:00

refactor: convert HTML with html-to-markdown instead of by hand (FMG-9) ... 06de333118

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) <noreply@anthropic.com>

All checks were successful

Hide checks

CI / goreleaser-lint (pull_request) Successful in 3s

Details

CI / format (pull_request) Successful in 33s

Details

CI / test (pull_request) Successful in 2m21s

Details

CI / lint (pull_request) Successful in 3m6s

Details

CI / build (pull_request) Successful in 3m13s

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin agent/full-stack-developer/7fb9f472e93e:agent/full-stack-developer/7fb9f472e93e

git switch agent/full-stack-developer/7fb9f472e93e

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff agent/full-stack-developer/7fb9f472e93e

git switch agent/full-stack-developer/7fb9f472e93e

git rebase main

git switch main

git merge --ff-only agent/full-stack-developer/7fb9f472e93e

git switch agent/full-stack-developer/7fb9f472e93e

git rebase main

git switch main

git merge --no-ff agent/full-stack-developer/7fb9f472e93e

git switch main

git merge --squash agent/full-stack-developer/7fb9f472e93e

git switch main

git merge --ff-only agent/full-stack-developer/7fb9f472e93e

git switch main

git merge agent/full-stack-developer/7fb9f472e93e

git push origin main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

No labels

Compat/Breaking

Kind/Bug

Kind/Documentation

Kind/Enhancement

Kind/Feature

Kind/Security

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Status

Abandoned

Status

Blocked

Status

Need More Info

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

fmartingr/smtp2shoutrrr!13

No description provided.