Skip to content

Commit

Permalink
Merge pull request #6812 from ampproject/fix/plugin-suppression-at-wp…
Browse files Browse the repository at this point in the history 
…-action

Account for validation-wrapped callbacks in plugin suppression
  • Loading branch information
@westonruter
westonruter authored Jan 26, 2022
2 parents 0bb7948 + 7be8965 commit a5f6909
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 49 deletions.
43 changes: 35 additions & 8 deletions includes/validation/class-amp-validation-callback-wrapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,29 @@ class AMP_Validation_Callback_Wrapper implements ArrayAccess {
*/
protected $callback;

/**
* Function to call before invoking the callback.
*
* @var callable|null
*/
protected $before_invoke;

/**
* AMP_Validation_Callback_Wrapper constructor.
*
* @param array $callback {
* @param array $callback {
* The callback data.
*
* @type callable $function
* @type int $accepted_args
* @type array $source
* @type array $indirect_sources
* }
* @param callable|null $before_invoke Additional callback to run before invoking original callback. Optional.
*/
public function __construct( $callback ) {
$this->callback = $callback;
public function __construct( $callback, $before_invoke = null ) {
$this->callback = $callback;
$this->before_invoke = $before_invoke;
}

/**
Expand All @@ -50,7 +59,7 @@ public function get_callback_function() {
*
* @since 1.5
*
* @param array ...$args Arguments.
* @param mixed ...$args Arguments.
* @return array Preparation data.
*
* @global WP_Scripts|null $wp_scripts
Expand Down Expand Up @@ -124,6 +133,10 @@ protected function prepare( ...$args ) {
* @return mixed Response.
*/
public function __invoke( ...$args ) {
if ( $this->before_invoke ) {
call_user_func( $this->before_invoke );
}

$preparation = $this->prepare( ...$args );

$result = call_user_func_array(
Expand All @@ -146,12 +159,26 @@ public function __invoke( ...$args ) {
* @return mixed
*/
public function invoke_with_first_ref_arg( &$first_arg, ...$other_args ) {
if ( $this->before_invoke ) {
call_user_func( $this->before_invoke );
}

$preparation = $this->prepare( $first_arg, ...$other_args );

$result = $this->callback['function'](
$first_arg,
...array_slice( $other_args, 0, (int) $this->callback['accepted_args'] - 1 )
);
$function = $this->callback['function'];
$other_accepted_args = array_slice( $other_args, 0, (int) $this->callback['accepted_args'] - 1 );

if ( $function instanceof self ) {
$result = $function->invoke_with_first_ref_arg(
$first_arg,
...$other_accepted_args
);
} else {
$result = $function(
$first_arg,
...$other_accepted_args
);
}

$this->finalize( $preparation );

Expand Down
27 changes: 15 additions & 12 deletions includes/validation/class-amp-validation-manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,7 @@ public static function wrap_block_callbacks( $args ) {
self::$original_block_render_callbacks[ $args['name'] ] = $original_function;
}

$args['render_callback'] = self::wrapped_callback(
$args['render_callback'] = new AMP_Validation_Callback_Wrapper(
[
'function' => $original_function,
'source' => $source,
Expand Down Expand Up @@ -1396,7 +1396,7 @@ public static function wrap_widget_callbacks() {
$accepted_args = 2; // For the $instance and $args arguments.
$callback = compact( 'function', 'accepted_args', 'source' );

$registered_widget['callback'] = self::wrapped_callback( $callback );
$registered_widget['callback'] = new AMP_Validation_Callback_Wrapper( $callback );
}
}

Expand Down Expand Up @@ -1461,23 +1461,23 @@ public static function wrap_hook_callbacks( $hook ) {
$source['hook'] = $hook;
$source['priority'] = $priority;
$original_function = $callback['function'];
$wrapped_callback = self::wrapped_callback(

$wrapped_callback = new AMP_Validation_Callback_Wrapper(
array_merge(
$callback,
compact( 'priority', 'source', 'indirect_sources' )
)
),
static function () use ( &$callback, $original_function ) {
// Restore the original callback function in case other plugins are introspecting filters.
// This logic runs immediately before the original function is actually invoked.
$callback['function'] = $original_function;
}
);

if ( 1 === $passed_by_ref ) {
$callback['function'] = static function( &$first, ...$other_args ) use ( &$callback, $wrapped_callback, $original_function ) {
$callback['function'] = $original_function; // Restore original.
return $wrapped_callback->invoke_with_first_ref_arg( $first, ...$other_args );
};
$callback['function'] = [ $wrapped_callback, 'invoke_with_first_ref_arg' ];
} else {
$callback['function'] = static function( ...$args ) use ( &$callback, $wrapped_callback, $original_function ) {
$callback['function'] = $original_function; // Restore original.
return $wrapped_callback( ...$args );
};
$callback['function'] = $wrapped_callback;
}
}
}
Expand Down Expand Up @@ -1671,6 +1671,9 @@ public static function can_output_buffer() {
* this indicates which plugin it was from.
* The call_user_func_array() logic is mainly copied from WP_Hook:apply_filters().
*
* @deprecated No longer used as of 2.2.1.
* @codeCoverageIgnore
*
* @param array $callback {
* The callback data.
*
Expand Down
34 changes: 34 additions & 0 deletions src/DevTools/CallbackReflection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace AmpProject\AmpWP\DevTools;

use AMP_Validation_Callback_Wrapper;
use AmpProject\AmpWP\Infrastructure\Service;
use Exception;
use ReflectionFunction;
Expand Down Expand Up @@ -38,6 +39,37 @@ public function __construct( FileReflection $file_reflection ) {
$this->file_reflection = $file_reflection;
}

/**
* Get the underlying callback in case it was wrapped by AMP_Validation_Callback_Wrapper.
*
* @since 2.2.1
*
* @param callable $callback Callback.
* @return callable Original callback.
*/
public function get_unwrapped_callback( $callback ) {
while ( $callback ) {
if ( $callback instanceof AMP_Validation_Callback_Wrapper ) {
$callback = $callback->get_callback_function();
} elseif (
is_array( $callback )
&&
is_callable( $callback )
&&
isset( $callback[0], $callback[1] )
&&
$callback[0] instanceof AMP_Validation_Callback_Wrapper
&&
'invoke_with_first_ref_arg' === $callback[1]
) {
$callback = $callback[0]->get_callback_function();
} else {
break;
}
}
return $callback;
}

/**
* Gets the plugin or theme of the callback, if one exists.
*
Expand All @@ -54,6 +86,8 @@ public function __construct( FileReflection $file_reflection ) {
* }
*/
public function get_source( $callback ) {
$callback = $this->get_unwrapped_callback( $callback );

$reflection = $this->get_reflection( $callback );

if ( ! $reflection ) {
Expand Down
9 changes: 8 additions & 1 deletion src/PluginSuppression.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,18 @@ private function prepare_user_for_response( $username ) {
*/
private function suppress_hooks( $suppressed_plugins ) {
global $wp_filter;
/** @var WP_Hook $filter */
foreach ( $wp_filter as $tag => $filter ) {
foreach ( $filter->callbacks as $priority => $prioritized_callbacks ) {
foreach ( $prioritized_callbacks as $callback ) {
if ( $this->is_callback_plugin_suppressed( $callback['function'], $suppressed_plugins ) ) {
$filter->remove_filter( $tag, $callback['function'], $priority );
// Obtain the original function which is necessary to pass into remove_filter() so that the
// expected key will be generated by _wp_filter_build_unique_id(). Alternatively, this could
// just below do `unset( $filter->callbacks[ $priority ][ $function_key ] )` but it seems safer
// to re-use all the existing logic in remove_filter().
$original_function = $this->callback_reflection->get_unwrapped_callback( $callback['function'] );

$filter->remove_filter( $tag, $original_function, $priority );
}
}
}
Expand Down
102 changes: 100 additions & 2 deletions tests/php/src/DevTools/CallbackReflectionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@

namespace AmpProject\AmpWP\Tests\DevTools;

use AMP_Validation_Callback_Wrapper;
use AmpProject\AmpWP\DevTools\CallbackReflection;
use AmpProject\AmpWP\DevTools\FileReflection;
use AmpProject\AmpWP\Tests\DependencyInjectedTestCase;
use AmpProject\AmpWP\Tests\Helpers\LoadsCoreThemes;
use ReflectionFunction;
use ReflectionMethod;
use AmpProject\AmpWP\Tests\Helpers\LoadsCoreThemes;

/**
* Tests for CallbackReflection class.
Expand Down Expand Up @@ -54,6 +54,104 @@ public function tearDown() {
$this->restore_theme_directories();
}

/**
* Test get_unwrapped_callback().
*
* @covers ::get_unwrapped_callback()
* @covers AMP_Validation_Callback_Wrapper::__invoke()
* @covers AMP_Validation_Callback_Wrapper::invoke_with_first_ref_arg()
*/
public function test_get_unwrapped_callback() {
$original_by_value_callback = function ( $number ) {
return $number + 1;
};
$original_by_ref_callback = function ( &$number ) {
$number++;
};

$this->assertSame(
$original_by_value_callback,
$this->callback_reflection->get_unwrapped_callback( $original_by_value_callback )
);
$this->assertSame(
$original_by_ref_callback,
$this->callback_reflection->get_unwrapped_callback( $original_by_ref_callback )
);

$wrapped_by_value_callback = new AMP_Validation_Callback_Wrapper(
[
'function' => $original_by_value_callback,
'accepted_args' => 1,
'source' => [],
'indirect_sources' => [],
]
);
$wrapped_by_ref_callback = new AMP_Validation_Callback_Wrapper(
[
'function' => $original_by_ref_callback,
'accepted_args' => 1,
'source' => [],
'indirect_sources' => [],
]
);

$this->assertSame(
$original_by_value_callback,
$this->callback_reflection->get_unwrapped_callback( $wrapped_by_value_callback )
);
$this->assertSame(
$original_by_ref_callback,
$this->callback_reflection->get_unwrapped_callback( $wrapped_by_ref_callback )
);
$this->assertSame(
$original_by_ref_callback,
$this->callback_reflection->get_unwrapped_callback( [ $wrapped_by_ref_callback, 'invoke_with_first_ref_arg' ] )
);

$rewrapped_by_value_callback = new AMP_Validation_Callback_Wrapper(
[
'function' => $wrapped_by_value_callback,
'accepted_args' => 1,
'source' => [],
'indirect_sources' => [],
]
);
$rewrapped_by_ref_callback = new AMP_Validation_Callback_Wrapper(
[
'function' => $wrapped_by_ref_callback,
'accepted_args' => 1,
'source' => [],
'indirect_sources' => [],
]
);

$this->assertSame(
$original_by_value_callback,
$this->callback_reflection->get_unwrapped_callback( $rewrapped_by_value_callback )
);
$this->assertSame(
$original_by_ref_callback,
$this->callback_reflection->get_unwrapped_callback( $rewrapped_by_ref_callback )
);
$this->assertSame(
$original_by_ref_callback,
$this->callback_reflection->get_unwrapped_callback( [ $rewrapped_by_ref_callback, 'invoke_with_first_ref_arg' ] )
);

$this->assertSame( 2, $original_by_value_callback( 1 ) );
$this->assertSame( 2, $wrapped_by_value_callback( 1 ) );
$this->assertSame( 2, $rewrapped_by_value_callback( 1 ) );

$number = 1;
$number_ref = &$number;
$original_by_ref_callback( $number_ref );
$this->assertSame( 2, $number );
$wrapped_by_ref_callback->invoke_with_first_ref_arg( $number_ref );
$this->assertSame( 3, $number );
$rewrapped_by_ref_callback->invoke_with_first_ref_arg( $number_ref );
$this->assertSame( 4, $number );
}

/** @return array */
public function data_get_source() {
require_once ABSPATH . '/wp-admin/includes/widgets.php';
Expand Down
Loading

0 comments on commit a5f6909

Please sign in to comment.