Hello,

I recently encountered a case where some custom variables attached to the global $user variables (set by a custom module) were mysteriously being overwritten part-way through various page requests. With a bit of debugging I was able to trace this to the custom_breadcrumbs module, and specifically the function custom_breadcrumbs_token_types(). Within that function $global user; is called, and then soon after $user = user_load(array('uid' => $user->uid)); is called. This essentially "resets" the global $user variable and overwrites any custom data that has been set there by any other modules.

From what I can gather this is considered bad practice. According to the docs for user_load() at http://api.drupal.org/api/drupal/modules!user!user.module/function/user_...

Drupal has a global $user object, which represents the currently-logged-in user. So to avoid confusion and to avoid clobbering the global $user object, it is a good idea to assign the result of this function to a different local variable, generally $account.

I know this is a quote from the D7 api, but I believe the same principle applies to D6?

Anyway, I'm wondering if it would simply make more sense to adjust the custom_breadcrumbs_token_types() function to use $account = user_load(array('uid' => $user->uid)); instead... unless of course there is some specific reason that the global $user variable needs to be reset there?

Comments

rjacobs’s picture

Status: Active » Needs review
StatusFileSize
new874 bytes

Attached is a small patch that implements the fix I described above.

mwadi’s picture

#1 fix this issue.

lamp5’s picture

Status: Needs review » Closed (outdated)