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
2026-09-10
refactor: convert HTML with html-to-markdown instead of by hand (FMG-9)
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>
fix: address review of the HTML conversion feature (FMG-9)
All checks were successful
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>
2026-09-09
feat: add a per-recipient Format option to convert HTML bodies (FMG-9)
All checks were successful
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>