Allow a format with a custom encoder to opt into HTML escaping in `.eex` templates

[timfjord]

Problem

Phoenix.Template.EExEngine picks the escaping engine only when the format's encoder is literally Phoenix.HTML.Engine:

case Phoenix.Template.format_encoder(format) do
  Phoenix.HTML.Engine -> [engine: Phoenix.HTML.Engine, trim: trim]
  _ -> [engine: EEx.SmartEngine]
end

So the encoder setting answers two unrelated questions at once: how to post-process the rendered output, and whether to escape interpolated values at compile time. Any format that needs its own encoder silently loses escaping, even when its templates are HTML-like markup.

We hit this with MJML emails rendered through Phoenix.Swoosh:

config :phoenix, :format_encoders, mjml: MyApp.Mjml  # compiles MJML to HTML

Every <%= %> in *.mjml.eex was interpolated as is. User input reached the final HTML unescaped, and input containing markup such as <b> broke MJML compilation. Nothing warns about this, and Phoenix.HTML.raw/1 does not work in such templates either, since EEx.SmartEngine cannot render {:safe, _}.

Current workaround

Register a custom engine for .eex that compiles mjml templates with Phoenix.HTML.Engine and delegates everything else to Phoenix.Template.EExEngine, and make the encoder accept {:safe, iodata}. It works, but it overrides the engine for every .eex template in the project to change one format.

Possible fixes

I see three options and would be happy to send a PR for whichever you prefer.

1. A list of HTML-safe formats in config

config :phoenix_template, :html_formats, [:mjml]

EExEngine uses Phoenix.HTML.Engine when the encoder is Phoenix.HTML.Engine or the format is in the list. Smallest change, nothing existing is touched. The downside is that the encoder and the flag live in two separate keys that must be kept in sync.

2. An optional callback on the encoder

defmodule MyApp.Mjml do
  def html_safe?, do: true
  def encode_to_iodata!({:safe, mjml}), do: ...
end

I prototyped this one. It reads nicely, but compile-time behaviour now depends on a user module. The engine has to Code.ensure_compiled/1 the encoder, and compile_all/4 has to record encoders as compile-time dependencies so templates recompile when the callback changes. That adds compile-time dependencies to every module with templates, including in apps that never opt in. I don't think it is worth it.

3. Encoder options in the existing config

config :phoenix_template, :format_encoders,
  mjml: {MyApp.Mjml, html_safe: true},
  csv: MyApp.CSV

A bare module keeps working and means {module, []}. Normalization happens inside compiled_format_encoders/0, and format_encoder/1 keeps returning the module, so phoenix, phoenix_view and other callers are unaffected. EExEngine reads the options through a new function. Like option 1 it is plain config, so there are no module dependencies and a config change already recompiles the project.

I like option 3 most: the flag sits next to the encoder it describes, it cannot be set for a format without an encoder, and it leaves room for other per-format options. But it does change the accepted shape of a well-known option, so I wanted to ask before sending anything.

In all three cases templates of an opted-in format would behave like .html.eex: values are escaped unless marked safe, and the encoder receives {:safe, iodata}.

Would you accept a PR for one of these? Or is a custom engine the intended way to handle this?

[josevalim]

Let’s go with option 1. The reason I prefer it is that enabling the html engine likely (or might) do more than just HTML safety. PRs welcome!