Make WordPress Core

Opened 4 months ago

Closed 3 months ago

#50592 closed defect (bug) (fixed)

Sitemaps: Pass full paths to home_url()

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

Description (last modified by swissspidy)

When using plain permalinks, WP_Sitemaps_Index::get_index_url() builds the sitemap index url like this:
add_query_arg( 'sitemap', 'index', home_url( '/' ) );

See: https://github.com/WordPress/WordPress/blob/fe823d698f5e57135c102e02d7e23d4019ac59d5/wp-includes/sitemaps/class-wp-sitemaps-index.php#L78

There is no issue on a fresh install but it causes issues with plugins or themes using the filter home_url. Any function hooked to this filter will interpret that the url being built is the homepage url when it's actually the sitemap index url which is being built.

I propose to change this to:
home_url( '/?sitemap=index' );
The result should be the same for WordPress but would be less confusing for users of the home_url filter.

I noticed 2 other similar constructions in: https://github.com/WordPress/WordPress/blob/fe823d698f5e57135c102e02d7e23d4019ac59d5/wp-includes/sitemaps/class-wp-sitemaps-renderer.php#L70 and https://github.com/WordPress/WordPress/blob/fe823d698f5e57135c102e02d7e23d4019ac59d5/wp-includes/sitemaps/class-wp-sitemaps-renderer.php#L101

Attachments (3)

50592.diff (1.4 KB) - added by Chouby 4 months ago.
50592.2.diff (3.0 KB) - added by Chouby 4 months ago.
50592.3.diff (2.8 KB) - added by swissspidy 3 months ago.

Download all attachments as: .zip

Change History (9)

4 months ago

#1 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 5.5

#2 @Chouby
4 months ago

  • Keywords has-patch added

For information, I looked at other places of WordPress how is handled this type of case, and I found:
$permalink = home_url( '?p=' . $post->ID ); in get_permalink().
$link = home_url( '?page_id=' . $post->ID ); in _get_page_link().

4 months ago

#3 @Chouby
4 months ago

50592.2.diff fixes an additional case that I had first overlooked.

#4 @swissspidy
3 months ago

  • Owner set to swissspidy
  • Status changed from new to reviewing

3 months ago

#5 @swissspidy
3 months ago

  • Description modified (diff)
  • Keywords commit added
  • Summary changed from Confusing usage of home_url() to Sitemaps: Pass full paths to home_url()

#6 @swissspidy
3 months ago

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

In 48470:

Sitemaps: Pass full paths to home_url() calls.

This makes it easier for plugins using the home_url filter to detect sitemap URLs.

Props Chouby.
Fixes #50592.

Note: See TracTickets for help on using tickets.