Changeset 65250

Timestamp:

11/03/2026 08:34:25 PM (15 hours ago)

Author:

jorbin

Message:

Rewrites: Avoid fatal errors when flushing rules for random content.

wp_random_content_flush_rewrite_rules_for_post() runs on the wp_after_insert_post and after_delete_post actions, which third-party code may fire with a missing or incorrectly typed $post. Also make the $post parameter of _update_posts_count_on_delete() optional for the same single-argument after_delete_post calls.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/14356

Follow-up to r64402.

Props jorbin, bradshawtm, therssoftware.
Fixes #64498.

Location:

trunk

Files:

• src/wp-includes/ms-blogs.php (modified) (1 diff)
• src/wp-includes/query.php (modified) (3 diffs)
• tests/phpunit/tests/query/randomContentRedirect.php (modified) (2 diffs)

trunk/src/wp-includes/ms-blogs.php

@@ -890 +890 @@
  * @since 4.0.0
  * @since 6.2.0 Added the `$post` parameter.
- *
- * @param int     $post_id Post ID.
- * @param WP_Post $post    Post object.
- */
-function _update_posts_count_on_delete( $post_id, $post ) {
-        if ( ! $post || 'publish' !== $post->post_status || 'post' !== $post->post_type ) {
+ * @since 7.2.0 The `$post` parameter isn't always guaranteed to be provided.
+ *
+ * @param int          $post_id Post ID.
+ * @param WP_Post|null $post    Optional. Post object. Default null.
+ */
+function _update_posts_count_on_delete( $post_id, $post = null ) {
+        if ( ! $post instanceof WP_Post || 'publish' !== $post->post_status || 'post' !== $post->post_type ) {
                 return;
         }

trunk/src/wp-includes/query.php

@@ -1514 +1514 @@
  *
  * Runs on the {@see 'wp_after_insert_post'} and {@see 'after_delete_post'} actions.
+ * Third-party code may fire these actions with unexpected values, so the parameters are
+ * not type declared. When `$post` is omitted it is looked up from `$post_id`. Passing
+ * anything other than a `WP_Post` as `$post`, or a post that cannot be looked up, is
+ * incorrect usage and nothing is flushed. `$post_before` is only considered on
+ * `wp_after_insert_post`, where passing anything other than a `WP_Post` or null is
+ * incorrect usage and the value is ignored.
  *
  * @since 7.2.0
@@ -1521 +1527 @@
  *
  * @param int          $post_id     Post ID.
- * @param WP_Post      $post        Post object.
+ * @param WP_Post|null $post        Optional. Post object. Looked up from `$post_id` when null. Default null.
  * @param bool         $update      Optional. Whether this is an existing post being updated. Default false.
  * @param WP_Post|null $post_before Optional. Post object before the update, null for new posts. Default null.
  */
-function wp_random_content_flush_rewrite_rules_for_post( int $post_id, WP_Post $post, bool $update = false, ?WP_Post $post_before = null ): void {
+function wp_random_content_flush_rewrite_rules_for_post( $post_id, $post = null, $update = false, $post_before = null ): void {
         global $wp_rewrite;
+
+        // Only look up a real ID: get_post() falls back to the global post for an empty value.
+        if ( null === $post && (int) $post_id > 0 ) {
+                $post = get_post( (int) $post_id );
+        }
+
+        if ( ! $post instanceof WP_Post ) {
+                _doing_it_wrong(
+                        __FUNCTION__,
+                        sprintf(
+                                /* translators: 1: $post, 2: WP_Post */
+                                __( 'The %1$s argument must be a %2$s object.' ),
+                                '<code>$post</code>',
+                                '<code>WP_Post</code>'
+                        ),
+                        '7.2.0'
+                );
+                return;
+        }
+
+        $posts = array( $post );
+
+        // Only wp_after_insert_post passes the previous post object, which matters when it was renamed or moved.
+        if ( 'wp_after_insert_post' === current_action() ) {
+                if ( $post_before instanceof WP_Post ) {
+                        $posts[] = $post_before;
+                } elseif ( null !== $post_before ) {
+                        _doing_it_wrong(
+                                __FUNCTION__,
+                                sprintf(
+                                        /* translators: 1: $post_before, 2: WP_Post */
+                                        __( 'The %1$s argument must be a %2$s object or null.' ),
+                                        '<code>$post_before</code>',
+                                        '<code>WP_Post</code>'
+                                ),
+                                '7.2.0'
+                        );
+                }
+        }
 
         if ( ! $wp_rewrite->using_permalinks() || ! wp_is_random_content_redirect_enabled() ) {
@@ -1532 +1577 @@
         }
 
-        foreach ( array( $post, $post_before ) as $page ) {
+        foreach ( $posts as $page ) {
                 if (
-                        $page instanceof WP_Post
-                        && $wp_rewrite->get_random_base() === $page->post_name
+                        $wp_rewrite->get_random_base() === $page->post_name
                         && (
                                 ( 'page' === $page->post_type && 0 === (int) $page->post_parent )

trunk/tests/phpunit/tests/query/randomContentRedirect.php

@@ -250 +250 @@
                         )
                 );
+        }
+
+        /**
+         * Creates a published post and returns the post object.
+         *
+         * @param array<string, mixed> $args Optional. Post arguments.
+         * @return WP_Post Post object.
+         */
+        protected function create_post_object( array $args = array() ): WP_Post {
+                $post = get_post( $this->create_post( $args ) );
+
+                $this->assertInstanceOf( WP_Post::class, $post );
+
+                return $post;
         }
 
@@ -1407 +1421 @@
         }
 
+        /**
+         * @dataProvider data_invalid_arguments
+         *
+         * @param array<int, mixed> $args The arguments to call the function with.
+         */
+        public function test_invalid_arguments_do_not_flush_rewrite_rules( array $args ): void {
+                // The data provider cannot query the database, so PHP_INT_MAX stands in for a nonexistent post ID.
+                $args = array_map(
+                        function ( $arg ) {
+                                return PHP_INT_MAX === $arg ? $this->get_nonexistent_post_id() : $arg;
+                        },
+                        $args
+                );
+
+                $this->setExpectedIncorrectUsage( 'wp_random_content_flush_rewrite_rules_for_post' );
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                wp_random_content_flush_rewrite_rules_for_post( ...$args ); // @phpstan-ignore argument.type (Intentionally passing invalid values.)
+
+                $this->assertSame( 0, $flushes->get_call_count(), 'Invalid arguments should not flush the rewrite rules.' );
+        }
+
+        /**
+         * Data provider.
+         *
+         * Core never fires the `wp_after_insert_post` and `after_delete_post` actions with invalid
+         * arguments, but third-party code can. Doing so is incorrect usage and must not cause a
+         * fatal error.
+         *
+         * `PHP_INT_MAX` is replaced with a post ID that is verified not to exist when the test runs.
+         *
+         * @return array<string, array{array<int, mixed>}>
+         */
+        public function data_invalid_arguments(): array {
+                return array(
+                        'nulls'                              => array( array( null, null, false, null ) ),
+                        'strings'                            => array( array( 'random', 'random', 'yes', 'random' ) ),
+                        'arrays'                             => array( array( array(), array( 'post_name' => 'random' ), 1, array() ) ),
+                        'nulls without optional values'      => array( array( null, null ) ),
+                        'strings without optional values'    => array( array( 'random', 'random' ) ),
+                        'post ID of zero without a post'     => array( array( 0 ) ),
+                        'nonexistent post ID without a post' => array( array( PHP_INT_MAX ) ),
+                );
+        }
+
+        /**
+         * Returns a post ID that does not exist.
+         *
+         * Starts at `PHP_INT_MAX` and decrements until no post is found.
+         *
+         * @return int Post ID.
+         */
+        protected function get_nonexistent_post_id(): int {
+                $post_id = PHP_INT_MAX;
+
+                while ( get_post( $post_id ) instanceof WP_Post ) {
+                        --$post_id;
+                }
+
+                return $post_id;
+        }
+
+        /**
+         * Tests that an omitted post object does not fall back to the global post.
+         *
+         * `get_post()` returns the global post for an empty value, which would flush the
+         * rewrite rules for whatever post is being displayed rather than the one passed in.
+         */
+        public function test_omitted_post_does_not_fall_back_to_the_global_post(): void {
+                $this->setExpectedIncorrectUsage( 'wp_random_content_flush_rewrite_rules_for_post' );
+
+                $GLOBALS['post'] = $this->create_post_object(
+                        array(
+                                'post_type' => 'page',
+                                'post_name' => 'random',
+                        )
+                );
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                wp_random_content_flush_rewrite_rules_for_post( null ); // @phpstan-ignore argument.type (Intentionally omitting the post ID.)
+
+                $this->assertSame( 0, $flushes->get_call_count(), 'An omitted post should not be looked up from the global post.' );
+        }
+
+        /**
+         * Tests that an omitted post object is looked up from the post ID.
+         *
+         * The `$post` parameter was added to `after_delete_post` in 5.5.0, but third-party
+         * code written against the original signature may still fire the action with only
+         * the post ID.
+         */
+        public function test_omitted_post_is_looked_up_from_the_post_id(): void {
+                $page_id = $this->create_post(
+                        array(
+                                'post_type' => 'page',
+                                'post_name' => 'random',
+                        )
+                );
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                wp_random_content_flush_rewrite_rules_for_post( $page_id );
+
+                $this->assertSame( 1, $flushes->get_call_count(), 'A page at the random base should flush the rewrite rules when only its ID is passed.' );
+        }
+
+        /**
+         * Tests that firing `after_delete_post` with only a post ID does not cause a fatal error.
+         *
+         * As the post is already deleted, it cannot be looked up from the ID, so the call is
+         * incorrect usage and the rewrite rules are left alone.
+         *
+         * @covers ::_update_posts_count_on_delete
+         */
+        public function test_after_delete_post_fired_with_only_a_post_id_does_not_flush_rewrite_rules(): void {
+                $post_id = $this->create_post();
+                wp_delete_post( $post_id, true );
+
+                $this->setExpectedIncorrectUsage( 'wp_random_content_flush_rewrite_rules_for_post' );
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                do_action( 'after_delete_post', $post_id ); // @phpstan-ignore WordPress.hookParamCountMismatch (Intentionally omitting the post object.)
+
+                $this->assertSame( 0, $flushes->get_call_count(), 'Firing after_delete_post for a deleted post without the post object should not flush the rewrite rules.' );
+        }
+
+        public function test_page_named_random_still_flushes_rewrite_rules_when_the_other_arguments_are_invalid(): void {
+                $page = $this->create_post_object(
+                        array(
+                                'post_type' => 'page',
+                                'post_name' => 'random',
+                        )
+                );
+
+                $this->setExpectedIncorrectUsage( 'wp_random_content_flush_rewrite_rules_for_post' );
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                do_action( 'wp_after_insert_post', (string) $page->ID, $page, 'yes', false );
+
+                $this->assertSame( 1, $flushes->get_call_count(), 'A page at the random base should flush the rewrite rules even when the other arguments are invalid.' );
+        }
+
+        /**
+         * Tests that the previous post object is only considered on `wp_after_insert_post`.
+         *
+         * `after_delete_post` has no previous post, so a value passed in its place is ignored
+         * rather than reported as incorrect usage.
+         */
+        public function test_post_before_is_ignored_outside_wp_after_insert_post(): void {
+                $page_before = $this->create_post_object(
+                        array(
+                                'post_type' => 'page',
+                                'post_name' => 'random',
+                        )
+                );
+                $post        = $this->create_post_object();
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                wp_random_content_flush_rewrite_rules_for_post( $post->ID, $post, false, 'not a post' ); // @phpstan-ignore argument.type (Intentionally passing an invalid value.)
+                wp_random_content_flush_rewrite_rules_for_post( $post->ID, $post, false, $page_before );
+
+                $this->assertSame( 0, $flushes->get_call_count(), 'The previous post should not be considered outside wp_after_insert_post.' );
+        }
+
+        public function test_renamed_page_at_random_base_flushes_rewrite_rules_via_post_before(): void {
+                $page_before = $this->create_post_object(
+                        array(
+                                'post_type' => 'page',
+                                'post_name' => 'random',
+                        )
+                );
+                $post        = $this->create_post_object();
+
+                $flushes = new MockAction();
+                add_filter( 'pre_update_option_rewrite_rules', array( $flushes, 'filter' ) );
+
+                do_action( 'wp_after_insert_post', $post->ID, $post, true, $page_before );
+
+                $this->assertSame( 1, $flushes->get_call_count(), 'A page previously at the random base should flush the rewrite rules on wp_after_insert_post.' );
+        }
+
         /*
          * Opting out.