Problem/Motivation

The parent issue has this for one of the use cases of token::replace (not always):

User-entered strings with markup should mean the return of Token::replace() is Xss::filter()d or explicitly trusted (i.e. marked as safe) and permission-restricted.

Except marked as safe is no longer possible. All the ways to do that is @internal or @deprecated . In my case $text is admin provided and so it can be trusted and all the placeholders are escaped by token anyways:

      $replacements[$token] = $value instanceof MarkupInterface ? $value : new HtmlEscapedText($value);

Proposed resolution

Add a TrustedTokenMarkup domain object? No idea really.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx created an issue. See original summary.

chx’s picture

Issue summary: View changes
chx’s picture

Status: Active » Needs review
FileSize
754 bytes

Tentative patch.

chx’s picture

Title: Token::replace @return is misleading » Token::replace should return MarkupInterface

Status: Needs review » Needs work

The last submitted patch, 3: 2662296_3.patch, failed testing.

chx’s picture

Status: Needs work » Needs review
FileSize
734 bytes

Status: Needs review » Needs work

The last submitted patch, 6: 2662296_3.patch, failed testing.

chx’s picture

Status: Needs work » Needs review

These are relatively easy to fix. Needs review more than work.

chx’s picture

Title: Token::replace should return MarkupInterface » There's no way to trust Token::replace() @return
Issue summary: View changes
Status: Needs review » Active
Parent issue: » #2567257: hook_tokens() $sanitize option incompatible with Html sanitisation requirements
chx’s picture

Issue summary: View changes
chx’s picture

Issue summary: View changes
alexpott’s picture

I think Token probably should have its own MarkupInterface object. And what's fun about this is that we could then delay replacement until render time - exactly the way we do with translated strings now.

alexpott’s picture

I'm pretty sure we have more to do on the safeness of token replacement though. It's a super complex issue.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Berdir’s picture

I think this is more or less a duplicate of #2580723: Fix token system confusion, with new function Token::replacePlain(), where I'm proposing something similar now, as a new method in a way that is as safe as possible (this is not).

chx’s picture

Status: Active » Closed (duplicate)