From 38cade3b07ee994817d3ce03885d3e422f1fbdbd Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Tue, 21 Jul 2020 13:55:45 +0000 Subject: [PATCH] Sitemaps: Correctly enforce maximum number of sitemaps in index. Before this change, the limit of 50k entries was enforced for the number of providers, not the amount of sitemaps all providers add to the index in total. Props pbiron, swissspidy. Fixes #50666. git-svn-id: https://develop.svn.wordpress.org/trunk@48532 602fd350-edb4-49c9-b593-d223f7449a82 --- .../sitemaps/class-wp-sitemaps-index.php | 16 ++++- .../sitemaps/class-wp-sitemaps-registry.php | 17 +----- tests/phpunit/includes/bootstrap.php | 1 + .../class-wp-sitemaps-large-test-provider.php | 59 +++++++++++++++++++ .../phpunit/tests/sitemaps/sitemaps-index.php | 23 ++++++++ 5 files changed, 98 insertions(+), 18 deletions(-) create mode 100644 tests/phpunit/includes/class-wp-sitemaps-large-test-provider.php diff --git a/src/wp-includes/sitemaps/class-wp-sitemaps-index.php b/src/wp-includes/sitemaps/class-wp-sitemaps-index.php index 4ec1b07d92..91ceea0ae0 100644 --- a/src/wp-includes/sitemaps/class-wp-sitemaps-index.php +++ b/src/wp-includes/sitemaps/class-wp-sitemaps-index.php @@ -24,6 +24,15 @@ class WP_Sitemaps_Index { */ protected $registry; + /** + * Maximum number of sitemaps to include in an index. + * + * @sincee 5.5.0 + * + * @var int Maximum number of sitemaps. + */ + private $max_sitemaps = 50000; + /** * WP_Sitemaps_Index constructor. * @@ -47,7 +56,7 @@ class WP_Sitemaps_Index { $providers = $this->registry->get_sitemaps(); /* @var WP_Sitemaps_Provider $provider */ - foreach ( $providers as $provider ) { + foreach ( $providers as $name => $provider ) { $sitemap_entries = $provider->get_sitemap_entries(); // Prevent issues with array_push and empty arrays on PHP < 7.3. @@ -57,9 +66,12 @@ class WP_Sitemaps_Index { // Using array_push is more efficient than array_merge in a loop. array_push( $sitemaps, ...$sitemap_entries ); + if ( count( $sitemaps ) >= $this->max_sitemaps ) { + break; + } } - return $sitemaps; + return array_slice( $sitemaps, 0, $this->max_sitemaps, true ); } /** diff --git a/src/wp-includes/sitemaps/class-wp-sitemaps-registry.php b/src/wp-includes/sitemaps/class-wp-sitemaps-registry.php index c232764924..c95f04c77f 100644 --- a/src/wp-includes/sitemaps/class-wp-sitemaps-registry.php +++ b/src/wp-includes/sitemaps/class-wp-sitemaps-registry.php @@ -24,15 +24,6 @@ class WP_Sitemaps_Registry { */ private $sitemaps = array(); - /** - * Maximum number of sitemaps to include in an index. - * - * @sincee 5.5.0 - * - * @var int Maximum number of sitemaps. - */ - private $max_sitemaps = 50000; - /** * Adds a sitemap with route to the registry. * @@ -69,19 +60,13 @@ class WP_Sitemaps_Registry { } /** - * Lists all registered sitemaps. + * Returns all registered sitemap providers. * * @since 5.5.0 * * @return WP_Sitemaps_Provider[] Array of sitemap providers. */ public function get_sitemaps() { - $total_sitemaps = count( $this->sitemaps ); - - if ( $total_sitemaps > $this->max_sitemaps ) { - return array_slice( $this->sitemaps, 0, $this->max_sitemaps, true ); - } - return $this->sitemaps; } } diff --git a/tests/phpunit/includes/bootstrap.php b/tests/phpunit/includes/bootstrap.php index 817aea3bbe..8c88fc45e2 100644 --- a/tests/phpunit/includes/bootstrap.php +++ b/tests/phpunit/includes/bootstrap.php @@ -162,6 +162,7 @@ require __DIR__ . '/class-wp-rest-test-configurable-controller.php'; require __DIR__ . '/class-wp-fake-block-type.php'; require __DIR__ . '/class-wp-sitemaps-test-provider.php'; require __DIR__ . '/class-wp-sitemaps-empty-test-provider.php'; +require __DIR__ . '/class-wp-sitemaps-large-test-provider.php'; /** * A class to handle additional command line arguments passed to the script. diff --git a/tests/phpunit/includes/class-wp-sitemaps-large-test-provider.php b/tests/phpunit/includes/class-wp-sitemaps-large-test-provider.php new file mode 100644 index 0000000000..67563c654d --- /dev/null +++ b/tests/phpunit/includes/class-wp-sitemaps-large-test-provider.php @@ -0,0 +1,59 @@ +name = 'tests'; + $this->object_type = 'test'; + + $this->num_entries = $num_entries; + } + + /** + * Gets a URL list for a sitemap. + * + * @param int $page_num Page of results. + * @param string $object_subtype Optional. Object subtype name. Default empty. + * @return array List of URLs for a sitemap. + */ + public function get_url_list( $page_num, $object_subtype = '' ) { + return array_fill( 0, $this->num_entries, array( 'loc' => home_url( '/' ) ) ); + } + + /** + * Lists sitemap pages exposed by this provider. + * + * The returned data is used to populate the sitemap entries of the index. + * + * @return array[] Array of sitemap entries. + */ + public function get_sitemap_entries() { + return array_fill( 0, $this->num_entries, array( 'loc' => home_url( '/' ) ) ); + } + + /** + * Query for determining the number of pages. + * + * @param string $object_subtype Optional. Object subtype. Default empty. + * @return int Total number of pages. + */ + public function get_max_num_pages( $object_subtype = '' ) { + return $this->num_entries; + } +} diff --git a/tests/phpunit/tests/sitemaps/sitemaps-index.php b/tests/phpunit/tests/sitemaps/sitemaps-index.php index 5df604af8a..56b92be569 100644 --- a/tests/phpunit/tests/sitemaps/sitemaps-index.php +++ b/tests/phpunit/tests/sitemaps/sitemaps-index.php @@ -20,6 +20,29 @@ class Test_WP_Sitemaps_Index extends WP_UnitTestCase { $this->assertCount( 24, $sitemap_index->get_sitemap_list() ); } + /** + * Test that a sitemap index won't contain more than 50000 sitemaps. + * + * @ticket 50666 + */ + public function test_get_sitemap_list_limit() { + $registry = new WP_Sitemaps_Registry(); + + // add 3 providers, which combined produce more than the maximum 50000 sitemaps in the index. + $registry->add_sitemap( 'provider_1', new WP_Sitemaps_Large_Test_Provider( 25000 ) ); + $registry->add_sitemap( 'provider_2', new WP_Sitemaps_Large_Test_Provider( 25000 ) ); + $registry->add_sitemap( 'provider_3', new WP_Sitemaps_Large_Test_Provider( 25000 ) ); + + $count = 0; + foreach ( $registry->get_sitemaps() as $provider ) { + $count += count( $provider->get_url_list( 1 ) ); + } + $this->assertGreaterThan( 50000, $count ); + + $sitemap_index = new WP_Sitemaps_Index( $registry ); + $this->assertCount( 50000, $sitemap_index->get_sitemap_list() ); + } + public function test_get_sitemap_list_no_entries() { $registry = new WP_Sitemaps_Registry();