Drupal coder: hook_load and data isolation

Submitted by Frederic Marand on

You're peacefully coding a hook_load implementation in your latest drupal module, when your admin account suddenly gets "Access denied" errors looking at nodes. Yikes...

Yesterday, I was coding a first version of helpdesk_load, the hook_load implementation in the helpdesk module, when all of a sudden, drupal stopped displaying my nodes and returned "Access denied".

At this point, the trouble lies in that I'm logged as admin on the site, which means that access isn't even checked, so access denial is theoretically out of the question: the access hooks are automatically bypassed in this case.

Commenting out helpdesk_load, problem disappears. Uncommenting, problem reappears. OK, it's got to be me, then... some Zend debugger work shows that the global $node->nid suddenly disappears after helpdesk_load, although it is correctly initialized upon entering it.

Look at the API spec for hook_load: it says the hook is used to load extra information, which is just what I'm doing. And I can see the extra information when I return from helpdesk_load into drupal's node_load.

Would it be some side effet ? Some (shudder) stack overwrite ? Not at all... it is actually an undocumented (yet) feature of hook_load, which is actually used not only to load extra information, but also possibly overwrite drupal's pre-loaded information. Including $node->nid. And as my code has its own nid value, uninitialized manually because it isn't used, this value was returned into node_load, which silently overwrites the current nid. Which meant it finds itself choking a few lines below because it no longer has a valid nid to process.

There's now an issue on this, so the documentation should be updated reasonably soon. Problem closed. Discussion on the drupal IRC channel has confirmed this overwriting behaviour is "as designed" and actually used by some essential modules.

In the meantime, this gives a lesson, IMHO: when coding a hook_load implementation, you never know what property names the next contributed module will define, which means name collisions are bound to happen, and even more so as drupal develops more widely day after day and the number of available modules increases constantly, and at some point, only alphabetical order of the modules defines the load order, in the current version. So make your names unique.

To achieve this uniqueness constraint without kilometric identifiers, it can be a good idea to wrap the returned object required by the hook_load spec into another object. The outer object will be stripped by node_load, which will then just see one property, the inner object, and add it to the global node's properties as a nicely isolated black-box object. That way you'll be sure your implemented can never overwrite a prior module's own properties, and no module loaded after yours can overwrite your own properties. OO-typical safety at a minimal overhead cost.

Tagged for , , , .