The function node_list_permissions() (called by node_permission()) builds the permission strings by passing the content type machine name to check_plain(), while node_node_access(), which uses those permissions, doesn't build those permission strings by passing the content type machine name to check_plain().

Considering that the characters allowed for a machine name are letters (which don't include accented letters), underscores, and numbers, the call to check_plain() returns the same strings it gets as parameter; therefore, calling check_plain() is not necessary.
If the value returned by check_plain() is different from the value it gets (for example, the content type machine names are allowed to contain >), then the permissions being defined would be different from the permissions being checked.

function node_list_permissions($type) {
  $info = node_type_get_type($type);
  $type = check_plain($info->type);

  // Build standard list of node permissions for this type.
  $perms = array(
    "create $type content" => array(
      'title' => t('%type_name: Create new content', array('%type_name' => $info->name)),
    ), 
    "edit own $type content" => array(
      'title' => t('%type_name: Edit own content', array('%type_name' => $info->name)),
    ),
    //... (omissis)
 }
function node_node_access($node, $op, $account) {
  $type = is_string($node) ? $node : $node->type;

  if (in_array($type, node_permissions_get_configured_types())) {
    if ($op == 'create' && user_access('create ' . $type . ' content', $account)) {
      return NODE_ACCESS_ALLOW;
    }
    // ... (omissis)
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

apaderno’s picture

Title: node_permission() uses check_plain() for the content type machine name, but node_node_access() doesn't » node_permission() uses check_plain() without a purpose
naxoc’s picture

Version: 7.x-dev » 8.x-dev
FileSize
1.36 KB

This is still an issue in D8. Here is a patch that gets rid of the check.

swentel’s picture

Status: Active » Needs review

Let's see what the bot says

sun’s picture

Status: Needs review » Needs work
+++ b/core/modules/node/node.module
@@ -3141,23 +3141,22 @@ function node_node_access($node, $op, $account) {
-  $type = check_plain($info->type);
...
-    "create $type content" => array(
+    "create {$info->type} content" => array(

You can just drop the line involving check_plain() and retain the $type in the strings.

naxoc’s picture

Status: Needs work » Needs review
FileSize
452 bytes

You can just drop the line involving check_plain() and retain the $type in the strings.

You are right - I thought there was some kind of validation in node_type_get_type(). But this is much simpler. New patch.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x and back-ported to 7.x.

Status: Fixed » Closed (fixed)

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

cweagans’s picture

Issue tags: +Needs backport to D7
apaderno’s picture

Issue summary: View changes

Fixed formatting.