Make WordPress Core

Opened 12 years ago

Closed 7 years ago

#19986 closed defect (bug) (wontfix)

Issue with add_filter() and add_action() when using anonymous functions.

Reported by: pagesimplify's profile pagesimplify Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.3.1
Component: Plugins Keywords: has-patch dev-feedback needs-unit-tests
Focuses: Cc:

Description

Referencing File: /wp-includes/plugin.php :: function add_filter()

function add_filter($tag, $function_to_add, $priority = 10, $accepted_args = 1) {
	global $wp_filter, $merged_filters;

	$idx = _wp_filter_build_unique_id($tag, $function_to_add, $priority);
	$wp_filter[$tag][$priority][$idx] = array('function' => $function_to_add, 'accepted_args' => $accepted_args);
	unset( $merged_filters[ $tag ] );
	return true;
}

When registering a function with add_filter() and add_action(), a unique id ($idx) is generated to be used as an array key in:

$wp_filter[$tag][$priority][$idx]

The function that creates this unique key ($idx) returns the function name passed into the $function_to_add parameter.

$idx = _wp_filter_build_unique_id($tag, $function_to_add, $priority);

If the $function_to_add parameter is a string, the function _wp_filter_build_unique_id() returns the original function name.

function _wp_filter_build_unique_id($tag, $function, $priority) {
	global $wp_filter;
	static $filter_id_count = 0;

	if ( is_string($function) )
		return $function;
   ...
}

Returning a string value for the unique id is a problem when the function is an anonymous function generated using create_function(). The reason is anonymous functions are named as "special character + lambda_XX" or as "ºlambda_XX" where the first character cannot be displayed.

See attached file: anonymous_function_name.jpg

Since array keys can only be integers or strings, the following array is invalid and causes inconsistent behaviors.

$wp_filter[$tag][$priority]['ºlambda_XX']

Although I've seen instances where this will still work, I've also observed strange issues with inconsistent results where plugins should work and sometimes do not. One example is with Otto's Simple Facebook Connect plugin.

See file: sfc-widgets.php

add_action('widgets_init', create_function('', 'return register_widget("SFC_Fan_Box_Widget");'));

I have not been able to isolate the conditions that allow this to work in one WordPress installation and not in others. However, I have been able to apply a patch to my installation with consistent and successful results.

See attached patch: plugin.php.diff

While this is an obscure and possibly very isolated issue, it could resolve unexplained issues related to using anonymous functions with add_filter() elsewhere.

Attachments (4)

anonymous_function_name.jpg (12.1 KB) - added by pagesimplify 12 years ago.
Anonymous Function Name viewed during XDebug
plugin.php.diff (370 bytes) - added by pagesimplify 12 years ago.
Simple patch to resolve issue with anonymous function names.
lambda_test.php (587 bytes) - added by kurtpayne 12 years ago.
Code to recreate
19986.diff (415 bytes) - added by DrewAPicture 8 years ago.
refresh

Download all attachments as: .zip

Change History (14)

@pagesimplify
12 years ago

Anonymous Function Name viewed during XDebug

@pagesimplify
12 years ago

Simple patch to resolve issue with anonymous function names.

#1 @scribu
12 years ago

  • Severity changed from normal to minor

While this looks harmless enough, I don't think we should commit it until we at least isolate the conditions under which it appears (PHP version + OS combination perhaps)?

#2 @pagesimplify
12 years ago

  • Cc pagesimplify added

Unfortunately, I noticed this in January 2011 and applied this patch to my WordPress installation then. So it's been a long while since I have observed the various conditions where I noticed this being an issue. I can say that I recall the issues were noticeable more between plugin/widget configurations for certain pages than between different system environments.

If I stretch my mental muscles just a bit more, I recall suspecting the issue was related to the combination of several properly named functions registered with an anonymous function, or perhaps multiple anonymous functions for a specific filter tag. I discovered the issue when attempting to step through code in a plugin that wasn't running properly. As I moved the break points up the possible callstack to the apply_filters function, I noticed I could never step past a certain number of function calls in the apply filters loop. This was because the apply_filter function could not retrieve the array item stored with an invalid array key name. I can't recall if this would work if no other functions were registered or if it was the last item in the array.

In this specific case, I do recall that by removing some widgets from a page or by deactivating other plugins, I could get the problematic filter to run. However, this is still anecdotal in that I never tested to isolate all conditions for this issue to occur. Once I isolated the issue while troubleshooting Otto's SFC plugin, I simply applied the fix and the very random issues I previously experienced were no longer an issue.

It may also be helpful to know I mostly run multisite installations and will manage many blogs running in the same network. So, I was encountering random issues with many different sites on the same network using different themes, activated plugins, and widget configurations. Again, problems went away after applying the trim() function to the string return value.

Hopefully, this can resolve a lot of random issues people experience with no real explanation.

#3 follow-up: @SergeyBiryukov
12 years ago

Anonymous function names indeed start with a null character, in order to ensure they don't clash with user-defined functions:

That said, null is also a valid array key (will evaluate to an empty string):
http://www.php.net/manual/en/language.types.array.php#30132

Both of these examples seem to work:

$foo = array( null => 'bar' );
echo $foo[''];

$foo = array( chr(0) . 'key' => 'bar' );
echo $foo[ chr(0) . 'key' ];

@kurtpayne
12 years ago

Code to recreate

#4 in reply to: ↑ 3 @kurtpayne
12 years ago

  • Cc kpayne@… added

Replying to SergeyBiryukov:

Anonymous function names indeed start with a null character, in order to ensure they don't clash with user-defined functions

You beat me to it!

Replying to pagesimplify:

Hopefully, this can resolve a lot of random issues people experience with no real explanation.

It doesn't look like the patch shouldn't cause a problem. But a patch like this has a lot of implications. It would probably be best to understand exactly what broke to begin with.

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


10 years ago

@DrewAPicture
8 years ago

refresh

#6 @DrewAPicture
8 years ago

  • Keywords dev-feedback added
  • Milestone changed from Awaiting Review to 4.4
  • Owner set to SergeyBiryukov
  • Status changed from new to assigned

19986.diff refreshes the patch.

Reproduced with @kurtpayne's test code.

#7 @DrewAPicture
8 years ago

  • Keywords needs-unit-tests added

#8 @SergeyBiryukov
8 years ago

Per the IRC discussion, the issue is specific to Xdebug:

nacin: if we trim whatever _wp_filter_build_unique_id() returns we risk breaking things
nacin: I would want to avoid messing with that.
JohnBeales: What if we just strip out null bytes?
nacin: well, it'd be slower for no reason, and the purpose of the null byte is also to guarantee its own namespace
nacin: it sounds to me like it would be very easy to create a non-WordPress proof-of-concept where this fails, and submit it as a bug to XDebug

#9 @SergeyBiryukov
8 years ago

  • Milestone changed from 4.4 to Awaiting Review
  • Owner SergeyBiryukov deleted

#10 @dd32
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

5 years on, I'm marking this as wontfix.

create_function() has mostly been replaced by utilising anonymous functions directly (which don't use the null naming scheme, but instead go through spl_object_hash()), and if XDebug hasn't been updated to support null array items yet, I don't think we need to work around it's issues.

Note: See TracTickets for help on using tickets.