Make WordPress Core

Opened 6 years ago

Last modified 6 years ago

#46188 new enhancement

esc_html does not have support for multiline output. esc_br_html or line-breaking parameter for esc_html is missing

Reported by: kestutisit's profile KestutisIT Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.0.3
Component: Formatting Keywords: needs-patch
Focuses: template Cc:

Description

Let's say that we want to save not a title, but a block of text in the database. So we have to support multiline escaping.

Now I have to do this:

<?php
$escapedMultilineItemDescriptionArray = array_map('esc_html', explode("\n", $data['item_description']));
$printItemDescription = implode("\n", $escapedMultilineItemDescriptionArray );

$objView = new View();
$objView->itemDescription = $printItemDescription;

But then the reviewers at Envato and other coding standards fans are not happy that at the template file I use:

<div class="item-description"><?=nl2br($itemDescription);?></div>

While following the concept of of 'escaping at the template' would could be instead 'esc_br_html':

<div class="item-description"><?=esc_br_html($itemDescription);?></div>

or with fuction esc_html($text, $escapeLineBreaks = FALSE) {...}

<div class="item-description"><?=nl2br(esc_html($itemDescription, TRUE));?></div>

I just see a lot of confusion and misinterpreation of escaping of text that has multiple lines, and there is NO function. And we should not do explode, implode, array_map things inside the template code, as the template is for designers, and ever CSS developer has to be able easily understand the template, so there so be no explodings, implodings.

Change History (4)

#1 follow-up: @swissspidy
6 years ago

  • Focuses coding-standards removed

What about using esc_textarea() or hooking into the esc_html filter instead?

#2 in reply to: ↑ 1 @KestutisIT
6 years ago

Replying to swissspidy:

What about using esc_textarea() or hooking into the esc_html filter instead?

I'm not sure if filter-hook is good decision. As this has to be global for all plugin developers, meaning a standard defined in coding standards.

What I did, is that I created a 'fake' formating.php file in my plugin folder to replicate the missing lines of code in \wp-includes\formatting.php:

PATCH could be the following for the \wp-includes\formatting.php file:

<?php

if(!function_exists('esc_br_html'))
{
    /**
     * Escape with line-breaks
     * 
     * Related ticked - https://core.trac.wordpress.org/ticket/46188
     * 
     * @param string $text
     * @return string
     */
    function esc_br_html($text)
    {
        $escaped_text_array = array_map('esc_html', explode("\n", $text));
        $escaped_multiline_text = implode("<br />", $escaped_text_array);

        return $escaped_multiline_text;
    }
}

Regarding the esc_textarea - that would be a BAD decision, as it impacts all the other chars, it is dedicated to use inside <textarea> HTML tag, and probably esc_textarea does not escapes single quotes. I mean the same title with just the need of span in via multiple lines is so much common case that I saw it over 1000 times in recent years, but only now everybody is bumping so much to the standards, so we need to finally make a solution for everybody, so I believe we need to add one more function to \wp-includes\formatting.php or to add an additional parameter support to esc_html.

Last edited 6 years ago by KestutisIT (previous) (diff)

#3 follow-up: @timph
6 years ago

In HTML you can preserve linebreaks using CSS, commonly it's used on a pre element, but could be used where you might see fit by setting the white-space property's value to pre. This would give you a similar style as replacing the newline with a <br /> in your example.

In WordPress there is already a function to handle similar formatting to replace newlines with breaks and to replace double new lines with paragraphs. wpautop: https://developer.wordpress.org/reference/functions/wpautop/.

If your user is entering it in some sort of textarea field and you're wanting them to have a visual representation there's the wp_editor() function to create a visual editor (tinymce instance), and one of the configurations you can pass in is for wpautop behavior.

#4 in reply to: ↑ 3 @KestutisIT
6 years ago

Replying to timph:

If your user is entering it in some sort of textarea field and you're wanting them to have a visual representation there's the wp_editor() function to create a visual editor (tinymce instance), and one of the configurations you can pass in is for wpautop behavior.

This is a bad suggestion that is against secure code practices. Secure code practices says that we should give the LEAST POSSIBLE access/rights to our users. So, if we don't need then to give a HTML rights, and that is i.e. "Notes" field in some kind of booking/reservation field, it is enough to give the user plain textarea with ability to enter multiple lines. That is a secure code practice. If we do not follow it, we would go into vulnerable-plugin situation with the things like this where all WP version are vulnerable until 5.0.3 that had this kind of permissions - https://thehackernews.com/2019/02/wordpress-remote-code-execution.html , while if only the plain text is allowed and line breaks - the website would not be affected, and all the website using that plugin would not become ready-for-hacking.

The standard way to preserve the line breaks is via BR HTML tag or \n character. Everything else is out of scope, custom hacks that are not well-know standards. If I have to escape the content - as WordPress says - escape all output, and I run the default esc_html(..) I don't see any way how wpautop would help here anyhow.

Note: See TracTickets for help on using tickets.