Download & Extend

Only consider user-defined functions as hooks

Project:Drupal core
Version:7.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:yhahn
Status:needs work

Issue Summary

In PHP 5.2 there is a new function called date_diff() which will create problems when using hook_diff() when the Date module is enabled. Not sure what to do about it, just a heads-up.

http://www.php.net/manual/en/function.date-diff.php.

Comments

#1

Edit, I mean PHP 5.3.

#2

Assigned to:Anonymous» yhahn

o man. hook #fail

will think about best path to victory for this one...

#3

That certainly explains why I cannot find the date_diff() in the date module that I happen to have installed.

The only thing I can imagine doing is adding a more unique string to the _diff() hook, but that would break existing modules.

Is it possible to blacklist certain names for drupal hooks?

I will look into this as well as I would like to get this solved asap.

#4

Title:date_diff hook» hook_diff() naming conflict with internal php function: date_diff()
Project:Diff» Drupal core
Version:6.x-2.x-dev» 6.x-dev
Component:Code» base system
Status:active» needs review

After looking around the PHP docs, this seems to be what we want to use:
http://us3.php.net/manual/en/function.get-defined-functions.php

PHP core functions get stored in the array called: internal
All others seem to get stored in the array called: user

The drupal_hook() needs to be changed to only call a hook if it is defined in the user array.

I have attached a patch that makes this check for drupal 6.x.

The patch changes module_invoke and module_invoke_all to have the following added:

<?php
$defined
= get_defined_functions();

if (
is_array($defined) && array_key_exists('user', $defined) && in_array($function, $defined['user'])){
 
// function calls wrapped here
}
?>

I have moved this thread to drupal core because I believe this is the proper solution to this problem.
If you think otherwise, please move this issue back to the Diff project.

AttachmentSizeStatusTest resultOperations
drupal-6.x-avoid_internal_hookage-1.patch2.82 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-6.x-avoid_internal_hookage-1.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.View details | Re-test

#5

Title:hook_diff() naming conflict with internal php function: date_diff()» Any consider user-defined functions as hooks
Version:6.x-dev» 7.x-dev
Category:task» bug report
Status:needs review» needs work

Bumping to D7 for consideration. Changing module_hook() seems to be the only thing necessary here.

#6

I wonder what would happen if the get_defined_function() failed to return a properly populated array.

I am now thinking that if the get_defined_function() function failed to return sane results that the code should gracefully fail.
Would isset() be a better choice in this case than is_array($defined) && array_key_exists('user', $defined)?

For example:

<?php
$defined
= get_defined_functions();

if ((isset(
$defined['user']) && in_array($function, $defined['user'])) || !isset($defined['user'])){
 
// function calls wrapped here
}
?>

The idea with that change would be to fallback to previous behavior if the $defined variable is not properly populated.

EDIT:
After reading #5, I just tried only changing module_hook(), but the original php error message appears.
Did you mean module_hook_all()?
I also just tried only changing module_hook_all() instead of module_hook() and the original php error message does not appear.

#7

Title:Any consider user-defined functions as hooks» Only consider user-defined functions as hooks

#8

Status:needs work» needs review

#4: drupal-6.x-avoid_internal_hookage-1.patch queued for re-testing.

#9

Status:needs review» needs work

The last submitted patch, drupal-6.x-avoid_internal_hookage-1.patch, failed testing.

nobody click here