Drupal\field_ui\DisplayOverview takes the definitions of extra fields, and runs the label through check_plain():

    $extra_fields = field_info_extra_fields($this->entity_type, $this->bundle, 'display');
// SNIP
        'human_name' => array(
          '#markup' => check_plain($extra_field['label']),
        ),

However, the documentation and the sample code for the hook that defines the label, http://api.drupal.org/api/drupal/core!modules!field!field.api.php/functi..., implies that it's the hook's responsibility to sanitize this:

> label: The human readable name of the element.

'Human labels' are usually sanitized text, I think. And the sample code has:

    $extra['node'][$bundle->type]['display']['language'] = array(
      'label' => t('Language'),
      'description' => $description,
      'weight' => 0,
      'visible' => FALSE,
    );

The problem with the user running check_plain() is that the '%' placeholder can't be used in the call to t() for emphasis.

For example, with Flag module, I'd like to say: "Flag %user-entered-label".

CommentFileSizeAuthor
#1 1991292-1.patch2.35 KBswentel

Comments

swentel’s picture

Status: Active » Needs review
StatusFileSize
new2.35 KB

Makes sense in a way. Most extra field labels are defined programmatically. Except for the title label for a content type.

yched’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Agreed.

catch’s picture

Title: output of labels from hook_field_extra_fields() should not use check_plain() » Change notice: output of labels from hook_field_extra_fields() should not use check_plain()
Priority: Normal » Major
Status: Reviewed & tested by the community » Active

Committed/pushed to 8.x

This could use a change notice, since modules currently not sanitizing user-entered text in this hook will become insecure now.

joachim’s picture

I don't remember the process for a CR. I've filed https://drupal.org/node/2153775 -- does it need someone to review it?

swentel’s picture

Title: Change notice: output of labels from hook_field_extra_fields() should not use check_plain() » Output of labels from hook_field_extra_fields() should not use check_plain()
Priority: Major » Normal
Status: Active » Fixed
Issue tags: -Needs backport to D7

Yep - and is good for me.

We can't really backport this without potentially breaking a lot of things in contrib.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.