Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#50643 closed defect (bug) (fixed)

Ensure correct HTTP status when sitemaps are disabled

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

Description

To reproduce:

Add add_filter( 'wp_sitemaps_enabled', '__return_false' ) in a plugin to disable sitemaps.

Expectation:

https://example.com/wp-sitemap.xml returns a 404 status and displays a 404 page.

Actual result:

https://example.com/wp-sitemap.xml returns a 200 status and displays the homepage.

This is because the rewrite rules are still there, but the sitemaps are actually disabled and wp_sitemaps_get_server() isn't initializing the sitemaps due to the filter

After flushing permalinks the result is as expected: a 404 status & page.

This inconsistent behavior should be fixed.

Attachments (4)

50643.diff (3.3 KB) - added by swissspidy 4 years ago.
50643.2.diff (27.1 KB) - added by kraftbj 4 years ago.
50643.3.diff (3.5 KB) - added by kraftbj 4 years ago.
50643.4.diff (5.0 KB) - added by swissspidy 4 years ago.

Download all attachments as: .zip

Change History (17)

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


4 years ago

#2 @johnbillion
4 years ago

  • Version set to trunk

#3 @SergeyBiryukov
4 years ago

  • Keywords needs-unit-tests added

#4 @swissspidy
4 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#5 @swissspidy
4 years ago

  • Milestone changed from Awaiting Review to 5.5

@swissspidy
4 years ago

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


4 years ago

#7 @kraftbj
4 years ago

Tested this out and works as expected. Updating the patch with inline docs for the filter explaining the rewrites would still be present.

@kraftbj
4 years ago

@kraftbj
4 years ago

This ticket was mentioned in PR #405 on WordPress/wordpress-develop by donmhico.


4 years ago
#8

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

Return 404 on sitemap page without the need to flush permalinks. This PR is based on swissspidy's patch. I just added a unit test.

Trac ticket: https://core.trac.wordpress.org/ticket/50643

#9 @donmhico
4 years ago

The PR I attached - https://github.com/WordPress/wordpress-develop/pull/405 - includes Unit tests for @swissspidy's patch (thank you by the way).

@swissspidy
4 years ago

#10 @swissspidy
4 years ago

  • Keywords commit added; needs-testing removed

Thanks all!

I extended the tests a little bit in the latest patch, and I think this is good to go now.

#11 @swissspidy
4 years ago

#50644 was marked as a duplicate.

#12 @whyisjake
4 years ago

  • Owner set to whyisjake
  • Resolution set to fixed
  • Status changed from new to closed

In 48523:

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.

#13 @swissspidy
4 years ago

In 48540:

Sitemaps: Rename wp_get_sitemaps() to wp_get_sitemaps_providers()

Following [48536], rename the function to match the rest of the sitemaps logic.

Also eliminates some dead code after [48523].

Props pbiron.
See #50724. See #50643.

Note: See TracTickets for help on using tickets.