From 100175145172c30948e9efe305fea307aea9bf8d Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 10:38:51 +1100 Subject: [PATCH 1/7] Add tests for search params and URL fragments. --- tests/phpunit/tests/general/paginateLinks.php | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/tests/phpunit/tests/general/paginateLinks.php b/tests/phpunit/tests/general/paginateLinks.php index 0d1057c919e55..b16b726fd62bf 100644 --- a/tests/phpunit/tests/general/paginateLinks.php +++ b/tests/phpunit/tests/general/paginateLinks.php @@ -438,4 +438,145 @@ public function test_pagination_links_without_trailing_slash() { 'Previous link should not have trailing slash when permalink structure has no trailing slash' ); } + + /** + * @ticket 61393 + * @ticket 63123 + * + * @dataProvider data_search_terms + * + * @param string $search_term Search term - should only be the value passed to `s` parameter. + * @param string $not_expected Search term that is unexpected - should only be the value unexpected in the `s` parameter. + */ + public function test_pagination_links_does_not_modify_search_terms_with_trailing_slash_permalinks( $search_term, $not_expected ) { + $this->set_permalink_structure( '/%postname%/' ); + + $args = array( + 'base' => 'http://example.org/%_%', + 'format' => 'page/%#%', + 'total' => 5, + 'current' => 3, + 'prev_next' => true, + 'add_args' => array( + 's' => $search_term, + ), + ); + + $links = paginate_links( $args ); + + $this->assertStringNotContainsString( + "?s={$not_expected}\"", + $links, + 'Search term should not be modified.' + ); + } + + /** + * @ticket 61393 + * @ticket 63123 + * + * @dataProvider data_search_terms + * + * @param string $search_term Search term - should only be the value passed to `s` parameter. + * @param string $not_expected Search term that is unexpected - should only be the value unexpected in the `s` parameter. + */ + public function test_pagination_links_does_not_modify_search_terms_without_trailing_slash_permalinks( $search_term, $not_expected ) { + $this->set_permalink_structure( '/%postname%' ); + + $args = array( + 'base' => 'http://example.org/%_%', + 'format' => 'page/%#%', + 'total' => 5, + 'current' => 3, + 'prev_next' => true, + 'add_args' => array( + 's' => $search_term, + ), + ); + + $links = paginate_links( $args ); + + $this->assertStringNotContainsString( + "?s={$not_expected}\"", + $links, + 'Search term should not be modified.' + ); + } + + /** + * Data provider for test_pagination_links_does_not_modify_search_terms_* tests. + * + * @return array[] Data provider + */ + public function data_search_terms() { + return array( + array( 'search+term', 'search+term/' ), + array( 'search+term/', 'search+term' ), + ); + } + + /** + * @ticket 61393 + * @ticket 63123 + * + * @dataProvider data_url_fragments + * + * @param string $url_fragment URL fragment - should only be the value following the `#`. + * @param string $not_expected URL fragment that is unexpected - should only be the value unexpected following the `#`. + */ + public function test_pagination_links_does_not_modify_url_fragments_with_trailing_slash_permalinks( $url_fragment, $not_expected ) { + $this->set_permalink_structure( '/%postname%/' ); + + $args = array( + 'base' => "http://example.org/%_%#{$url_fragment}", + 'format' => 'page/%#%', + 'total' => 5, + 'current' => 3, + 'prev_next' => true, + ); + + $links = paginate_links( $args ); + + $this->assertStringNotContainsString( + "#{$not_expected}\"", + $links, + 'URL fragments should not be modified to include a trailing slash.' + ); + } + + /** + * @ticket 61393 + * @ticket 63123 + * + * @dataProvider data_url_fragments + * + * @param string $url_fragment URL fragment - should only be the value following the `#`. + * @param string $not_expected URL fragment that is unexpected - should only be the value unexpected following the `#`. + */ + public function test_pagination_links_does_not_modify_url_fragments_without_trailing_slash_permalinks( $url_fragment, $not_expected ) { + $this->set_permalink_structure( '/%postname%' ); + + $args = array( + 'base' => "http://example.org/%_%#{$url_fragment}", + 'format' => 'page/%#%', + 'total' => 5, + 'current' => 3, + 'prev_next' => true, + ); + + $links = paginate_links( $args ); + + $this->assertStringNotContainsString( + "#{$not_expected}\"", + $links, + 'URL fragments should not be modified to include a trailing slash.' + ); + } + + public function data_url_fragments() { + return array( + array( 'url-fragment', 'url-fragment/' ), + array( 'url-fragment/', 'url-fragment' ), + ); + } } From c51b8131b5228f7940a61dfb5c391608e13a4f2d Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 10:41:15 +1100 Subject: [PATCH 2/7] Search term in base tests. --- tests/phpunit/tests/general/paginateLinks.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/phpunit/tests/general/paginateLinks.php b/tests/phpunit/tests/general/paginateLinks.php index b16b726fd62bf..9b7bba3216b32 100644 --- a/tests/phpunit/tests/general/paginateLinks.php +++ b/tests/phpunit/tests/general/paginateLinks.php @@ -471,6 +471,38 @@ public function test_pagination_links_does_not_modify_search_terms_with_trailing ); } + /** + * @ticket 61393 + * @ticket 63123 + * + * @dataProvider data_search_terms + * + * @param string $search_term Search term - should only be the value passed to `s` parameter. + * @param string $not_expected Search term that is unexpected - should only be the value unexpected in the `s` parameter. + */ + public function test_pagination_links_does_not_modify_search_terms_in_base_with_trailing_slash_permalinks( $search_term, $not_expected ) { + $this->set_permalink_structure( '/%postname%/' ); + + $args = array( + 'base' => "http://example.org/%_%/?s={$search_term}", + 'format' => 'page/%#%', + 'total' => 5, + 'current' => 3, + 'prev_next' => true, + 'add_args' => array( + 's' => $search_term, + ), + ); + + $links = paginate_links( $args ); + + $this->assertStringNotContainsString( + "?s={$not_expected}\"", + $links, + 'Search term should not be modified.' + ); + } + /** * @ticket 61393 * @ticket 63123 @@ -483,6 +515,35 @@ public function test_pagination_links_does_not_modify_search_terms_with_trailing public function test_pagination_links_does_not_modify_search_terms_without_trailing_slash_permalinks( $search_term, $not_expected ) { $this->set_permalink_structure( '/%postname%' ); + $args = array( + 'base' => "http://example.org/%_%?s={$search_term}", + 'format' => 'page/%#%', + 'total' => 5, + 'current' => 3, + 'prev_next' => true, + ); + + $links = paginate_links( $args ); + + $this->assertStringNotContainsString( + "?s={$not_expected}\"", + $links, + 'Search term should not be modified.' + ); + } + + /** + * @ticket 61393 + * @ticket 63123 + * + * @dataProvider data_search_terms + * + * @param string $search_term Search term - should only be the value passed to `s` parameter. + * @param string $not_expected Search term that is unexpected - should only be the value unexpected in the `s` parameter. + */ + public function test_pagination_links_does_not_modify_search_terms_in_base_without_trailing_slash_permalinks( $search_term, $not_expected ) { + $this->set_permalink_structure( '/%postname%' ); + $args = array( 'base' => 'http://example.org/%_%', 'format' => 'page/%#%', From 9055b1e7ec34a1f8c92c88b2828bcd68f02850f6 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 08:30:40 +1100 Subject: [PATCH 3/7] Fix search pagination. Co-Authored-By: im3dabasia1 --- src/wp-includes/general-template.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/general-template.php b/src/wp-includes/general-template.php index 2f23f0e8781e1..e6148f7be3d18 100644 --- a/src/wp-includes/general-template.php +++ b/src/wp-includes/general-template.php @@ -4746,7 +4746,7 @@ function paginate_links( $args = '' ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - $link = get_option( 'permalink_structure' ) ? user_trailingslashit( $link, 'paged' ) : $link; + $link = get_option( 'permalink_structure' ) && ! is_search() ? user_trailingslashit( $link, 'paged' ) : $link; $page_links[] = sprintf( '%s', From aa92918e102d39d99ced271965961ea51b4d9aed Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 08:39:54 +1100 Subject: [PATCH 4/7] =?UTF-8?q?Don=E2=80=99t=20add=20trailing=20slashes=20?= =?UTF-8?q?for=20URLs=20containing=20fragments=20or=20query=20string=20par?= =?UTF-8?q?ams.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/wp-includes/general-template.php | 39 +++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/general-template.php b/src/wp-includes/general-template.php index e6148f7be3d18..5c9f8fc7bfa16 100644 --- a/src/wp-includes/general-template.php +++ b/src/wp-includes/general-template.php @@ -4713,7 +4713,18 @@ function paginate_links( $args = '' ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - $link = get_option( 'permalink_structure' ) ? user_trailingslashit( $link, 'paged' ) : $link; + if ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + /* + * Maybe add a trailing slash to the link according to the site's settings. + * + * Only add this for links without a query string or a fragment. Links with + * these components will have data changed if the trailing slash is added. + * + * This only affects sites with pretty permalinks, as sites without them + * enabled will include a query string parameter. + */ + $link = user_trailingslashit( $link, 'paged' ); + } $page_links[] = sprintf( '', @@ -4746,7 +4757,18 @@ function paginate_links( $args = '' ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - $link = get_option( 'permalink_structure' ) && ! is_search() ? user_trailingslashit( $link, 'paged' ) : $link; + if ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + /* + * Maybe add a trailing slash to the link according to the site's settings. + * + * Only add this for links without a query string or a fragment. Links with + * these components will have data changed if the trailing slash is added. + * + * This only affects sites with pretty permalinks, as sites without them + * enabled will include a query string parameter. + */ + $link = user_trailingslashit( $link, 'paged' ); + } $page_links[] = sprintf( '%s', @@ -4771,7 +4793,18 @@ function paginate_links( $args = '' ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - $link = get_option( 'permalink_structure' ) ? user_trailingslashit( $link, 'paged' ) : $link; + if ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + /* + * Maybe add a trailing slash to the link according to the site's settings. + * + * Only add this for links without a query string or a fragment. Links with + * these components will have data changed if the trailing slash is added. + * + * This only affects sites with pretty permalinks, as sites without them + * enabled will include a query string parameter. + */ + $link = user_trailingslashit( $link, 'paged' ); + } $page_links[] = sprintf( '', From 8b893171038e9880de5dcc9fa618b1189a44d423 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 10:37:46 +1100 Subject: [PATCH 5/7] Ensure trailing slashes do not affect query strings or URL fragments. --- src/wp-includes/general-template.php | 88 ++++++++++++++++------------ 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/src/wp-includes/general-template.php b/src/wp-includes/general-template.php index 5c9f8fc7bfa16..b0759ae1cf1cc 100644 --- a/src/wp-includes/general-template.php +++ b/src/wp-includes/general-template.php @@ -4709,22 +4709,28 @@ function paginate_links( $args = '' ) { if ( $args['prev_next'] && $current && 1 < $current ) : $link = str_replace( '%_%', 2 === $current ? '' : $args['format'], $args['base'] ); $link = str_replace( '%#%', $current - 1, $link ); + /* + * Maybe add a trailing slash to the link according to the site's settings. + * + * Only add this for links without a query string or a fragment. Links with + * these components will have data changed if the trailing slash is added. + * + * This only affects sites with pretty permalinks, as sites without them + * enabled will include a query string parameter. + * + * For links to the base of a domain, a trailing slash is always added to + * the link as that's how browsers handle URLs. + */ + if ( in_array( wp_parse_url( $link, PHP_URL_PATH ), array( null, '/' ), true ) && ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + $link = trailingslashit( $link ); + } elseif ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + $link = user_trailingslashit( $link, 'paged' ); + } + if ( $add_args ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - if ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { - /* - * Maybe add a trailing slash to the link according to the site's settings. - * - * Only add this for links without a query string or a fragment. Links with - * these components will have data changed if the trailing slash is added. - * - * This only affects sites with pretty permalinks, as sites without them - * enabled will include a query string parameter. - */ - $link = user_trailingslashit( $link, 'paged' ); - } $page_links[] = sprintf( '', @@ -4753,22 +4759,27 @@ function paginate_links( $args = '' ) { if ( $args['show_all'] || ( $n <= $end_size || ( $current && $n >= $current - $mid_size && $n <= $current + $mid_size ) || $n > $total - $end_size ) ) : $link = str_replace( '%_%', 1 === $n ? '' : $args['format'], $args['base'] ); $link = str_replace( '%#%', $n, $link ); + /* + * Maybe add a trailing slash to the link according to the site's settings. + * + * Only add this for links without a query string or a fragment. Links with + * these components will have data changed if the trailing slash is added. + * + * This only affects sites with pretty permalinks, as sites without them + * enabled will include a query string parameter. + * + * For links to the base of a domain, a trailing slash is always added to + * the link as that's how browsers handle URLs. + */ + if ( in_array( wp_parse_url( $link, PHP_URL_PATH ), array( null, '/' ), true ) && ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + $link = trailingslashit( $link ); + } elseif ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + $link = user_trailingslashit( $link, 'paged' ); + } if ( $add_args ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - if ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { - /* - * Maybe add a trailing slash to the link according to the site's settings. - * - * Only add this for links without a query string or a fragment. Links with - * these components will have data changed if the trailing slash is added. - * - * This only affects sites with pretty permalinks, as sites without them - * enabled will include a query string parameter. - */ - $link = user_trailingslashit( $link, 'paged' ); - } $page_links[] = sprintf( '%s', @@ -4789,22 +4800,27 @@ function paginate_links( $args = '' ) { if ( $args['prev_next'] && $current && $current < $total ) : $link = str_replace( '%_%', $args['format'], $args['base'] ); $link = str_replace( '%#%', $current + 1, $link ); + /* + * Maybe add a trailing slash to the link according to the site's settings. + * + * Only add this for links without a query string or a fragment. Links with + * these components will have data changed if the trailing slash is added. + * + * This only affects sites with pretty permalinks, as sites without them + * enabled will include a query string parameter. + * + * For links to the base of a domain, a trailing slash is always added to + * the link as that's how browsers handle URLs. + */ + if ( in_array( wp_parse_url( $link, PHP_URL_PATH ), array( null, '/' ), true ) && ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + $link = trailingslashit( $link ); + } elseif ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { + $link = user_trailingslashit( $link, 'paged' ); + } if ( $add_args ) { $link = add_query_arg( $add_args, $link ); } $link .= $args['add_fragment']; - if ( ! str_contains( $link, '?' ) && ( ! str_contains( $link, '#' ) ) ) { - /* - * Maybe add a trailing slash to the link according to the site's settings. - * - * Only add this for links without a query string or a fragment. Links with - * these components will have data changed if the trailing slash is added. - * - * This only affects sites with pretty permalinks, as sites without them - * enabled will include a query string parameter. - */ - $link = user_trailingslashit( $link, 'paged' ); - } $page_links[] = sprintf( '', From 2336b1b9b7ecb4c62ea2b2a569131f34ce5f3630 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 10:54:11 +1100 Subject: [PATCH 6/7] Docblock fixes in the tests. --- tests/phpunit/tests/general/paginateLinks.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/tests/general/paginateLinks.php b/tests/phpunit/tests/general/paginateLinks.php index 9b7bba3216b32..12478ac0cc95a 100644 --- a/tests/phpunit/tests/general/paginateLinks.php +++ b/tests/phpunit/tests/general/paginateLinks.php @@ -567,7 +567,7 @@ public function test_pagination_links_does_not_modify_search_terms_in_base_witho /** * Data provider for test_pagination_links_does_not_modify_search_terms_* tests. * - * @return array[] Data provider + * @return array[] Data provider. */ public function data_search_terms() { return array( @@ -634,6 +634,11 @@ public function test_pagination_links_does_not_modify_url_fragments_without_trai ); } + /** + * Data provider for test_pagination_links_does_not_modify_url_fragments_* tests. + * + * @return array[] Data provider. + */ public function data_url_fragments() { return array( array( 'url-fragment', 'url-fragment/' ), From d9ca32f8dbda4aae62538d7f90fc54fd2513d434 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Tue, 25 Mar 2025 10:56:06 +1100 Subject: [PATCH 7/7] Named data providers. --- tests/phpunit/tests/general/paginateLinks.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/general/paginateLinks.php b/tests/phpunit/tests/general/paginateLinks.php index 12478ac0cc95a..575dd04bd2d00 100644 --- a/tests/phpunit/tests/general/paginateLinks.php +++ b/tests/phpunit/tests/general/paginateLinks.php @@ -571,8 +571,8 @@ public function test_pagination_links_does_not_modify_search_terms_in_base_witho */ public function data_search_terms() { return array( - array( 'search+term', 'search+term/' ), - array( 'search+term/', 'search+term' ), + 'search term without trailing slash' => array( 'search+term', 'search+term/' ), + 'search term with trailing slash' => array( 'search+term/', 'search+term' ), ); } @@ -641,8 +641,8 @@ public function test_pagination_links_does_not_modify_url_fragments_without_trai */ public function data_url_fragments() { return array( - array( 'url-fragment', 'url-fragment/' ), - array( 'url-fragment/', 'url-fragment' ), + 'url fragment without trailing slash' => array( 'url-fragment', 'url-fragment/' ), + 'url fragment with trailing slash' => array( 'url-fragment/', 'url-fragment' ), ); } }