WordPress.org

Make WordPress Core

#50666 closed defect (bug) (fixed)

Size limit not correctly applied for sitemaps index

Reported by: swissspidy Owned by: swissspidy
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.5
Component: Sitemaps Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

Sitemaps have a few size restrictions:

  • each Sitemap file that you provide must have no more than 50,000 URLs
  • Sitemap index files may not list more than 50,000 Sitemaps

The latter is seemingly enforced in \WP_Sitemaps_Registry::get_sitemaps(), but actually this method only limits the number of sitemap providers, not the amount of sitemaps all providers add to the index in total.

Thus, this logic needs to be moved to \WP_Sitemaps_Index::get_sitemap_list()

Attachments (2)

50666.diff (4.9 KB) - added by swissspidy 11 months ago.
50666.2.diff (6.8 KB) - added by pbiron 11 months ago.

Download all attachments as: .zip

Change History (8)

@swissspidy
11 months ago

#1 @swissspidy
11 months ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

This ticket was mentioned in Slack in #core-sitemaps by swissspidy. View the logs.


11 months ago

#3 @pbiron
11 months ago

50666.diff works as expected, and all tests pass (existing and the new one).

However, I'd suggest one change to WP_Sitemaps_Index::get_sitemap_list(): for efficiency, break out of the loop as soon as count( $sitemaps ) >= $this->max_sitemaps, no need to have all providers generate entries if the max has already been reached.

I'd also structure the new test a little differently.

Updated patch coming shortly.

@pbiron
11 months ago

#4 @pbiron
11 months ago

50666.2.diff is the same as 50666.diff except for:

  1. WP_Sitemaps_Index::get_sitemap_list(): break out of the loop as soon as count( $sitemaps ) >= $this->max_sitemaps
  2. the new unit test is more explict about what it is testing
    • as part of that, the constructor for the new WP_Sitemaps_Large_Test_Provider class takes a parameter for how many entries the test provider should generate
    • this new parameter may also be useful in future uses of this test provider

#5 @swissspidy
11 months ago

  • Keywords commit added
  • Status changed from assigned to reviewing

#6 @swissspidy
11 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 48532:

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.

Note: See TracTickets for help on using tickets.