1
<h1 class="page-title"><?php esc_html_e( 'Oops! Something went wrong.', '_amnth' ); ?></h1>

I think this should simply use the __() function, since it's a static string and it cannot be dynamic and in contrast, HTML.

At the same time, here's some source code from WooCommerce itself:

<th class="product-name"><?php esc_html_e( 'Product', 'woocommerce' ); ?></th>

So, is it whenever you output a string, dynamic or not, to HTML, you should also escape it with esc_html and so, even if it's just a string being output, you should use esc_html_e?

Please assume extremely strict escaping rules, where everything needs to be escaped in the right way. (Even so, using esc_html_e seems overkill to the power of 10 ).

3

You should always escape output. end of sentence, and end of story.

The questions is like asking if you can use globals and goto. Sure they are part of the language and in some edge cases they are the only way to produce working and readable code, but the guide is to never use them unless you can prove you have too.

Going down from the theoretical to the practical, it is not true that __ is static string, as if it was static you would not have used it at all, and just used the literal string instead. It is even worse, as the function is meant to supply an easy way to override a string to any other string, a string you might have no idea about its content.

The only place where escaping might result in a bad result on the front end is when someone tries to add html tags to the string as part of the translation, but this is extremely rare case, especially if you had the translators in mind while creating code that does the output.

Following the discussion in the comments, I feel like maybe I should state the reasoning in a different way. Translations should be treated as user input, since users can actually edit them, or use plugin to change them. And all user inputs should be suspect, if not for security reasons than for broken html reasons, and therefor it should be validated, sanitized and escaped.

With a more traditional user input you sometimes can avoid escaping by doing a more rigorous validation and sanitation, but you have no chance of doing that with regard to translation, and escaping is your almost only tool to be able to protect against security holes, and broken html.

  • I don't agree that it's a rare case. Just take a look at WP or TwentyX themes - translating formatted strings created with sprintf is pretty common practice... (translate.wordpress.org/projects/wp/dev/pl/…) – Krzysiek Dróżdż Jun 3 '18 at 7:29
  • 1
    @KrzysiekDróżdż I used to be in the opinion implied by the OP, but I was converted ;) wordpress core code is wrong. You start to ask yourself real questions about the need to "secure" the translations when you start to outsource it for your themes and plugins, and you do not feel like going over the translation and inspecting every line to make sure no JS or broken HTML was injected there. Even worse when the user can edit the translation directly or with plugin and you want to protect him from using out of ignorance things like >. Core gets away because it has trusted translators – Mark Kaplun Jun 3 '18 at 10:50
  • Even translators in the theme and plugin repository needs to be trusted by the authors before they can contribute, but I assume no plugin and theme author actually thinks about it before giving the trust. – Mark Kaplun Jun 3 '18 at 10:53
  • it has trusted translators, but... it still can be translated by any plugin using gettext filter ;) I agree that it’s not the best practice, I just can’t agree that it’s so rare (I’m one of translators for polish language and such translation strings are really common in core) ;) – Krzysiek Dróżdż Jun 3 '18 at 10:54
  • @KrzysiekDróżdż, but this is part of why people are annoyed with core security practices. Always escape output is actually part of the wordpress coding standard compliance tool check list. There is probably no one that will argue against that principal, but core is stuck in ways reflecting the security knowledge from 15 years ago and do not wish to change. As a core translator it is not that hard for you to suggest different handling for strings that require html tags in them, and those are probably less than 1% of core, and total 0 in most plugins where the translators just do the minimum – Mark Kaplun Jun 3 '18 at 11:05
2

Your argument appears to be that "Product" will always be "Product". As others have pointed out, this is not the case because (1) it may print a translation, and (2) it can be altered by filters in your theme or any plugin you have installed.

On the first point: Translator content should not be considered trusted (in my opinion) and so you should ALWAYS (when possible*) escape any localised string. Even if you trust your translators personally, they could innocently enter text that would break your web page if left unescaped.

So to your original question "Is this a correct usage of esc_html_e?"

Yes it is absolutely correct for both your examples.

If you're really worried that esc_html is overkill (presumably you're concerned about performance) you should at least use PHP's native htmlspecialchars function. I'll probably get lynched for suggesting that, because it's not best practice from a WordPress perspective. (you'll miss out on various other filters and character set checks).


* There is another case worth discussing here:

If the translation is intended to contain HTML, like this example then you will often see authors printing unescaped strings. Ideally untrusted HTML should be sanitized before printing, but even the WordPress core does not do this.

Sanitizing HTML carries a bigger overhead than simply escaping it, which possibly explains why even WordPress has examples of unescaped HTML translations. Here's one in twenty seventeen. Presumably the user-provided search query in %s has been sanitized, but the translation hasn't. Open up your favourite PO editor and enter a translation like this:

#: search.php:19
msgid "Search Results for: %s"
msgstr "Oh dear <script>alert(\"HACKED\")</script>"

Voila. Script injected into search results page. Try it. This actually works in twentyseventeen.

We can argue all day whether translator content should be trusted. I'd argue that it isn't. A stray, but perfectly innocent "<" could break your page.

1

It depends... As in almost any case...

So, is it whenever you output a string, dynamic or not, to HTML, you should also escape it with esc_html and so, even if it's just a string being output, you should use esc_html_e?

Yes and no. If you want to allow the HTML to be shown in there, then you don’t have to escape it. But if you want to be more secure and be certain that no HTML will be shown, then yes - you can do the escaping.

This “static string” is translated, so it’s not static anymore. Translators can put anything as translation. And even worse - you can use gettext filter to modify it, so not only translators, but also any plugin or theme can change that text...

It’s not so important with esc_html, but it starts to be, when you’ll take a look at esc_attr - translators can easily break your site putting wrong characters in string that is printed as HTML attribute.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.