From 60d339cfde53d6c5d947c7865e05f7d840ba53ba Mon Sep 17 00:00:00 2001 From: Jake Spurlock Date: Tue, 21 Jul 2020 00:55:20 +0000 Subject: [PATCH] Sitemaps: Ensure correct HTTP status when sitemaps are disabled If sitemaps are disabled, previously there would be a rewrite rule for the sitemap endpoint. This endpoint would display the homepage since there was a rewrite rule. Now, Sitemaps are loaded, and the proper HTTP headers are returned. Fixes #50643. Props swissspidy, kraftbj, donmhico. git-svn-id: https://develop.svn.wordpress.org/trunk@48523 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/sitemaps.php | 15 +------ .../sitemaps/class-wp-sitemaps.php | 44 +++++++++++++++++- tests/phpunit/tests/sitemaps/sitemaps.php | 45 +++++++++++++++++++ 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/src/wp-includes/sitemaps.php b/src/wp-includes/sitemaps.php index 8678993dbd..9600540156 100644 --- a/src/wp-includes/sitemaps.php +++ b/src/wp-includes/sitemaps.php @@ -17,26 +17,13 @@ * * @global WP_Sitemaps $wp_sitemaps Global Core Sitemaps instance. * - * @return WP_Sitemaps|null Sitemaps instance, or null if sitemaps are disabled. + * @return WP_Sitemaps Sitemaps instance. */ function wp_sitemaps_get_server() { global $wp_sitemaps; $is_enabled = (bool) get_option( 'blog_public' ); - /** - * Filters whether XML Sitemaps are enabled or not. - * - * @since 5.5.0 - * - * @param bool $is_enabled Whether XML Sitemaps are enabled or not. Defaults to true for public sites. - */ - $is_enabled = (bool) apply_filters( 'wp_sitemaps_enabled', $is_enabled ); - - if ( ! $is_enabled ) { - return null; - } - // If there isn't a global instance, set and bootstrap the sitemaps system. if ( empty( $wp_sitemaps ) ) { $wp_sitemaps = new WP_Sitemaps(); diff --git a/src/wp-includes/sitemaps/class-wp-sitemaps.php b/src/wp-includes/sitemaps/class-wp-sitemaps.php index 918220317b..828e7931dc 100644 --- a/src/wp-includes/sitemaps/class-wp-sitemaps.php +++ b/src/wp-includes/sitemaps/class-wp-sitemaps.php @@ -56,19 +56,54 @@ class WP_Sitemaps { /** * Initiates all sitemap functionality. * + * If sitemaps are disabled, only the rewrite rules will be registered + * by this method, in order to properly send 404s. + * * @since 5.5.0 */ public function init() { // These will all fire on the init hook. $this->register_rewrites(); + + add_action( 'template_redirect', array( $this, 'render_sitemaps' ) ); + + if ( ! $this->sitemaps_enabled() ) { + return; + } + $this->register_sitemaps(); // Add additional action callbacks. - add_action( 'template_redirect', array( $this, 'render_sitemaps' ) ); add_filter( 'pre_handle_404', array( $this, 'redirect_sitemapxml' ), 10, 2 ); add_filter( 'robots_txt', array( $this, 'add_robots' ), 0, 2 ); } + /** + * Determines whether sitemaps are enabled or not. + * + * @since 5.5.0 + * + * @return bool Whether sitemaps are enabled. + */ + public function sitemaps_enabled() { + $is_enabled = (bool) get_option( 'blog_public' ); + + /** + * Filters whether XML Sitemaps are enabled or not. + * + * When XML Sitemaps are disabled via this filter, rewrite rules are still + * in place to ensure a 404 is returned. + * + * @see WP_Sitemaps::register_rewrites() + * + * @since 5.5.0 + * + * @param bool $is_enabled Whether XML Sitemaps are enabled or not. Defaults + * to true for public sites. + */ + return (bool) apply_filters( 'wp_sitemaps_enabled', $is_enabled ); + } + /** * Registers and sets up the functionality for all supported sitemaps. * @@ -155,6 +190,12 @@ class WP_Sitemaps { return; } + if ( ! $this->sitemaps_enabled() ) { + $wp_query->set_404(); + status_header( 404 ); + return; + } + // Render stylesheet if this is stylesheet route. if ( $stylesheet_type ) { $stylesheet = new WP_Sitemaps_Stylesheet(); @@ -186,6 +227,7 @@ class WP_Sitemaps { // Force a 404 and bail early if no URLs are present. if ( empty( $url_list ) ) { $wp_query->set_404(); + status_header( 404 ); return; } diff --git a/tests/phpunit/tests/sitemaps/sitemaps.php b/tests/phpunit/tests/sitemaps/sitemaps.php index 4370f655b2..35a5d6e8b6 100644 --- a/tests/phpunit/tests/sitemaps/sitemaps.php +++ b/tests/phpunit/tests/sitemaps/sitemaps.php @@ -444,4 +444,49 @@ class Test_Sitemaps extends WP_UnitTestCase { $this->assertContains( $sitemap_string, $robots_text, 'Sitemap URL not prefixed with "\n".' ); } + + /** + * @ticket 50643 + */ + public function test_sitemaps_enabled() { + $before = wp_sitemaps_get_server()->sitemaps_enabled(); + add_filter( 'wp_sitemaps_enabled', '__return_false' ); + $after = wp_sitemaps_get_server()->sitemaps_enabled(); + remove_filter( 'wp_sitemaps_enabled', '__return_false' ); + + $this->assertTrue( $before ); + $this->assertFalse( $after ); + } + + /** + * @ticket 50643 + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_disable_sitemap_should_return_404() { + add_filter( 'wp_sitemaps_enabled', '__return_false' ); + + $this->go_to( home_url( '/?sitemap=index' ) ); + + wp_sitemaps_get_server()->render_sitemaps(); + + remove_filter( 'wp_sitemaps_enabled', '__return_false' ); + + $this->assertTrue( is_404() ); + } + + /** + * @ticket 50643 + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_empty_url_list_should_return_404() { + wp_register_sitemap( 'foo', new WP_Sitemaps_Empty_Test_Provider( 'foo' ) ); + + $this->go_to( home_url( '/?sitemap=foo' ) ); + + wp_sitemaps_get_server()->render_sitemaps(); + + $this->assertTrue( is_404() ); + } }