Filters don't check that the function is callable

[aaroncampbell]

This is a problem that I found in Shopp. Basically, they used add_filter('redirect_canonical', 'canonical_home'); when it should have been add_filter('redirect_canonical', array( $this, 'canonical_home' ));, so the function added to redirect_canonical didn't actually exist. However, it's still called and NULL is always returned, so the canonical redirects completely stopped working.

It seems like we should add an is_callable test to either apply_filters or add_filter. I'd prefer to see it on apply_filters simply because more filters are added than are actually run.

As Nacin pointed out, this is additional execution and our filters need to stay lean and mean. Still, I see both sides to this so I thought I'd try to get some feedback from other devs.

[scribu]

It seems like we should add an is_callable test to either apply_filters or add_filter. I'd prefer to see it on apply_filters simply because more filters are added than are actually run.

There might be cases where the callback being added won't be available until the apply_filters() call.

The current behaviour triggers an error, making it easier to debug.

We could do the test ourselves and trigger our own error, but what's the point?

[aaroncampbell]

I know that it's really the fault of the plugin author, and should really be something they are more careful about. However, I also know that we do have things in core to try to keep plugin authors from doing stupid things. I'm just on the fence over whether it's worth it since this could potentially have a big impact.

[aaroncampbell]

Oh, and I hadn't thought about the callback not being available when the filter is added. I was really thinking this would go into apply_filters, which would avoid that issue. However, I'd like to see some stats on what is used most in a default install.

[nacin]

Argument for:

• Prevents silent failures by skipping the filter.

• We code defensively for plugins in many other places.

Arguments against:

• Speed. We need these functions to be fast as possible.

• The callback may not be available on add_filter, and apply_filters runs 1500 times on a typical pageload. (300 times for add_filter -- per westi in IRC.)

• Silent failures do not help plugin authors.

[nacin]

When the callback doesn't exist, an E_WARNING is generated. I see no reason to guard against an obvious error such as that.

[aaroncampbell]

I appreciate the feedback. It would be nice to protect sites from bad plugins/themes, but it's pretty obvious this is too much.

[nacin]

Replying to aaroncampbell:

It would be nice to protect sites from bad plugins/themes, but it's pretty obvious this is too much.

That's what "Deactivate" and "Delete" is for :-)