Replace hooks after iterating to prevent infinite loop in PHP<=5.4

ampproject · Feb 21, 2018 · 0484bd9 · 0484bd9
1 parent 79856bc
commit 0484bd9
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 31 deletions.
diff --git a/includes/utils/class-amp-validation-utils.php b/includes/utils/class-amp-validation-utils.php
@@ -294,22 +294,32 @@ public static function callback_wrappers() {
 		if ( ! self::do_validate_front_end() ) {
 			return;
 		}
+		$pending_wrap_callbacks = array();
 		foreach ( $wp_filter as $filter_tag => $wp_hook ) {
 			if ( 'query' === $filter_tag ) {
 				continue;
 			}
 			foreach ( $wp_hook->callbacks as $priority => $callbacks ) {
 				foreach ( $callbacks as $callback ) {
-					$function = $callback['function'];
-					$plugin   = self::get_plugin( $function );
+					$plugin = self::get_plugin( $callback['function'] );
 					if ( isset( $plugin ) ) {
-						remove_action( $filter_tag, $function, $priority );
-						$wrapped_callback = self::wrapped_callback( $callback, $plugin );
-						add_action( $filter_tag, $wrapped_callback, $priority, $callback['accepted_args'] );
+						$pending_wrap_callbacks[ $filter_tag ][] = array_merge(
+							$callback,
+							compact( 'plugin', 'priority' )
+						);
 					}
 				}
 			}
 		}
+
+		// Iterate over hooks to replace after iterating over all to begin with to prevent infinite loop in PHP<=5.4.
+		foreach ( $pending_wrap_callbacks as $hook => $callbacks ) {
+			foreach ( $callbacks as $callback ) {
+				remove_action( $hook, $callback['function'], $callback['priority'] );
+				$wrapped_callback = self::wrapped_callback( $callback );
+				add_action( $hook, $wrapped_callback, $callback['priority'], $callback['accepted_args'] );
+			}
+		}
 	}
 
 	/**
@@ -319,29 +329,33 @@ public static function callback_wrappers() {
 	 * @return string|null $plugin   The plugin to which the callback belongs, or null.
 	 */
 	public static function get_plugin( $callback ) {
-		if ( is_string( $callback ) && is_callable( $callback ) ) {
-			// The $callback is a function or static method.
-			$exploded_callback = explode( '::', $callback );
-			if ( count( $exploded_callback ) > 1 ) {
-				$reflection = new ReflectionClass( $exploded_callback[0] );
-			} else {
+		try {
+			if ( is_string( $callback ) && is_callable( $callback ) ) {
+				// The $callback is a function or static method.
+				$exploded_callback = explode( '::', $callback );
+				if ( count( $exploded_callback ) > 1 ) {
+					$reflection = new ReflectionClass( $exploded_callback[0] );
+				} else {
+					$reflection = new ReflectionFunction( $callback );
+				}
+			} elseif ( is_array( $callback ) && isset( $callback[0], $callback[1] ) && method_exists( $callback[0], $callback[1] ) ) {
+				// The $callback is a method.
+				$reflection = new ReflectionClass( $callback[0] );
+			} elseif ( is_object( $callback ) && ( 'Closure' === get_class( $callback ) ) ) {
 				$reflection = new ReflectionFunction( $callback );
 			}
-		} elseif ( is_array( $callback ) && isset( $callback[0], $callback[1] ) && method_exists( $callback[0], $callback[1] ) ) {
-			// The $callback is a method.
-			$reflection = new ReflectionClass( $callback[0] );
-		} elseif ( is_object( $callback ) && ( 'Closure' === get_class( $callback ) ) ) {
-			$reflection = new ReflectionFunction( $callback );
-		}
 
-		$file = isset( $reflection ) ? $reflection->getFileName() : null;
-		if ( ! isset( $file ) ) {
+			$file = isset( $reflection ) ? $reflection->getFileName() : null;
+			if ( ! isset( $file ) ) {
+				return null;
+			}
+			$source_data = self::get_source( $file );
+			if ( 'plugins' === $source_data['type'] ) {
+				return $source_data['source'];
+			}
+		} catch ( Exception $e ) {
 			return null;
 		}
-		$source_data = self::get_source( $file );
-		if ( 'plugins' === $source_data['type'] ) {
-			return $source_data['source'];
-		}
 		return null;
 	}
 
@@ -352,12 +366,17 @@ public static function get_plugin( $callback ) {
 	 * this indicates which plugin it was from.
 	 * The call_user_func_array() logic is mainly copied from WP_Hook:apply_filters().
 	 *
-	 * @param array  $callback The callback data, including values for 'function' and 'accepted_args'.
-	 * @param string $plugin   The plugin where the callback is located.
+	 * @param array $callback {
+	 *     The callback data.
+	 *
+	 *     @type callable $function
+	 *     @type int      $accepted_args
+	 *     @type string   $plugin
+	 * }
 	 * @return closure $wrapped_callback The callback, wrapped in comments.
 	 */
-	public static function wrapped_callback( $callback, $plugin ) {
-		return function() use ( $callback, $plugin ) {
+	public static function wrapped_callback( $callback ) {
+		return function() use ( $callback ) {
 			$function      = $callback['function'];
 			$accepted_args = $callback['accepted_args'];
 			$args          = func_get_args();
@@ -373,9 +392,9 @@ public static function wrapped_callback( $callback, $plugin ) {
 			$output = ob_get_clean();
 
 			if ( ! empty( $output ) ) {
-				printf( '<!--before:%s-->', esc_attr( $plugin ) );
+				printf( '<!--before:%s-->', esc_attr( $callback['plugin'] ) );
 				echo $output; // WPCS: XSS ok.
-				printf( '<!--after:%s-->', esc_attr( $plugin ) );
+				printf( '<!--after:%s-->', esc_attr( $callback['plugin'] ) );
 			}
 			return $result;
 		};

diff --git a/tests/test-class-amp-validation-utils.php b/tests/test-class-amp-validation-utils.php
@@ -456,9 +456,9 @@ public function test_wrapped_callback() {
 		$callback         = array(
 			'function'      => 'the_ID',
 			'accepted_args' => 0,
+			'plugin'        => 'amp',
 		);
-		$plugin           = 'amp';
-		$wrapped_callback = AMP_Validation_Utils::wrapped_callback( $callback, $plugin );
+		$wrapped_callback = AMP_Validation_Utils::wrapped_callback( $callback );
 		$this->assertTrue( $wrapped_callback instanceof Closure );
 		ob_start();
 		call_user_func( $wrapped_callback );