diff --git a/src/wp-includes/canonical.php b/src/wp-includes/canonical.php index 2fb3d74bb9c3e..5ec43f99c498c 100644 --- a/src/wp-includes/canonical.php +++ b/src/wp-includes/canonical.php @@ -28,11 +28,12 @@ * * @since 2.3.0 * - * @global WP_Rewrite $wp_rewrite WordPress rewrite component. + * @global WP_Rewrite $wp_rewrite WordPress rewrite component. * @global bool $is_IIS - * @global WP_Query $wp_query WordPress Query object. - * @global wpdb $wpdb WordPress database abstraction object. - * @global WP $wp Current WordPress environment instance. + * @global WP_Query $wp_query WordPress Query object. + * @global WP_Query $wp_the_query WordPress main Query object. + * @global wpdb $wpdb WordPress database abstraction object. + * @global WP $wp Current WordPress environment instance. * * @param string $requested_url Optional. The URL that was requested, used to * figure if redirect is needed. @@ -41,12 +42,24 @@ * @phpstan-return ( $do_redirect is true ? null : string|null ) */ function redirect_canonical( $requested_url = null, $do_redirect = true ) { - global $wp_rewrite, $is_IIS, $wp_query, $wpdb, $wp; + global $wp_rewrite, $is_IIS, $wp_query, $wp_the_query, $wpdb, $wp; if ( isset( $_SERVER['REQUEST_METHOD'] ) && ! in_array( strtoupper( $_SERVER['REQUEST_METHOD'] ), array( 'GET', 'HEAD' ), true ) ) { return null; } + /* + * Random content requests with a selected post are redirected to that post by + * wp_random_content_redirect() later on the `template_redirect` action. A canonical + * redirect would redirect the request twice and may drop the `random` query variable. + */ + if ( $wp_the_query instanceof WP_Query + && wp_is_random_content_query( $wp_the_query ) + && $wp_the_query->post instanceof WP_Post + ) { + return null; + } + /* * If we're not in wp-admin and the post has been published and preview nonce * is non-existent or invalid then no need for preview in query. diff --git a/src/wp-includes/class-wp.php b/src/wp-includes/class-wp.php index 1907844e95ef4..e51614e9020ad 100644 --- a/src/wp-includes/class-wp.php +++ b/src/wp-includes/class-wp.php @@ -719,10 +719,12 @@ public function query_posts() { * - {@see wp_random_content_posts_orderby()} on the {@see 'posts_orderby'} filter. * - {@see wp_random_content_post_limits()} on the {@see 'post_limits'} filter. * - * If no post is selected, for example on an empty archive, the request is a 404 as - * there is no post to redirect to. If a post is selected but the request is not - * redirected, the request continues with the results of the main query. The main - * query is not run again. + * If a post is selected, the redirect is sent by {@see wp_random_content_redirect()} + * on the {@see 'template_redirect'} action at priority 100, so the request is set up + * and the {@see 'wp'} action has fired before the redirect. If no post is selected, + * for example on an empty archive, the request is a 404 as there is no post to + * redirect to. If a post is selected but the request is not redirected, the request + * continues with the results of the main query. The main query is not run again. * * @since 7.2.0 * @@ -748,9 +750,13 @@ public function handle_random(): bool { remove_filter( 'posts_orderby', 'wp_random_content_posts_orderby', 20 ); remove_filter( 'post_limits', 'wp_random_content_post_limits', 20 ); - wp_random_content_redirect(); + if ( ! wp_is_random_content_query( $wp_the_query ) ) { + return true; + } - if ( wp_is_random_content_query( $wp_the_query ) && ! $wp_the_query->post instanceof WP_Post ) { + if ( $wp_the_query->post instanceof WP_Post ) { + add_action( 'template_redirect', 'wp_random_content_redirect', 100 ); + } else { $wp_the_query->set_404(); status_header( 404 ); nocache_headers(); diff --git a/src/wp-includes/query.php b/src/wp-includes/query.php index 40a9a285c9fbe..7daa3c57c0738 100644 --- a/src/wp-includes/query.php +++ b/src/wp-includes/query.php @@ -1464,27 +1464,35 @@ function wp_random_content_post_limits( $limits, WP_Query $query ): string { /** * Redirects random content requests to the randomly selected post. * - * Called by {@see WP::handle_random()} immediately after the main query selects the - * random post, to avoid unnecessary processing before the redirect. As the redirect - * is sent before {@see WP::send_headers()}, the headers preventing the redirect being - * cached or indexed are sent with it. + * Added to the {@see 'template_redirect'} action at priority 100 by + * {@see WP::handle_random()} once the main query has selected the random post, so the + * redirect is sent after the request is set up and the {@see 'wp'} action has fired. + * The headers preventing the redirect being cached or indexed are sent with it. + * + * The post is read from the main query rather than the current query, so the redirect + * is still sent when an earlier callback replaces the current query, for example by + * calling {@see query_posts()}. * * If no post is selected, for example on an empty archive, {@see WP::handle_random()} - * handles the request as a 404. If the post's permalink is on another site, the - * request is handled as a regular archive request. + * handles the request as a 404 and this function is not added. If the post's permalink + * is on another site, the request is handled as a regular archive request. * * @since 7.2.0 * - * @global WP_Query $wp_query WordPress Query object. + * @global WP_Query $wp_the_query WordPress main Query object. */ function wp_random_content_redirect(): void { - global $wp_query; + global $wp_the_query; - if ( ! wp_is_random_content_query( $wp_query ) || ! $wp_query->post instanceof WP_Post ) { + if ( + ! $wp_the_query instanceof WP_Query + || ! wp_is_random_content_query( $wp_the_query ) + || ! $wp_the_query->post instanceof WP_Post + ) { return; } - $location = get_permalink( $wp_query->post ); + $location = get_permalink( $wp_the_query->post ); // Only consider redirecting permitted locations so wp_safe_redirect doesn't redirect to the fallback. if ( ! $location || ! wp_validate_redirect( $location, '' ) ) { diff --git a/tests/phpunit/tests/query/randomContentRedirect.php b/tests/phpunit/tests/query/randomContentRedirect.php index 0baa2a82791e3..a7eef8b0116b8 100644 --- a/tests/phpunit/tests/query/randomContentRedirect.php +++ b/tests/phpunit/tests/query/randomContentRedirect.php @@ -154,6 +154,14 @@ public function set_up(): void { // Capture redirects and prevent `exit` from ending the test run. add_filter( 'wp_redirect', array( $this, 'filter_wp_redirect' ), 10, 2 ); + + /* + * The random redirect is sent on `template_redirect`, see go_to(). Canonical + * redirects run on the same action and exit after redirecting even when the + * redirect is suppressed, which would end the test run. Their interplay with + * random content requests is covered by test_redirect_canonical_skips_random_content_requests_with_a_post(). + */ + remove_action( 'template_redirect', 'redirect_canonical' ); } /** @@ -204,6 +212,17 @@ public function record_query( string $query ) { return $query; } + /** + * Requests a URL and fires the `template_redirect` action the redirect is sent on. + * + * @param string $url The URL to request. + */ + public function go_to( $url ): void { + parent::go_to( $url ); + + do_action( 'template_redirect' ); + } + /** * Requests a URL and records the queries made after the request is parsed. * @@ -1889,17 +1908,135 @@ public function test_no_redirect_when_the_permalink_is_empty(): void { } /** - * Ensure the redirect happens before the `wp` action fires. + * Ensure the redirect is sent on the `template_redirect` action at priority 100. */ - public function test_redirect_happens_before_the_wp_action(): void { + public function test_redirect_is_added_to_template_redirect_at_priority_100(): void { + parent::go_to( home_url( '/random/' ) ); + + $this->assertSame( 100, has_action( 'template_redirect', 'wp_random_content_redirect' ), 'The redirect should be added to the template_redirect action at priority 100.' ); + $this->assertNotRedirected( 'The redirect should not be sent before the template_redirect action.' ); + + do_action( 'template_redirect' ); + + $this->assertRedirectedToOneOf( array( self::$arts_post, self::$sport_post ), 'The redirect should be sent on the template_redirect action.' ); + } + + /** + * Ensure the redirect is not added when there is no post to redirect to. + */ + public function test_redirect_is_not_added_to_template_redirect_without_a_post(): void { + parent::go_to( home_url( '/random/category/empty/' ) ); + + $this->assertFalse( has_action( 'template_redirect', 'wp_random_content_redirect' ), 'The redirect should not be added when no post is selected.' ); + } + + /** + * Ensure canonical redirects, which run earlier on `template_redirect`, leave random content requests alone. + * + * A canonical redirect sent before the random redirect would redirect the request + * twice and may drop the `random` query variable, sending the visitor to an archive. + * + * @ticket 66661 + * + * @covers ::redirect_canonical + * + * @dataProvider data_redirect_canonical_skips_random_content_requests_with_a_post + * + * @param string $path Path of the random content request. + */ + public function test_redirect_canonical_skips_random_content_requests_with_a_post( string $path ): void { + parent::go_to( home_url( $path ) ); + + $this->assertSame( 100, has_action( 'template_redirect', 'wp_random_content_redirect' ), 'The request should be a random content request with a post.' ); + $this->assertNull( redirect_canonical( home_url( $path ), false ), 'Canonical redirects must not redirect random content requests with a post.' ); + } + + /** + * Data provider. + * + * @return array + */ + public static function data_redirect_canonical_skips_random_content_requests_with_a_post(): array { + return array( + 'random base' => array( '/random/' ), + 'category' => array( '/random/category/arts/' ), + 'tag' => array( '/random/tag/impressionism/' ), + 'author' => array( '/random/author/random-editor/' ), + 'query string' => array( '/?random' ), + 'cat query var' => array( '/?random&category_name=sport' ), + 'random base with date query' => array( '/random/?m=1844' ), + 'random base with cat query' => array( '/random/?category_name=sport' ), + ); + } + + /** + * Ensure canonical redirects still apply to random content requests without a post to redirect to. + * + * @ticket 66661 + * + * @covers ::redirect_canonical + */ + public function test_redirect_canonical_applies_to_random_requests_that_are_not_redirected(): void { + add_filter( 'wp_enable_random_content_redirect', '__return_false' ); + + parent::go_to( home_url( '/?random&category_name=sport' ) ); + + $this->assertSame( + home_url( '/category/sport/?random' ), + redirect_canonical( home_url( '/?random&category_name=sport' ), false ), + 'Canonical redirects should apply when random content redirects are disabled.' + ); + } + + /** + * Ensure the redirect is sent when an earlier callback replaces the current query. + * + * The selected post is in the main query, which `query_posts()` does not replace. + * + * @ticket 66661 + * + * @dataProvider data_actions_before_the_redirect + * + * @param string $action Action the current query is replaced on. + */ + public function test_redirect_is_sent_when_the_current_query_is_replaced( string $action ): void { + add_action( + $action, + static function (): void { + query_posts( array( 'post_type' => 'page' ) ); + } + ); + + $this->go_to( home_url( '/random/category/arts/' ) ); + + $this->assertNotSame( $GLOBALS['wp_the_query'], $GLOBALS['wp_query'], 'The current query should have been replaced.' ); + $this->assertSame( get_permalink( self::$arts_post ), $this->redirect_location, 'The redirect should be to the post selected by the main query.' ); + } + + /** + * Data provider. + * + * @return array + */ + public static function data_actions_before_the_redirect(): array { + return array( + 'wp' => array( 'wp' ), + 'template_redirect' => array( 'template_redirect' ), + ); + } + + /** + * Ensure the redirect happens after the `wp` action fires. + */ + public function test_redirect_happens_after_the_wp_action(): void { $wp = new MockAction(); add_action( 'wp', array( $wp, 'action' ) ); - $redirected_before_wp = null; + $redirected_after_wp = null; add_filter( 'wp_redirect', - static function ( string $location ) use ( $wp, &$redirected_before_wp ): string { - $redirected_before_wp = 0 === $wp->get_call_count(); + static function ( string $location ) use ( $wp, &$redirected_after_wp ): string { + $redirected_after_wp = 1 === $wp->get_call_count(); return $location; }, 5 @@ -1907,7 +2044,7 @@ static function ( string $location ) use ( $wp, &$redirected_before_wp ): string $this->go_to( home_url( '/random/' ) ); - $this->assertTrue( $redirected_before_wp, 'The redirect should happen before the wp action.' ); + $this->assertTrue( $redirected_after_wp, 'The redirect should happen after the wp action.' ); } /* @@ -2504,8 +2641,12 @@ public function test_handle_random_runs_the_main_query_for_random_requests(): vo $this->assertTrue( $wp->handle_random(), 'Random content requests should be handled.' ); $this->assertTrue( $this->get_main_query()->is_random(), 'The main query should be the random content query.' ); - $this->assertRedirectedToOneOf( array( self::$arts_post, self::$sport_post ), 'The random content request should redirect after running the main query.' ); - $this->assertFalse( is_404(), 'A random content request with a post should not 404 when the redirect does not exit.' ); + $this->assertSame( 100, has_action( 'template_redirect', 'wp_random_content_redirect' ), 'The redirect should be added to the template_redirect action after running the main query.' ); + $this->assertFalse( is_404(), 'A random content request with a post should not 404.' ); + + do_action( 'template_redirect' ); + + $this->assertRedirectedToOneOf( array( self::$arts_post, self::$sport_post ), 'The random content request should redirect on the template_redirect action.' ); } /**