A code block in react-email was rendering HTML instead of showing it

I've been looking for small React bugs to fix in open source projects, and this week I sent one to react-email. The change is only a few lines, but the reason for it is a trap I'd like to remember.

The issue was short and had a proper repro, which is always a good sign. The <Markdown> component takes a string of markdown and turns it into HTML for an email. Give it a code block with HTML inside:

```html
<b>bold</b> & <i>x</i>
```

Inline: `<br>`

and you'd expect to see those tags as text, because showing the snippet is the whole point of a code block. What actually came out was this (inline styles stripped):

<pre><code><b>bold</b> & <i>x</i>
</code></pre>
<p>Inline: <code><br></code></p>

The tags went into the email as real tags. So a tutorial explaining <b> shows you bold text, and the <br> in that sentence is just a line break. Nothing crashes and nothing logs an error. The code sample quietly goes missing.

Why it happened

react-email uses marked to parse the markdown, and marked's own renderer does escape code. This is the relevant line from the code method in its current source:

(escaped ? code : escapeHtmlEntities(code, true))

The catch is that email needs inline styles, so react-email replaces marked's renderers with its own, one per element, each adding a style attribute. Its code and codespan renderers looked like this, shortened:

renderer.code = ({ text }) => {
  text = `${text.replace(/\n$/, '')}\n`;
  return `<pre style="..."><code>${text}</code></pre>\n`;
};

Overriding a renderer replaces all of it, including the part you weren't thinking about. The styling came across and the escaping didn't. It's an easy thing to miss, since every example you test with during development is probably plain code with no angle brackets in it.

The fix

marked doesn't export its escape helper, so I added a small one next to the existing markdown utils, escaping the same five characters:

const htmlEscapes: Record<string, string> = {
  '&': '&amp;',
  '<': '&lt;',
  '>': '&gt;',
  '"': '&quot;',
  "'": '&#39;',
};

export const escapeHtml = (text: string) =>
  text.replace(/[&<>"']/g, (char) => htmlEscapes[char] ?? char);

It's one regex with a lookup rather than a chain of .replace() calls, which also means & doesn't need to go first. In a chain, escaping < and then & would turn < into <.

Then the two renderers use it:

renderer.code = ({ text, escaped }) => {
  // ...
  return `...<code>${escaped ? text : escapeHtml(text)}</code></pre>\n`;
};

renderer.codespan = ({ text }) => {
  return `...>${escapeHtml(text)}</code>`;
};

That escaped flag is the bit I nearly didn't think about. A marked extension, such as a syntax highlighter, can escape the code itself and set the flag on the token. Escaping everything regardless would escape that output a second time, and readers would see a literal < in their code sample. So I kept the same check marked uses.

The test puts HTML in both a fenced block and inline code, then checks both sides: the escaped text has to be there, and the raw tags have to be gone.

expect(actualOutput).toContain('&lt;br&gt;</code>');
expect(actualOutput).not.toContain('<br>');

It fails on canary and passes with the change.

What this doesn't do

It's tempting to call this a security fix, and it isn't one. It only covers code. marked passes raw HTML in the rest of the markdown straight through, and I didn't change that, so this still ends up in your email as a real tag:

Some text with <b>real HTML</b> in it.

That's normal for markdown, but it means <Markdown> isn't a sanitiser. If the string comes from your users, you still need to clean it before it gets there.

I also left the existing TODO: Support all options alone, so the code block still ignores its language. It would have been easy to tidy that up in the same PR, but a maintainer reviewing a change from a stranger wants one thing to look at.

The PR is open and waiting for review, so the code may change before it lands. The thing I'm taking from it applies well beyond this repo, though: when you override a default, read what the default was doing first. The part you're replacing is rarely the only thing it does.

First published on my blog. Cover photo: Bhullargraphic, CC BY-SA 4.0, via Wikimedia Commons (source).