Problem/Motivation

drupal_get_messages() calls SafeMarkup::set() which is meant to be for internal use only.

Proposed resolution

  • Remove the call by refactoring the code.
  • If refactoring is not possible, thoroughly document where the string is coming from and why it is safe, and why SafeMarkup::set() is required.

Remaining tasks

  1. Evaluate whether the string can be refactored to one of the formats outlined in this change record: https://www.drupal.org/node/2311123
  2. Identify whether there is existing automated test coverage for the sanitization of the string. If there is, list the test in the issue summary. If there isn't, add an automated test for it.
  3. If the string cannot be refactored, the SafeMarkup::set() usage needs to be thoroughly audited and documented.

Manual testing steps (for XSS and double escaping)

Not necessary, we are only adding documentation.

User interface changes

N/A

API changes

N/A

CommentFileSizeAuthor
#1 document-2501451-1.patch856 bytesstar-szr

Comments

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new document-2501451-1.patch856 bytes

This one is legitimate, only marking strings as safe if they passed SafeMarkup::isSafe() in the previous request.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Beauty, thanks for adding the @see drupal_set_message()

  • xjm committed 7b94869 on 8.0.x 
    Issue #2501451 by Cottser, joelpittet: Document SafeMarkup::set in...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

I smiled at "let autoescape do its thing", but decided I'm in favor of that wording. :) This call is an intentional part of the API for drupal_set_message() so referencing that function makes that intent clear.

This issue only changes documentation (and is part of resolving a critical), so per https://www.drupal.org/core/beta-changes, this can be completed any time during the Drupal 8 beta phase. Committed and pushed to 8.0.x. Thanks!

Status: Fixed » Closed (fixed)

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