Skip to content

Commit dea662b

Browse files
peterwilsonccpeterwilsoncc
committed
Permalinks: Follow rewrite rules for trailing slashes in paginate_links().
Modifies the default settings of `paginate_links()` to only include trailing slashes, eg `example.org/page/2/`, when the permalink settings include a trailing slash. When the permalink settings do not include a slash, neither do the generated links, eg `example.org/page/2`. This prevents the generated links from hitting a URL that subsequently redirects when the permalink structure does not include a trailing slash. Props adamsilverstein, ankitkumarshah, audrasjb, hmbashar, huzaifaalmesbah, joedolson, juanmaguitar, krupajnanda, mai21, ozgursar, peterwilsoncc, rahulsprajapati, sirlouen, welcher, westonruter. Fixes #61393. git-svn-id: https://develop.svn.wordpress.org/trunk@62036 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 5a6e222 commit dea662b

2 files changed

Lines changed: 197 additions & 3 deletions

File tree

‎src/wp-includes/general-template.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4669,12 +4669,26 @@ function paginate_links( $args = '' ) {
46694669
$total = $wp_query->max_num_pages ?? 1;
46704670
$current = get_query_var( 'paged' ) ? (int) get_query_var( 'paged' ) : 1;
46714671

4672-
// Append the format placeholder to the base URL.
4673-
$pagenum_link = trailingslashit( $url_parts[0] ) . '%_%';
4672+
/*
4673+
* Ensures sites not using trailing slashes get links in the form
4674+
* `/page/2` rather than `/page/2/`. On these sites, linking to the
4675+
* URL with a trailing slash will result in a 301 redirect from the
4676+
* incorrect URL to the correctly formatted one. This presents an
4677+
* unnecessary performance hit.
4678+
*/
4679+
if ( $wp_rewrite->using_permalinks() && ! $wp_rewrite->use_trailing_slashes ) {
4680+
$pagenum_link = untrailingslashit( $url_parts[0] );
4681+
} else {
4682+
$pagenum_link = trailingslashit( $url_parts[0] );
4683+
}
4684+
$pagenum_link .= '%_%';
46744685

46754686
// URL base depends on permalink settings.
46764687
$format = $wp_rewrite->using_index_permalinks() && ! strpos( $pagenum_link, 'index.php' ) ? 'index.php/' : '';
46774688
$format .= $wp_rewrite->using_permalinks() ? user_trailingslashit( $wp_rewrite->pagination_base . '/%#%', 'paged' ) : '?paged=%#%';
4689+
if ( $wp_rewrite->using_permalinks() && ! $wp_rewrite->use_trailing_slashes ) {
4690+
$format = '/' . ltrim( $format, '/' );
4691+
}
46784692

46794693
$defaults = array(
46804694
'base' => $pagenum_link, // http://example.com/all_posts.php%_% : %_% is replaced by format (below).

‎tests/phpunit/tests/general/paginateLinks.php‎

Lines changed: 181 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,28 @@
77
*/
88
class Tests_General_PaginateLinks extends WP_UnitTestCase {
99

10-
private $i18n_count = 0;
10+
private int $i18n_count = 0;
11+
12+
/**
13+
* Set up shared fixtures.
14+
*
15+
* @param WP_UnitTest_Factory $factory Factory instance.
16+
*/
17+
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ): void {
18+
$category_id = $factory->term->create(
19+
array(
20+
'taxonomy' => 'category',
21+
'name' => 'Categorized',
22+
)
23+
);
24+
self::assertIsInt( $category_id );
25+
26+
$post_ids = $factory->post->create_many( 10 );
27+
foreach ( $post_ids as $post_id ) {
28+
self::assertIsInt( $post_id );
29+
self::assertIsArray( wp_set_post_categories( $post_id, array( $category_id ) ) );
30+
}
31+
}
1132

1233
public function set_up() {
1334
parent::set_up();
@@ -383,4 +404,163 @@ public function test_custom_base_query_arg_should_be_stripped_from_current_url_b
383404
$page_2_url = home_url() . '?foo=2';
384405
$this->assertContains( "<a class=\"page-numbers\" href=\"$page_2_url\">2</a>", $links );
385406
}
407+
408+
/**
409+
* Ensures pagination links include trailing slashes when the permalink structure includes them.
410+
*
411+
* @ticket 61393
412+
*/
413+
public function test_permalinks_with_trailing_slash_produce_links_with_trailing_slashes(): void {
414+
update_option( 'posts_per_page', 2 );
415+
$this->set_permalink_structure( '/%postname%/' );
416+
417+
$this->go_to( '/category/categorized/page/2/' );
418+
419+
// `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above.
420+
$links = paginate_links( array( 'current' => 2 ) );
421+
422+
$processor = new WP_HTML_Tag_Processor( $links );
423+
$found_links = 0;
424+
while ( $processor->next_tag( 'A' ) ) {
425+
++$found_links;
426+
$href = (string) $processor->get_attribute( 'href' );
427+
$this->assertStringEndsWith( '/', $href, "Pagination links should end with a trailing slash, found: $href" );
428+
}
429+
$this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' );
430+
}
431+
432+
/**
433+
* Ensures pagination links do not include trailing slashes when the permalink structure doesn't include them.
434+
*
435+
* @ticket 61393
436+
*/
437+
public function test_permalinks_without_trailing_slash_produce_links_without_trailing_slashes(): void {
438+
update_option( 'posts_per_page', 2 );
439+
$this->set_permalink_structure( '/%postname%' );
440+
441+
$this->go_to( '/category/categorized/page/2' );
442+
443+
// `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above.
444+
$links = paginate_links( array( 'current' => 2 ) );
445+
446+
$processor = new WP_HTML_Tag_Processor( $links );
447+
$found_links = 0;
448+
while ( $processor->next_tag( 'A' ) ) {
449+
++$found_links;
450+
$href = (string) $processor->get_attribute( 'href' );
451+
$this->assertStringEndsNotWith( '/', $href, "Pagination links should not end with a trailing slash, found: $href" );
452+
}
453+
$this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' );
454+
}
455+
456+
/**
457+
* Ensures pagination links do not include trailing slashes when the permalink structure is plain.
458+
*
459+
* @ticket 61393
460+
*/
461+
public function test_plain_permalinks_are_not_modified_with_trailing_slash(): void {
462+
update_option( 'posts_per_page', 2 );
463+
$this->set_permalink_structure( '' );
464+
465+
$term = get_category_by_slug( 'categorized' );
466+
$this->assertInstanceOf( WP_Term::class, $term );
467+
$category_id = $term->term_id;
468+
$this->go_to( "/?cat={$category_id}&paged=2" );
469+
470+
// `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above.
471+
$links = paginate_links( array( 'current' => 2 ) );
472+
473+
$expected_links = array(
474+
home_url( "?cat={$category_id}" ), // Previous
475+
home_url( "?cat={$category_id}" ), // Page 1
476+
home_url( "?paged=3&cat={$category_id}" ), // Page 3
477+
home_url( "?paged=4&cat={$category_id}" ), // Page 4
478+
home_url( "?paged=5&cat={$category_id}" ), // Page 5
479+
home_url( "?paged=3&cat={$category_id}" ), // Next
480+
);
481+
482+
$processor = new WP_HTML_Tag_Processor( $links );
483+
$found_links = 0;
484+
while ( $processor->next_tag( 'A' ) ) {
485+
$expected_link = $expected_links[ $found_links ] ?? '';
486+
++$found_links;
487+
$href = (string) $processor->get_attribute( 'href' );
488+
$this->assertSame( $expected_link, $href, "Pagination links should include the category query string, found: $href" );
489+
}
490+
$this->assertSame( count( $expected_links ), $found_links, 'There should be this number of pagination links found.' );
491+
}
492+
493+
/**
494+
* Ensures the pagination links do not modify query strings (permalinks with trailing slash).
495+
*
496+
* @ticket 61393
497+
* @ticket 63123
498+
*
499+
* @dataProvider data_query_strings
500+
*
501+
* @param string $query_string Query string.
502+
*/
503+
public function test_permalinks_with_trailing_slash_do_not_modify_query_strings( string $query_string ): void {
504+
update_option( 'posts_per_page', 2 );
505+
$this->set_permalink_structure( '/%postname%/' );
506+
507+
$this->go_to( "/page/2/?{$query_string}" );
508+
509+
// `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above.
510+
$links = paginate_links( array( 'current' => 2 ) );
511+
512+
$processor = new WP_HTML_Tag_Processor( $links );
513+
$found_links = 0;
514+
while ( $processor->next_tag( 'A' ) ) {
515+
++$found_links;
516+
$href = (string) $processor->get_attribute( 'href' );
517+
$this->assertStringEndsWith( "/?{$query_string}", $href, "Pagination links should not modify the query string, found: $href" );
518+
}
519+
$this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' );
520+
}
521+
522+
/**
523+
* Ensures the pagination links do not modify query strings (permalinks without trailing slash).
524+
*
525+
* @ticket 61393
526+
* @ticket 63123
527+
*
528+
* @dataProvider data_query_strings
529+
*
530+
* @param string $query_string Query string.
531+
*/
532+
public function test_permalinks_without_trailing_slash_do_not_modify_query_strings( string $query_string ): void {
533+
update_option( 'posts_per_page', 2 );
534+
$this->set_permalink_structure( '/%postname%' );
535+
536+
$this->go_to( "/page/2?{$query_string}" );
537+
538+
// `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above.
539+
$links = paginate_links( array( 'current' => 2 ) );
540+
541+
$processor = new WP_HTML_Tag_Processor( $links );
542+
$found_links = 0;
543+
while ( $processor->next_tag( 'A' ) ) {
544+
++$found_links;
545+
$href = (string) $processor->get_attribute( 'href' );
546+
$this->assertStringEndsWith( "?{$query_string}", $href, "Pagination links should not modify the query string, found: $href" );
547+
$this->assertStringEndsNotWith( "/?{$query_string}", $href, "Pagination links should not be slashed before the query string, found: $href" );
548+
}
549+
$this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' );
550+
}
551+
552+
/**
553+
* Data provider.
554+
*
555+
* @see self::test_permalinks_without_trailing_slash_do_not_modify_query_strings()
556+
* @see self::test_permalinks_with_trailing_slash_do_not_modify_query_strings()
557+
*
558+
* @return array<string, array{ 0: string }> Data provider.
559+
*/
560+
public function data_query_strings(): array {
561+
return array(
562+
'single query var' => array( 'foo=bar' ),
563+
'multi query vars' => array( 'foo=bar&pen=pencil' ),
564+
);
565+
}
386566
}

0 commit comments

Comments
 (0)