WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 8 months ago

#38116 new enhancement

Introduce is_callable check into apply_filters function to avoid failure of call_user_func_array causing all filters to fail.

Reported by: garrett-eclipse Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Plugins Keywords: needs-patch
Focuses: Cc:

Description

Hello,

Just wanted to suggest an improvement for the apply_filters function to avoid failures introduced by custom code or plugins that invalidly set an add_filter without providing the scope or misspelling the function.

Currently if a filter is applied incorrectly... in the case I came across I had a custom plugin which I missed wrapping the array($this,) around the function name. This caused an error in debug for call_user_func_array expecting valid callback. Due to this NULL is returned and all other filters are basically ignored.

Would be nice to do an is_callable check before execution of the call_user_func_array so as to skip any broken add_filters applied. I'd say still log an error so people can troubleshoot and find out why their filter failed, but would be nicer than returning null.

Thank you

Change History (4)

#1 @desrosj
15 months ago

  • Component changed from General to Plugins

#2 follow-up: @SergeyBiryukov
8 months ago

#49002 was marked as a duplicate.

#3 in reply to: ↑ 2 @sjoerdlinders
8 months ago

Replying to SergeyBiryukov:

#49002 was marked as a duplicate.

With the solution to solve the problem in file: class-wp-hooks.php

public function apply_filters( $value, $args ) {
    ...
    foreach ( $this->callbacks[ $priority ] as $the_ ) {
        if( ! $this->doing_action ) {
            $args[ 0 ] = $value;
        }

        // Check if callable function or method exists.
        if ( !isset( $the_['function'] ) || !is_callable( $the_['function'] ) ) continue;

        ....
    }
Last edited 8 months ago by sjoerdlinders (previous) (diff)

#4 @johnbillion
8 months ago

  • Keywords needs-patch added
  • Type changed from defect (bug) to enhancement
  • Version 4.6.1 deleted

Silently discarding an invalid callback isn't a good idea because it turns a visible error into an invisible one.

PHP errors exist for a reason and I think this is a valid case for an error. It's unfortunate that subsequent callbacks receive a null value, so if this guard condition is going to be put into place it needs to trigger a PHP error too.

This will need performance profiling too to ensure there's minimal performance impact.

Note: See TracTickets for help on using tickets.