Magento 2 : What's the best practice when it comes to escaping in templates?

Question

This is a question about best practices when it comes to writing templates / block classes and security.

When it comes to writing templates I know you need to escape everything that comes into the template. Just for security's sake. But most of the time the data you're fetching comes from dedicated block classes that are (most of the times) already tightly coupled with the phtml files.

For example: why would you do this:

// block class code:

public function getTitle():string
{
    return $this->getSomeEntity()->getTitle();
}

// template code

<h1><?= $block->escapeHtml($block->getTitle()) ?></h1>

... if you can also do this?:

// block class code:

public function getTitle():string
{
    return $this->escapeHtml($this->getSomeEntity()->getTitle());
}

// template code

<h1><?= $block->getTitle() ?></h1>

Or when it comes to text/x-magento-init-statements, you would even have something like this:

"title": "<?= $block->escapeJs($block->escapeHtml($block->getTitle())) ?>"

The above pattern you can see all through Magento, but I am wondering: why? I like to keep my templates as dumb as possible, and I don't want my template to be 100% responsible about the security of my theme. At any given point! It's also not DRY, which means that if getTitle() is called anywhere else, the escaping needs to be done again. And every time the author needs to get the order right so that the escaping happens in the right sequence.

The methods in blocks are written and meant to be used in templates. So in my opinion it would make much more sense to have a block have methods like:

public function getTitle():string
{
    return $this->escapeHtml($this->getSomeEntity()->getTitle());
}

public function getJsTitle():string
{
    return $this->escapeJs($this->getTitle());
}

Now the template that uses this block class is much more readable and whenever you use getTitle() in your template you can be sure that it's escaped on the proper way.

Now I am wondering what are the community thoughts on this matter? I know that if you want to learn Magento you should look at how Magento does it themselves, but this just seems a little bit off to me.

Answer 1

TL;DR ->escapeHtml() or ->escapeJs() belong in the PHTML template and not in the block class.

---

I think you would have to look at what the responsibilities are of 1. the block class and 2. the PHTML template. And specifically the separation of responsibilities.

Block Class

The block class exists to provide the template (or the people that work with the template) with additional processed data. Better said, it is to not have business logic (if-else, switch) or data processing (looping over arrays, doing something, creating a data set out/via of a collection) inside the template.

Also the block class does not know or does not need to know where or how this data will be used, so cannot know how to escape the data (HTML escape, JS escape, etc...)

Template

The responsibility of the template is to display the data visually. In a PHTML template, this is done in HTML (or JS) with PHP echo statements. The responsibility of the template is to display the data correctly and do anything that is necessary to do so for the HTML (or JS) specification.

Security

Although security is something that plays most in logic and detection in your back end and PHP classes, but as said to display data properly, one needs to escape certain characters inside the data in the PHTML template. Displaying a product title that contains a <script /> tag because of a XSS attack correctly as &ltscript /> is important for security but also for correct formatting, just like any other special characters that can be in a title.

HTML?

Escaping data through ->escapeHtml() or ->escapeJs() is specific for HTML or JS in Magento's PHTML templates, but what if:

• You have replaced Magento's native template engine for Twig (that automatically escapes, unless | raw filter is used)
• You are not rendering HTML, but XML+XSLT, or PDF? The Block doesn't (need to) know
• ...