Wrong check for caching user roles in user_access()

Not sure how to reproduce, if it was possible you would get a SQL error, because the IN() clausule in the query would be empty. Note that $account->roles is used to build the query, hence the check (which is what's wrong here).

Please, examine this code snippet in user_access():

  if (is_null($account)) {
    $account = $user;
  }

Ok, this is because user_access defaults to check permissions for current user, if no account object has been passed in the second argument. Hence, from this point on, $user is not needed. It is all about $account.

Now, examine this:

  // To reduce the number of SQL queries, we cache the user's permissions
  // in a static variable.
  if (!isset($perm[$account->uid]) && count($user->roles)) {
    $result = db_query("SELECT DISTINCT(p.perm) FROM {role} r INNER JOIN {permission} p ON p.rid = r.rid WHERE r.rid IN (%s)", implode(',', array_keys($account->roles)));

    $perm[$account->uid] = '';
    while ($row = db_fetch_object($result)) {
      $perm[$account->uid] .= "$row->perm, ";
    }
  }

Note the WHERE condition is built from $account->roles, hence the if condition should check for it, rather than checking for $user->roles. Otherwise, if $account->roles was empty you would get a SQL error.

However, you will never see this error because "roles" is never empty. In fact, checking for count() is somehow redundant, which may lead then to remove it from the IF expression, so the attached patch would have to be re-worked. I felt it was safer to check for the proper property rather than removing the check.

In other words, checking for count($user->roles) makes no sense. On the other hand, checking for count($account->roles) would avoid an SQL error, if the array was empty.

Yep! jvandyk, you're right. My patch was wrong. Thanks

http://drupal.org/node/46239

It seems there are places where $account->roles is empty.

fixed here: http://drupal.org/node/46137