Plugins: Prevent hook callback IDs from being cast to integer array keys

src/wp-includes/class-wp-hook.php

@@ -20,8 +20,8 @@
  *   accepted_args: int,
  * }
  *
- * @phpstan-implements Iterator<int, array<string, Hook_Callback>>
- * @phpstan-implements ArrayAccess<int, array<string, Hook_Callback>>
+ * @phpstan-implements Iterator<int, array<non-decimal-int-string, Hook_Callback>>
+ * @phpstan-implements ArrayAccess<int, array<non-decimal-int-string, Hook_Callback>>
  */
 #[AllowDynamicProperties]
 final class WP_Hook implements Iterator, ArrayAccess {
@@ -31,7 +31,7 @@ final class WP_Hook implements Iterator, ArrayAccess {
 	 *
 	 * @since 4.7.0
 	 * @var array
-	 * @phpstan-var array<int, array<string, Hook_Callback>>
+	 * @phpstan-var array<int, array<non-decimal-int-string, Hook_Callback>>
 	 */
 	public $callbacks = array();
 
@@ -502,7 +502,7 @@ public function offsetExists( $offset ) {
 	 *
 	 * @param int $offset The offset to retrieve.
 	 * @return array|null If set, the value at the specified offset, null otherwise.
-	 * @phpstan-return array<string, Hook_Callback>|null
+	 * @phpstan-return array<non-decimal-int-string, Hook_Callback>|null
 	 */
 	#[ReturnTypeWillChange]
 	public function offsetGet( $offset ) {
@@ -518,7 +518,7 @@ public function offsetGet( $offset ) {
 	 *
 	 * @param int|null $offset The offset to assign the value to.
 	 * @param array    $value The value to set.
-	 * @phpstan-param array<string, Hook_Callback> $value
+	 * @phpstan-param array<non-decimal-int-string, Hook_Callback> $value
 	 */
 	#[ReturnTypeWillChange]
 	public function offsetSet( $offset, $value ) {
@@ -554,7 +554,7 @@ public function offsetUnset( $offset ) {
 	 * @link https://www.php.net/manual/en/iterator.current.php
 	 *
 	 * @return array|false Array of callbacks at current priority, false if there are no more elements.
-	 * @phpstan-return array<string, Hook_Callback>|false
+	 * @phpstan-return array<non-decimal-int-string, Hook_Callback>|false
 	 */
 	#[ReturnTypeWillChange]
 	public function current() {
@@ -569,7 +569,7 @@ public function current() {
 	 * @link https://www.php.net/manual/en/iterator.next.php
 	 *
 	 * @return array|false Array of callbacks at next priority, false if there are no more elements.
-	 * @phpstan-return array<string, Hook_Callback>|false
+	 * @phpstan-return array<non-decimal-int-string, Hook_Callback>|false
 	 */
 	#[ReturnTypeWillChange]
 	public function next() {

src/wp-includes/class-wp-widget-factory.php

@@ -16,11 +16,25 @@
 #[AllowDynamicProperties]
 class WP_Widget_Factory {
 
+	/**
+	 * Prefix for the key under which a widget registered as an instance is stored.
+	 *
+	 * Without it the key would be the decimal representation of an integer, which PHP casts from
+	 * string to int when it is used as an array key.
+	 *
+	 * @since 7.1.1
+	 */
+	private const INSTANCE_KEY_PREFIX = 'spl_object_id:';
+
 	/**
 	 * Widgets array.
 	 *
+	 * Keyed by class name for a widget registered by name, and by a prefixed object ID for a widget
+	 * registered as an instance.
+	 *
 	 * @since 2.8.0
-	 * @var array
+	 * @var array<string, WP_Widget>
+	 * @phpstan-var array<non-decimal-int-string, WP_Widget>
 	 */
 	public $widgets = array();
 
@@ -52,12 +66,13 @@ public function WP_Widget_Factory() {
 	 * @since 2.8.0
 	 * @since 4.6.0 Updated the `$widget` parameter to also accept a WP_Widget instance object
 	 *              instead of simply a `WP_Widget` subclass name.
+	 * @since 7.1.1 The key for an instance is prefixed so that it is never cast to an integer.
 	 *
 	 * @param string|WP_Widget $widget Either the name of a `WP_Widget` subclass or an instance of a `WP_Widget` subclass.
 	 */
 	public function register( $widget ) {
 		if ( $widget instanceof WP_Widget ) {
-			$this->widgets[ spl_object_id( $widget ) ] = $widget;
+			$this->widgets[ self::INSTANCE_KEY_PREFIX . spl_object_id( $widget ) ] = $widget;
 		} else {
 			$this->widgets[ $widget ] = new $widget();
 		}
@@ -69,12 +84,13 @@ public function register( $widget ) {
 	 * @since 2.8.0
 	 * @since 4.6.0 Updated the `$widget` parameter to also accept a WP_Widget instance object
 	 *              instead of simply a `WP_Widget` subclass name.
+	 * @since 7.1.1 The key for an instance is prefixed so that it is never cast to an integer.
 	 *
 	 * @param string|WP_Widget $widget Either the name of a `WP_Widget` subclass or an instance of a `WP_Widget` subclass.
 	 */
 	public function unregister( $widget ) {
 		if ( $widget instanceof WP_Widget ) {
-			unset( $this->widgets[ spl_object_id( $widget ) ] );
+			unset( $this->widgets[ self::INSTANCE_KEY_PREFIX . spl_object_id( $widget ) ] );
 		} else {
 			unset( $this->widgets[ $widget ] );
 		}

src/wp-includes/plugin.php

@@ -989,6 +989,7 @@ function _wp_call_all_hook( $args ) {
  *              and no longer returns false, but can still return void for invalid callbacks.
  * @since 6.9.0 Returns explicit null if an invalid callback is supplied.
  * @since 7.1.0 Uses spl_object_id() instead of spl_object_hash() for performance.
+ * @since 7.1.1 The ID for an object callback is prefixed so that it is never cast to an integer array key.
  *
  * @access private
  *
@@ -998,14 +999,21 @@ function _wp_call_all_hook( $args ) {
  * @param int      $priority  Unused. The order in which the functions
  *                            associated with a particular action are executed.
  * @return string|null Unique function ID for usage as array key, or null if it couldn't be determined.
+ *
+ * @phpstan-return non-decimal-int-string|null
  */
 function _wp_filter_build_unique_id( $hook_name, $callback, $priority ): ?string {
 	if ( is_string( $callback ) ) {
 		return $callback;
 	}
 
 	if ( is_object( $callback ) ) {
-		return (string) spl_object_id( $callback );
+		/*
+		 * The prefix keeps the ID from being the decimal representation of an integer. PHP casts such a
+		 * string to int when it is used as an array key, which would change the type of the keys in
+		 * WP_Hook::$callbacks and break consumers that pass them to string functions.
+		 */
+		return 'spl_object_id:' . spl_object_id( $callback );
 	}
 
 	if ( ! isset( $callback[1] ) || ! is_string( $callback[1] ) ) {

tests/phpunit/tests/hooks/buildUniqueId.php

@@ -8,29 +8,63 @@
  */
 class Tests_Hooks_BuildUniqueId extends WP_UnitTestCase {
 
+	public static function set_up_before_class() {
+		parent::set_up_before_class();
+
+		require_once __DIR__ . '/../../includes/mock-invokable.php';
+	}
+
+	/**
+	 * @ticket 58291
+	 * @ticket 65919
+	 */
 	public function test_string_callback_returns_string(): void {
 		$result = _wp_filter_build_unique_id( '', '__return_null', 10 );
-		$this->assertIsString( $result );
+		$this->assertIsNonDecimalIntString( $result );
 		$this->assertSame( '__return_null', $result );
 	}
 
+	/**
+	 * @ticket 58291
+	 * @ticket 65919
+	 */
 	public function test_closure_returns_string(): void {
 		$cb     = function (): void {};
 		$result = _wp_filter_build_unique_id( '', $cb, 10 );
-		$this->assertIsString( $result );
+		$this->assertIsNonDecimalIntString( $result );
+	}
+
+	/**
+	 * @ticket 58291
+	 * @ticket 65919
+	 */
+	public function test_invokable_object_returns_string(): void {
+		$result = _wp_filter_build_unique_id( '', new Mock_Invokable(), 10 );
+		$this->assertIsNonDecimalIntString( $result );
 	}
 
+	/**
+	 * @ticket 58291
+	 * @ticket 65919
+	 */
 	public function test_object_callback_returns_string(): void {
 		$a      = new MockAction();
 		$result = _wp_filter_build_unique_id( '', array( $a, 'action' ), 10 );
-		$this->assertIsString( $result );
+		$this->assertIsNonDecimalIntString( $result );
 	}
 
+	/**
+	 * @ticket 58291
+	 * @ticket 65919
+	 */
 	public function test_static_callback_returns_string(): void {
 		$result = _wp_filter_build_unique_id( '', array( 'MockAction', 'action' ), 10 );
-		$this->assertIsString( $result );
+		$this->assertIsNonDecimalIntString( $result );
 	}
 
+	/**
+	 * @ticket 58291
+	 */
 	public function test_two_different_objects_produce_different_ids(): void {
 		$a = new MockAction();
 		$b = new MockAction();
@@ -40,6 +74,9 @@ public function test_two_different_objects_produce_different_ids(): void {
 		);
 	}
 
+	/**
+	 * @ticket 58291
+	 */
 	public function test_same_object_produces_same_id(): void {
 		$a = new MockAction();
 		$this->assertSame(
@@ -48,15 +85,42 @@ public function test_same_object_produces_same_id(): void {
 		);
 	}
 
+	/**
+	 * @ticket 58291
+	 */
 	public function test_malformed_array_missing_method_returns_null(): void {
 		$a      = new MockAction();
 		$result = _wp_filter_build_unique_id( '', array( $a ), 10 );
 		$this->assertNull( $result );
 	}
 
+	/**
+	 * @ticket 58291
+	 */
 	public function test_malformed_array_non_string_method_returns_null(): void {
 		$a      = new MockAction();
 		$result = _wp_filter_build_unique_id( '', array( $a, 123 ), 10 );
 		$this->assertNull( $result );
 	}
+
+	/**
+	 * Asserts that a value is a string which PHP does not cast to an integer when used as an array key.
+	 *
+	 * The return value of _wp_filter_build_unique_id() is used as the callback key in WP_Hook::$callbacks.
+	 * PHP silently casts an array key from string to int when the string is the canonical decimal
+	 * representation of an integer, which changes the type of the keys that consumers of that public
+	 * property read back. This is the runtime equivalent of PHPStan's `non-decimal-int-string` type.
+	 *
+	 * @param mixed $value Value to check.
+	 */
+	private function assertIsNonDecimalIntString( $value ): void {
+		$this->assertIsString( $value, 'The unique ID is not a string.' );
+
+		$array = array( $value => true );
+
+		$this->assertIsString(
+			array_key_first( $array ),
+			sprintf( 'The unique ID "%s" was cast to an integer when used as an array key.', $value )
+		);
+	}
 }

tests/phpunit/tests/widgets.php

@@ -45,6 +45,56 @@ public function test_register_and_unregister_widget_core_widget() {
 		$this->assertArrayNotHasKey( $widget_class, $wp_widget_factory->widgets );
 	}
 
+	/**
+	 * Tests that a widget registered as an instance is keyed by a string.
+	 *
+	 * WP_Widget_Factory::$widgets is public, and its keys were strings in every release before the
+	 * object ID was introduced. PHP casts an array key from string to int whenever the string is the
+	 * canonical decimal representation of an integer, so a bare spl_object_id() value would change
+	 * the type of the keys that consumers of that property read back.
+	 *
+	 * @see register_widget()
+	 * @ticket 65919
+	 *
+	 * @global WP_Widget_Factory $wp_widget_factory
+	 */
+	public function test_register_widget_instance_is_keyed_by_string() {
+		global $wp_widget_factory;
+
+		register_widget( new WP_Widget_Search() );
+
+		$this->assertCount( 1, $wp_widget_factory->widgets );
+		$this->assertIsString( array_key_first( $wp_widget_factory->widgets ) );
+	}
+
+	/**
+	 * Tests that the key returned for a widget registered as an instance is a string.
+	 *
+	 * WP_Widget_Factory::get_widget_key() is documented as returning a string, and core passes what
+	 * it returns to the_widget(), which in turn passes it to the 'the_widget' action, both of which
+	 * document the value as a string.
+	 *
+	 * @see WP_Widget_Factory::get_widget_key()
+	 * @ticket 65919
+	 *
+	 * @global WP_Widget_Factory $wp_widget_factory
+	 */
+	public function test_get_widget_key_for_instance_returns_string() {
+		global $wp_widget_factory;
+
+		$widget                              = new WP_Widget_Search();
+		$widget->id_base                     = 'better_search';
+		$widget->name                        = 'Better Search';
+		$widget->option_name                 = 'widget_' . $widget->id_base;
+		$widget->widget_options['classname'] = 'widget_' . $widget->id_base;
+		$widget->control_options['id_base']  = $widget->id_base;
+
+		register_widget( $widget );
+
+		$this->assertIsString( $wp_widget_factory->get_widget_key( 'better_search' ) );
+		$this->assertSame( $widget, $wp_widget_factory->get_widget_object( 'better_search' ) );
+	}
+
 	/**
 	 * Test that registering a widget class and registering a widget instance work together.
 	 *