Faire / mjml-react

React component library to generate the HTML emails on the fly
https://www.npmjs.com/package/@faire/mjml-react
MIT License
387 stars 17 forks source link

`<MjmlRaw>` escapes HTML characters #15

Closed nahtnam closed 2 years ago

nahtnam commented 2 years ago

Hello,

I'm trying to add some custom templating from an external backend library (like mustache) and I'm noticing that certain characters are being escaped.

For example:

<MjmlRaw>{`%{ forall x <- xs }`}</MjmlRaw>

gets converted to

%{ forall x &lt;- xs }

Dropping this in the MJML live editor doesn't have the same problem: https://mjml.io/try-it-live/JFNNq1k86 (Although it does trip up the syntax highlighting with the <-)

Reproduced example with mjml-react: https://codesandbox.io/s/magical-browser-zt96bj?file=/src/index.tsx

I tried to spend some time figuring out what portion of the library caused the escaping but couldn't really figure it out. Our temporary solution might be to just unescape the whole file (or anything within {})

Any pointers would be appreciated, thanks!

nahtnam commented 2 years ago

Well, I was able to find this comment and it worked! https://github.com/wix-incubator/mjml-react/issues/41#issuecomment-713028786

However, I guess that brings up the question of, should we update MjmlRaw to work like the above code?

IanEdington commented 2 years ago

Thanks for reporting this issue @nahtnam. What templating language do you use? Also sorry for the delay in responding to you, I was out of office last week.

The root issue is that React prevents injection attacks by escaping these characters.

To get around this you need to use (dangerouslySetInnerHtml)[https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml] in some way.

Also the reason dangerouslySetInnerHtml doesn't work on MjmlRaw is because it gets converted to dangerously-set-inner-html here. We should probably add it to some components. I'll make a ticket to do this.

We use Mustache which didn't have the same problem but we wrapped all our Mustache tags in components like below. For you this would have be benefit of not needing to dangerouslySetInnerHtml all over the codebase. You will still need to tackle the problem of when to use MjmlRaw vs React.Fragment, and add the dangerouslySetInnerHtml to these components.

If you build something like this I'd love to include it in the utils section!

Let me know what you come up with or if you run into other issues

/**
 * This module provides mustache tags in an interface that is easy to use and hard to break.
 *
 * MTag   = {{tagName}}
 * MTrue  = {{# tagName}} children {{/ tagName}}
 * MFalse = {{^ tagName}} children {{/ tagName}}
 */

import { MjmlRaw } from "@faire/mjml-react";
import React from "react";

export const MTag: React.FC<{
  name: string;
  dangerouslyUnescaped?: boolean;
}> = ({ name, dangerouslyUnescaped, children }) => {
  if (children) {
    throw new Error("MTag cannot contain children");
  }
  assertTagNameIsValid(name);

  const mustacheTag = dangerouslyUnescaped ? `{{{${name}}}}` : `{{${name}}}`;
  return <MjmlRaw>{mustacheTag}</MjmlRaw>;
};

/**
 * for conditional using # / or ^ / please see MTrue or MFalse
 * we explicitly don't want to support partials and `&` for unescape a variable
 */
const invalidTagNameRegex = /^ *[#^/>&]/;

function assertTagNameIsValid(tagName: string): undefined | void {
  if (invalidTagNameRegex.test(tagName)) {
    throw Error(
      `Your tag name {{ ${tagName} }} doesn't fit nicely into our mold. Please use one of the custom mustache components or <MjRaw>{{${tagName}}}</MjRaw>`
    );
  }
  if (tagName.includes(" ")) {
    throw Error(
      "you probably don't want a space in your tag name. Mustache doesn't play nicely with extra spaces."
    );
  }
}

function MConditionalFactory(
  tagPrefix: "#" | "^"
): React.FC<{
  name: string;
  raw?: boolean;
}> {
  return ({ name, raw = false, children }) => {
    if (!children) {
      throw new Error("MTrue requires children");
    }

    assertTagNameIsValid(name);

    const openTag = `{{${tagPrefix} ${name}}}`;
    const closeTag = `{{/ ${name}}}`;
    return raw ? (
      <>
        <MjmlRaw>{openTag}</MjmlRaw>
        {children}
        <MjmlRaw>{closeTag}</MjmlRaw>
      </>
    ) : (
      <>
        {openTag}
        {children}
        {closeTag}
      </>
    );
  };
}

export const MTrue = MConditionalFactory("#");

export const MFalse = MConditionalFactory("^");
nahtnam commented 2 years ago

Hey, super sorry for the late response, was on PTO!

Here is what we ended with:

import React, {PropsWithChildren} from 'react'

export function Raw(props: PropsWithChildren<unknown>) {
  const {children} = props
  return React.createElement('mj-raw', {
    dangerouslySetInnerHTML: {
      __html: children,
    },
  })
}

Seemingly works well and pretty simple

emmclaughlin commented 2 years ago

@nahtnam glad you have a working solution! We do have a PR up to expose dangerouslySetInnerHTML on all ending tags in the V3 release as well: https://github.com/Faire/mjml-react/pull/42#issue-1421350357. Feel free to leave us any feedback you have on the approach

emmclaughlin commented 2 years ago

Closing as there is currently a workaround and we have a plan to support this in V3 (see #42)