Query: Send the random content redirect on template_redirect.

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.

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();

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, '' ) ) {

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,25 +1908,143 @@ 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<string, array{string}>
+	 */
+	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<string, array{string}>
+	 */
+	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
 		);
 
 		$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.' );
 	}
 
 	/**