Make WordPress Core

Changeset 63334


Ignore:
Timestamp:
08/21/2026 04:15:34 PM (8 weeks ago)
Author:
westonruter
Message:

Plugins: Prevent object IDs from becoming integer array keys.

To improve performance, r62408 replaced spl_object_hash() with spl_object_id() when building the array keys for WP_Hook::$callbacks and for WP_Widget_Factory::$widgets. Even when the spl_object_id() return value is cast to a string to preserve the previous string array keys, PHP automatically converts such numeric strings to int when assigning the array key. This resulted in a back-compat breakage for plugins that expected to use the array keys in string functions, including possible fatal errors when using strict_types.

To preserve the previous string key behavior, any array keys using spl_object_id() are now prefixed with a string to prevent coercion to int. This not only fixes the expected array key types when iterating over these arrays, but it also fixes the return type for WP_Widget_Factory::get_widget_key() so it actually returns a string.

In addition to the added tests, the non-decimal-int-string type in PHPStan is leveraged instead of a plain string to prevent this regression from returning.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/13209.
Follow-up to r62408, r62733.

Props westonruter, dugi-digitaly, sergeybiryukov.
See #58291.
Fixes #65919.

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/wp-includes/class-wp-hook.php

    r62733 r63334  
    2121 * }
    2222 *
    23  * @phpstan-implements Iterator<int, array<string, Hook_Callback>>
    24  * @phpstan-implements ArrayAccess<int, array<string, Hook_Callback>>
     23 * @phpstan-implements Iterator<int, array<non-decimal-int-string, Hook_Callback>>
     24 * @phpstan-implements ArrayAccess<int, array<non-decimal-int-string, Hook_Callback>>
    2525 */
    2626#[AllowDynamicProperties]
    … …  
    3232         * @since 4.7.0
    3333         * @var array
    34          * @phpstan-var array<int, array<string, Hook_Callback>>
     34         * @phpstan-var array<int, array<non-decimal-int-string, Hook_Callback>>
    3535         */
    3636        public $callbacks = array();
    … …  
    503503         * @param int $offset The offset to retrieve.
    504504         * @return array|null If set, the value at the specified offset, null otherwise.
    505          * @phpstan-return array<string, Hook_Callback>|null
     505         * @phpstan-return array<non-decimal-int-string, Hook_Callback>|null
    506506         */
    507507        #[ReturnTypeWillChange]
    … …  
    519519         * @param int|null $offset The offset to assign the value to.
    520520         * @param array    $value The value to set.
    521          * @phpstan-param array<string, Hook_Callback> $value
     521         * @phpstan-param array<non-decimal-int-string, Hook_Callback> $value
    522522         */
    523523        #[ReturnTypeWillChange]
    … …  
    555555         *
    556556         * @return array|false Array of callbacks at current priority, false if there are no more elements.
    557          * @phpstan-return array<string, Hook_Callback>|false
     557         * @phpstan-return array<non-decimal-int-string, Hook_Callback>|false
    558558         */
    559559        #[ReturnTypeWillChange]
    … …  
    570570         *
    571571         * @return array|false Array of callbacks at next priority, false if there are no more elements.
    572          * @phpstan-return array<string, Hook_Callback>|false
     572         * @phpstan-return array<non-decimal-int-string, Hook_Callback>|false
    573573         */
    574574        #[ReturnTypeWillChange]
  • trunk/src/wp-includes/class-wp-widget-factory.php

    r62408 r63334  
    1818
    1919        /**
     20         * Prefix for the key under which a widget registered as an instance is stored.
     21         *
     22         * Without it the key would be the decimal representation of an integer, which PHP casts from
     23         * string to int when it is used as an array key.
     24         *
     25         * @since 7.1.1
     26         */
     27        private const INSTANCE_KEY_PREFIX = 'spl_object_id:';
     28
     29        /**
    2030         * Widgets array.
    2131         *
     32         * Keyed by class name for a widget registered by name, and by a prefixed object ID for a widget
     33         * registered as an instance.
     34         *
    2235         * @since 2.8.0
    23          * @var array
     36         * @var array<string, WP_Widget>
     37         * @phpstan-var array<non-decimal-int-string, WP_Widget>
    2438         */
    2539        public $widgets = array();
    … …  
    5367         * @since 4.6.0 Updated the `$widget` parameter to also accept a WP_Widget instance object
    5468         *              instead of simply a `WP_Widget` subclass name.
     69         * @since 7.1.1 The key for an instance is prefixed so that it is never cast to an integer.
    5570         *
    5671         * @param string|WP_Widget $widget Either the name of a `WP_Widget` subclass or an instance of a `WP_Widget` subclass.
    … …  
    5873        public function register( $widget ) {
    5974                if ( $widget instanceof WP_Widget ) {
    60                         $this->widgets[ spl_object_id( $widget ) ] = $widget;
     75                        $this->widgets[ self::INSTANCE_KEY_PREFIX . spl_object_id( $widget ) ] = $widget;
    6176                } else {
    6277                        $this->widgets[ $widget ] = new $widget();
    … …  
    7085         * @since 4.6.0 Updated the `$widget` parameter to also accept a WP_Widget instance object
    7186         *              instead of simply a `WP_Widget` subclass name.
     87         * @since 7.1.1 The key for an instance is prefixed so that it is never cast to an integer.
    7288         *
    7389         * @param string|WP_Widget $widget Either the name of a `WP_Widget` subclass or an instance of a `WP_Widget` subclass.
    … …  
    7591        public function unregister( $widget ) {
    7692                if ( $widget instanceof WP_Widget ) {
    77                         unset( $this->widgets[ spl_object_id( $widget ) ] );
     93                        unset( $this->widgets[ self::INSTANCE_KEY_PREFIX . spl_object_id( $widget ) ] );
    7894                } else {
    7995                        unset( $this->widgets[ $widget ] );
  • trunk/src/wp-includes/plugin.php

    r62842 r63334  
    990990 * @since 6.9.0 Returns explicit null if an invalid callback is supplied.
    991991 * @since 7.1.0 Uses spl_object_id() instead of spl_object_hash() for performance.
     992 * @since 7.1.1 The ID for an object callback is prefixed so that it is never cast to an integer array key.
    992993 *
    993994 * @access private
    … …  
    9991000 *                            associated with a particular action are executed.
    10001001 * @return string|null Unique function ID for usage as array key, or null if it couldn't be determined.
     1002 *
     1003 * @phpstan-return non-decimal-int-string|null
    10011004 */
    10021005function _wp_filter_build_unique_id( $hook_name, $callback, $priority ): ?string {
    … …  
    10061009
    10071010        if ( is_object( $callback ) ) {
    1008                 return (string) spl_object_id( $callback );
     1011                /*
     1012                 * The prefix keeps the ID from being the decimal representation of an integer. PHP casts such a
     1013                 * string to int when it is used as an array key, which would change the type of the keys in
     1014                 * WP_Hook::$callbacks and break consumers that pass them to string functions.
     1015                 */
     1016                return 'spl_object_id:' . spl_object_id( $callback );
    10091017        }
    10101018
  • trunk/tests/phpunit/tests/hooks/buildUniqueId.php

    r62408 r63334  
    99class Tests_Hooks_BuildUniqueId extends WP_UnitTestCase {
    1010
     11        public static function set_up_before_class() {
     12                parent::set_up_before_class();
     13
     14                require_once __DIR__ . '/../../includes/mock-invokable.php';
     15        }
     16
     17        /**
     18         * @ticket 58291
     19         * @ticket 65919
     20         */
    1121        public function test_string_callback_returns_string(): void {
    1222                $result = _wp_filter_build_unique_id( '', '__return_null', 10 );
    13                 $this->assertIsString( $result );
     23                $this->assertIsNonDecimalIntString( $result );
    1424                $this->assertSame( '__return_null', $result );
    1525        }
    1626
     27        /**
     28         * @ticket 58291
     29         * @ticket 65919
     30         */
    1731        public function test_closure_returns_string(): void {
    1832                $cb     = function (): void {};
    1933                $result = _wp_filter_build_unique_id( '', $cb, 10 );
    20                 $this->assertIsString( $result );
     34                $this->assertIsNonDecimalIntString( $result );
    2135        }
    2236
     37        /**
     38         * @ticket 58291
     39         * @ticket 65919
     40         */
     41        public function test_invokable_object_returns_string(): void {
     42                $result = _wp_filter_build_unique_id( '', new Mock_Invokable(), 10 );
     43                $this->assertIsNonDecimalIntString( $result );
     44        }
     45
     46        /**
     47         * @ticket 58291
     48         * @ticket 65919
     49         */
    2350        public function test_object_callback_returns_string(): void {
    2451                $a      = new MockAction();
    2552                $result = _wp_filter_build_unique_id( '', array( $a, 'action' ), 10 );
    26                 $this->assertIsString( $result );
     53                $this->assertIsNonDecimalIntString( $result );
    2754        }
    2855
     56        /**
     57         * @ticket 58291
     58         * @ticket 65919
     59         */
    2960        public function test_static_callback_returns_string(): void {
    3061                $result = _wp_filter_build_unique_id( '', array( 'MockAction', 'action' ), 10 );
    31                 $this->assertIsString( $result );
     62                $this->assertIsNonDecimalIntString( $result );
    3263        }
    3364
     65        /**
     66         * @ticket 58291
     67         */
    3468        public function test_two_different_objects_produce_different_ids(): void {
    3569                $a = new MockAction();
    … …  
    4175        }
    4276
     77        /**
     78         * @ticket 58291
     79         */
    4380        public function test_same_object_produces_same_id(): void {
    4481                $a = new MockAction();
    … …  
    4986        }
    5087
     88        /**
     89         * @ticket 58291
     90         */
    5191        public function test_malformed_array_missing_method_returns_null(): void {
    5292                $a      = new MockAction();
    … …  
    5595        }
    5696
     97        /**
     98         * @ticket 58291
     99         */
    57100        public function test_malformed_array_non_string_method_returns_null(): void {
    58101                $a      = new MockAction();
    … …  
    60103                $this->assertNull( $result );
    61104        }
     105
     106        /**
     107         * Asserts that a value is a string which PHP does not cast to an integer when used as an array key.
     108         *
     109         * The return value of _wp_filter_build_unique_id() is used as the callback key in WP_Hook::$callbacks.
     110         * PHP silently casts an array key from string to int when the string is the canonical decimal
     111         * representation of an integer, which changes the type of the keys that consumers of that public
     112         * property read back. This is the runtime equivalent of PHPStan's `non-decimal-int-string` type.
     113         *
     114         * @param mixed $value Value to check.
     115         */
     116        private function assertIsNonDecimalIntString( $value ): void {
     117                $this->assertIsString( $value, 'The unique ID is not a string.' );
     118
     119                $array = array( $value => true );
     120
     121                $this->assertIsString(
     122                        array_key_first( $array ),
     123                        sprintf( 'The unique ID "%s" was cast to an integer when used as an array key.', $value )
     124                );
     125        }
    62126}
  • trunk/tests/phpunit/tests/widgets.php

    r61466 r63334  
    4444                unregister_widget( $widget_class );
    4545                $this->assertArrayNotHasKey( $widget_class, $wp_widget_factory->widgets );
     46        }
     47
     48        /**
     49         * Tests that a widget registered as an instance is keyed by a string.
     50         *
     51         * WP_Widget_Factory::$widgets is public, and its keys were strings in every release before the
     52         * object ID was introduced. PHP casts an array key from string to int whenever the string is the
     53         * canonical decimal representation of an integer, so a bare spl_object_id() value would change
     54         * the type of the keys that consumers of that property read back.
     55         *
     56         * @see register_widget()
     57         * @ticket 65919
     58         *
     59         * @global WP_Widget_Factory $wp_widget_factory
     60         */
     61        public function test_register_widget_instance_is_keyed_by_string() {
     62                global $wp_widget_factory;
     63
     64                register_widget( new WP_Widget_Search() );
     65
     66                $this->assertCount( 1, $wp_widget_factory->widgets );
     67                $this->assertIsString( array_key_first( $wp_widget_factory->widgets ) );
     68        }
     69
     70        /**
     71         * Tests that the key returned for a widget registered as an instance is a string.
     72         *
     73         * WP_Widget_Factory::get_widget_key() is documented as returning a string, and core passes what
     74         * it returns to the_widget(), which in turn passes it to the 'the_widget' action, both of which
     75         * document the value as a string.
     76         *
     77         * @see WP_Widget_Factory::get_widget_key()
     78         * @ticket 65919
     79         *
     80         * @global WP_Widget_Factory $wp_widget_factory
     81         */
     82        public function test_get_widget_key_for_instance_returns_string() {
     83                global $wp_widget_factory;
     84
     85                $widget                              = new WP_Widget_Search();
     86                $widget->id_base                     = 'better_search';
     87                $widget->name                        = 'Better Search';
     88                $widget->option_name                 = 'widget_' . $widget->id_base;
     89                $widget->widget_options['classname'] = 'widget_' . $widget->id_base;
     90                $widget->control_options['id_base']  = $widget->id_base;
     91
     92                register_widget( $widget );
     93
     94                $this->assertIsString( $wp_widget_factory->get_widget_key( 'better_search' ) );
     95                $this->assertSame( $widget, $wp_widget_factory->get_widget_object( 'better_search' ) );
    4696        }
    4797
Note: See TracChangeset for help on using the changeset viewer.