In some cases strings returned from hook implementations are not completely sanitized before they are printed; sometimes they are. It is left up to the module developer to figure out to what degree. Why not make this clear either in the hook example or the doc block? This could avoid some misunderstandings and potential security issues if a module developer is not familiar with or misunderstands what happens when the hook is invoked.

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Postponed (maintainer needs more info)

I don't think anyone would argue with this principle, or with clarifying hook documentation in this way.

But this issue is not really actionable as it is... What hooks specifically do you think need more documentation in this regard?

David_Rothstein’s picture

David_Rothstein commented
Issue tags: +Needs backport to D7

This was discussed a bit in the Drupal Security Team before we concluded it was essentially a documentation issue and therefore decided it was OK to file publicly. The example that @Tor Arne Thune brought up in the private discussion originally was hook_verify_update_archive(), which is expected to return already-sanitized strings (which are then printed as-is to the page). I then brought up hook_permission() which is another example of the same thing.

Right at this moment, and off the top of my head, and I can't think of any examples of hooks where this is not the case, except for things like hook_menu() where you don't even translate the string either.

Maybe in core it's the case that whenever your code is supposed to return a translated string, it's also supposed to sanitize it for use in HTML, and otherwise it's not? If so, we could use that as a basis for documentation.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

OK. Some of the hooks we have are already documented with "this should be translated", etc. Some are probably not... Anyway, a patch would be good, or even a careful inventory of what needs to be documented would help.

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.

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

pameeela’s picture

pameeela commented
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Closed (outdated)
Issue tags: +Bug Smash Initiative

As part of the Bug smash initiative, we are triaging issues that are marked 'Postponed (maintainer needs more info)'. This issue is no longer relevant so I am marking it 'Closed (outdated)'.