Sitemaps: Don't return a 404 status for valid sitemap requests when the main query has no posts

src/wp-includes/class-wp.php

@@ -746,8 +746,9 @@ public function handle_404() {
 
 		$set_404 = true;
 
-		// Never 404 for the admin, robots, or favicon.
-		if ( is_admin() || is_robots() || is_favicon() ) {
+		// Never 404 here for the admin, robots, favicon, or sitemaps.
+		// Sitemap routes send their own status in WP_Sitemaps::render_sitemaps().
+		if ( is_admin() || is_robots() || is_favicon() || is_sitemap() || get_query_var( 'sitemap-stylesheet' ) ) {
 			$set_404 = false;
 
 			// If posts were found, check for paged content.

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

@@ -157,30 +157,45 @@ public function register_rewrites() {
 	 * Renders sitemap templates based on rewrite rules.
 	 *
 	 * @since 5.5.0
-	 *
-	 * @global WP_Query $wp_query WordPress Query object.
 	 */
 	public function render_sitemaps() {
-		global $wp_query;
+		/*
+		 * Bail early if this isn't a sitemap or stylesheet route.
+		 *
+		 * This runs on every front-end request, so it comes before any
+		 * sanitizing. The raw query vars are tested here, matching
+		 * WP::handle_404(), which exempts sitemap requests from its own 404 on
+		 * the same basis. Testing the sanitized values instead would let a
+		 * request that handle_404() exempted fall through both, leaving it a 200.
+		 */
+		if ( ! get_query_var( 'sitemap' ) && ! get_query_var( 'sitemap-stylesheet' ) ) {
+			return;
+		}
 
-		$sitemap         = sanitize_text_field( get_query_var( 'sitemap' ) );
-		$object_subtype  = sanitize_text_field( get_query_var( 'sitemap-subtype' ) );
-		$stylesheet_type = sanitize_text_field( get_query_var( 'sitemap-stylesheet' ) );
+		$sitemap         = $this->get_sanitized_query_var( 'sitemap' );
+		$object_subtype  = $this->get_sanitized_query_var( 'sitemap-subtype' );
+		$stylesheet_type = $this->get_sanitized_query_var( 'sitemap-stylesheet' );
 		$paged           = absint( get_query_var( 'paged' ) );
 
-		// Bail early if this isn't a sitemap or stylesheet route.
+		// Force a 404 and bail early if the route did not survive sanitizing.
 		if ( ! ( $sitemap || $stylesheet_type ) ) {
+			$this->send_404();
 			return;
 		}
 
 		if ( ! $this->sitemaps_enabled() ) {
-			$wp_query->set_404();
-			status_header( 404 );
+			$this->send_404();
 			return;
 		}
 
 		// Render stylesheet if this is stylesheet route.
 		if ( $stylesheet_type ) {
+			// Force a 404 and bail early if the stylesheet type is not recognized.
+			if ( ! in_array( $stylesheet_type, array( 'sitemap', 'index' ), true ) ) {
+				$this->send_404();
+				return;
+			}
+
 			$stylesheet = new WP_Sitemaps_Stylesheet();
 
 			$stylesheet->render_stylesheet( $stylesheet_type );
@@ -197,7 +212,9 @@ public function render_sitemaps() {
 
 		$provider = $this->registry->get_provider( $sitemap );
 
+		// Force a 404 and bail early if the requested provider is not registered.
 		if ( ! $provider ) {
+			$this->send_404();
 			return;
 		}
 
@@ -209,15 +226,56 @@ public function render_sitemaps() {
 
 		// Force a 404 and bail early if no URLs are present.
 		if ( empty( $url_list ) ) {
-			$wp_query->set_404();
-			status_header( 404 );
+			$this->send_404();
 			return;
 		}
 
 		$this->renderer->render_sitemap( $url_list );
 		exit;
 	}
 
+	/**
+	 * Reads a sitemap query var as a sanitized string.
+	 *
+	 * A public query var can hold any type — an array, for instance, when the
+	 * request supplies one — so anything that is not a scalar becomes an empty
+	 * string, which the caller treats as a route it cannot serve.
+	 *
+	 * @since 7.1.1
+	 *
+	 * @param string $query_var Query variable name.
+	 * @return string Sanitized value, or an empty string.
+	 */
+	private function get_sanitized_query_var( string $query_var ): string {
+		$value = get_query_var( $query_var );
+
+		if ( ! is_scalar( $value ) ) {
+			return '';
+		}
+
+		return sanitize_text_field( (string) $value );
+	}
+
+	/**
+	 * Sends a 404 for a sitemap route that cannot be served.
+	 *
+	 * WP::handle_404() exempts sitemap requests, so every sitemap 404 is issued
+	 * here instead. That includes the no-cache headers handle_404() sends with
+	 * its own 404, so an intermediary does not retain a 404 for a route that
+	 * becomes valid once the site has more content.
+	 *
+	 * @since 7.1.1
+	 *
+	 * @global WP_Query $wp_query WordPress Query object.
+	 */
+	private function send_404(): void {
+		global $wp_query;
+
+		$wp_query->set_404();
+		status_header( 404 );
+		nocache_headers();
+	}
+
 	/**
 	 * Redirects a URL to the wp-sitemap.xml
 	 *

tests/phpunit/tests/sitemaps/sitemaps.php

@@ -493,4 +493,108 @@ public function test_empty_url_list_should_return_404() {
 
 		$this->assertTrue( is_404() );
 	}
+
+	/**
+	 * Ensures a paged subtype route with no URLs still 404s, now that
+	 * WP::handle_404() no longer sets a 404 for sitemap requests.
+	 *
+	 * @ticket 65945
+	 */
+	public function test_empty_url_list_for_subtype_should_return_404() {
+		wp_register_sitemap_provider( 'foo', new WP_Sitemaps_Empty_Test_Provider( 'foo' ) );
+
+		$this->go_to( home_url( '/?sitemap=foo&sitemap-subtype=bar&paged=2' ) );
+
+		wp_sitemaps_get_server()->render_sitemaps();
+
+		$this->assertTrue( is_404() );
+	}
+
+	/**
+	 * Ensures a sitemap query var that does not survive sanitizing 404s.
+	 *
+	 * WP::handle_404() exempts these requests on the raw query var, while
+	 * render_sitemaps() acts on the sanitized value. Without a matching bail
+	 * they fall through both and an arbitrary URL is served as a 200.
+	 *
+	 * @ticket 65945
+	 *
+	 * @dataProvider data_unusable_sitemap_query_vars
+	 *
+	 * @param non-falsy-string $query_string Query string to append to a nonexistent URL.
+	 */
+	public function test_unusable_sitemap_query_var_should_return_404( string $query_string ) {
+		$this->set_permalink_structure( '/%postname%/' );
+
+		// Instantiate the server before navigating: registering the sitemap
+		// rewrite tags is what adds the query vars to `$wp->public_query_vars`.
+		$sitemaps = wp_sitemaps_get_server();
+
+		$this->go_to( home_url( '/this-page-does-not-exist/' . $query_string ) );
+
+		$this->assertFalse( is_404(), 'WP::handle_404() should not have set a 404.' );
+
+		$sitemaps->render_sitemaps();
+
+		$this->assertTrue( is_404(), 'render_sitemaps() should have set a 404.' );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array<non-falsy-string, array{ non-falsy-string }>
+	 */
+	public function data_unusable_sitemap_query_vars(): array {
+		return array(
+			'value stripped by sanitizing' => array( '?sitemap=<>' ),
+			'array sitemap value'          => array( '?sitemap[]=index' ),
+			'array stylesheet value'       => array( '?sitemap-stylesheet[]=sitemap' ),
+		);
+	}
+
+	/**
+	 * Ensures an unrecognized stylesheet type 404s from render_sitemaps().
+	 *
+	 * WP::handle_404() exempts any request carrying a `sitemap-stylesheet`
+	 * query var, and WP_Sitemaps_Stylesheet::render_stylesheet() echoes nothing
+	 * for a type other than 'sitemap' or 'index', so this route would otherwise
+	 * be served as a 200 with an empty body.
+	 *
+	 * @ticket 65945
+	 */
+	public function test_unrecognized_stylesheet_type_should_return_404() {
+		// Instantiate the server before navigating: registering the sitemap rewrite
+		// tags is what adds `sitemap-stylesheet` to `$wp->public_query_vars`.
+		$sitemaps = wp_sitemaps_get_server();
+
+		$this->go_to( home_url( '/?sitemap-stylesheet=this-is-not-a-stylesheet' ) );
+
+		$this->assertFalse( is_404(), 'WP::handle_404() should not have set a 404.' );
+
+		$sitemaps->render_sitemaps();
+
+		$this->assertTrue( is_404(), 'render_sitemaps() should have set a 404.' );
+	}
+
+	/**
+	 * Ensures an unregistered provider 404s from render_sitemaps().
+	 *
+	 * WP::handle_404() exempts every sitemap request, so this route would
+	 * otherwise be served with a 200.
+	 *
+	 * @ticket 65945
+	 */
+	public function test_unregistered_provider_should_return_404() {
+		// Instantiate the server before navigating: registering the sitemap
+		// rewrite tags is what adds `sitemap` to `$wp->public_query_vars`.
+		$sitemaps = wp_sitemaps_get_server();
+
+		$this->go_to( home_url( '/?sitemap=this-provider-does-not-exist' ) );
+
+		$this->assertFalse( is_404(), 'WP::handle_404() should not have set a 404.' );
+
+		$sitemaps->render_sitemaps();
+
+		$this->assertTrue( is_404(), 'render_sitemaps() should have set a 404.' );
+	}
 }

tests/phpunit/tests/wp/handle404.php

@@ -0,0 +1,117 @@
+<?php
+
+/**
+ * @group wp
+ * @group sitemaps
+ *
+ * @covers WP::handle_404
+ */
+class Tests_WP_Handle404 extends WP_UnitTestCase {
+
+	public function set_up() {
+		parent::set_up();
+
+		$this->set_permalink_structure( '/%postname%/' );
+
+		/*
+		 * Priming the server re-registers the sitemap query vars. tear_down()
+		 * replaces the $wp global with a fresh WP instance, which carries only
+		 * the built-in public query vars, and nulls $GLOBALS['wp_sitemaps'] so
+		 * that this call re-runs WP_Sitemaps::init() and adds them back.
+		 */
+		wp_sitemaps_get_server();
+	}
+
+	/**
+	 * A sitemap request must not be turned into a 404 by an empty main query.
+	 *
+	 * Whether the sitemap exists is decided later by WP_Sitemaps::render_sitemaps(),
+	 * so some of these URLs still 404 in a full request, just not from here.
+	 *
+	 * @ticket 65945
+	 *
+	 * @dataProvider data_sitemap_requests
+	 *
+	 * @param non-falsy-string $url Sitemap URL to request.
+	 */
+	public function test_sitemap_requests_should_not_be_404ed_by_an_empty_main_query( string $url ) {
+		$this->go_to( home_url( $url ) );
+
+		$this->assertTrue( is_sitemap(), 'The request should be recognized as a sitemap request.' );
+		$this->assertFalse( is_404(), 'WP::handle_404() should not have set a 404.' );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array<non-falsy-string, array{ non-falsy-string }>
+	 */
+	public function data_sitemap_requests(): array {
+		return array(
+			'index'                     => array( '/?sitemap=index' ),
+			'posts provider'            => array( '/?sitemap=posts&sitemap-subtype=post' ),
+			'posts provider, paged'     => array( '/?sitemap=posts&sitemap-subtype=post&paged=2' ),
+			'pages provider, paged'     => array( '/?sitemap=posts&sitemap-subtype=page&paged=2' ),
+			'taxonomies provider'       => array( '/?sitemap=taxonomies&sitemap-subtype=category' ),
+			'taxonomies provider,paged' => array( '/?sitemap=taxonomies&sitemap-subtype=category&paged=3' ),
+			'users provider, paged'     => array( '/?sitemap=users&paged=2' ),
+		);
+	}
+
+	/**
+	 * The sitemap stylesheet routes must not be 404ed either.
+	 *
+	 * Covered separately because is_sitemap() only reflects the `sitemap` query var.
+	 *
+	 * @ticket 65945
+	 *
+	 * @dataProvider data_sitemap_stylesheet_requests
+	 *
+	 * @param non-falsy-string $url Stylesheet URL to request.
+	 */
+	public function test_sitemap_stylesheet_requests_should_not_be_404ed_by_an_empty_main_query( string $url ) {
+		$this->go_to( home_url( $url ) );
+
+		$this->assertFalse( is_404(), 'WP::handle_404() should not have set a 404.' );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array<non-falsy-string, array{ non-falsy-string }>
+	 */
+	public function data_sitemap_stylesheet_requests(): array {
+		return array(
+			'sitemap stylesheet'        => array( '/?sitemap-stylesheet=sitemap' ),
+			'index stylesheet'          => array( '/?sitemap-stylesheet=index' ),
+			// Not a real route, but the only stylesheet case is_home() doesn't already cover.
+			'sitemap stylesheet, paged' => array( '/?sitemap-stylesheet=sitemap&paged=2' ),
+		);
+	}
+
+	/**
+	 * A genuinely unknown URL must still 404.
+	 *
+	 * @ticket 65945
+	 */
+	public function test_non_sitemap_request_should_still_404() {
+		$this->go_to( home_url( '/this-page-does-not-exist/' ) );
+
+		$this->assertFalse( is_sitemap(), 'The request should not be a sitemap request.' );
+		$this->assertTrue( is_404(), 'An unknown URL should still be a 404.' );
+	}
+
+	/**
+	 * An unregistered sitemap provider must not be turned into a 404 here.
+	 *
+	 * render_sitemaps() sends that status itself, covered in Tests_Sitemaps_Sitemaps.
+	 *
+	 * @ticket 65945
+	 */
+	public function test_unregistered_sitemap_provider_should_not_404_in_handle_404() {
+		$this->go_to( home_url( '/?sitemap=this-provider-does-not-exist' ) );
+
+		$this->assertTrue( is_sitemap(), 'The request should be recognized as a sitemap request.' );
+		$this->assertFalse( is_404(), 'WP::handle_404() should not have set a 404.' );
+	}
+}