Should do an exact match on perm, not substring. I also just cleaned up default perms to make a little more sense.

CommentFileSizeAuthor
#2 user.perm__0.patch1.57 KBRobRoy
user.perm_.patch1.04 KBRobRoy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RobRoy’s picture

Status: Needs review » Needs work

Whoops, didn't really test this. It's still an issue, but lemme get a working patch. :D

RobRoy’s picture

Status: Needs work » Needs review
FileSize
1.57 KB

Okay, working patch. This is a potential security hole / oversight. Imagine a perm 'minister' or something. Then all those ministers can 'administer nodes'! :P

Should I set a variable to isset($permission) or is isset just as fast? I'm open to reorganizing this code if someone thinks it yucky.

Anonymous’s picture

Status: Needs review » Needs work

The if (isset($permission)) doesn't work since $permission is always set. I suggest changing the default to FALSE and using if ($permission !== FALSE) instead.

RobRoy’s picture

Status: Needs work » Needs review

Sorry, you're wrong. Something set to NULL is actually "unset" in PHP's eyes. Please see http://us.php.net/manual/en/types.comparisons.php or test the code in PHP yourself.

function user_roles($membersonly = FALSE, $permission = NULL) {
Anonymous’s picture

@RobRoy: Thanks for the reference to the documentation and sorry for the misinformation.

The patch looks good, I'm not setup yet to test though.

TR’s picture

Status: Needs review » Needs work

The method signature and the check on !empty($permissions) were already addressed in commits a816fead and b17cd3b6.

The promiscuous matching of permission name is still in there - I don't know whether this was by design or not, but it *has* been removed in Drupal 7 where an exact match is now required. Leaving this open as a Drupal 6.x issue.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.