Improve sanitize_title_with_dashes % removal

[duck_]

The current method of % removal involves placeholders to prevent stomping on escape sequences:

// Preserve escaped octets.
$title = preg_replace('|%([a-fA-F0-9][a-fA-F0-9])|', '---$1---', $title);
// Remove percent signs that are not part of an octet.
$title = str_replace('%', '', $title);
// Restore octets.
$title = preg_replace('|---([a-fA-F0-9][a-fA-F0-9])---|', '%$1', $title);

This leads to sanitize_title_with_dashes('---aa---') producing %aa instead of just aa.

[2189]

[duck_]

25021.negative-lookahead.diff​ replaces the placeholder system with a single regex with a negative lookahead. This fixes the issue, but is very slightly slower in my very limited testing.

[SergeyBiryukov]

Related: #3329

[nacin]

I'm going to guess "very slightly slower" is insignificant enough to warrant this change? We've always suggested that sanitize_title() and friends are not light and thus should be avoided where possible anyway.

How might this affect sanitize_title_for_query() (as in, working with older slugs that don't sanitize the same way)?

[SergeyBiryukov]

Related: ticket:14773:14

[nacin]

Replying to nacin:

How might this affect sanitize_title_for_query() (as in, working with older slugs that don't sanitize the same way)?

As long as this doesn't break anything, I'm game.

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

[SirLouen]

Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/9011.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 7.4.33
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 7.4.33)
• Browser: Chrome 137.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Testing instructions

• Add the code proposed by OP anywhere that can be run
• For example as the one provided in artifacts, using [sanitize_shortcode] shortcode somewhere in a post
• 🐞 Text displayed: %aa

Expected results

• Text displayed aa

Actual Results

1. ✅ Issue resolved with patch.

Additional Notes

• This is old, but is still valid.
• I've patch refreshed, added some unit tests and posted this PR just to run the whole testing suite and see if it’s passing.
• I know that these kinds of regexes are not tasteful, but I see that this can move on.

Supplemental Artifacts

Code used for testing:

function sanitize_title_with_dashes_test() {
	$title = '---aa---';
	$sanitized_title = sanitize_title_with_dashes( $title );
	return $sanitized_title;
}
add_shortcode( 'sanitize_shortcode', 'sanitize_title_with_dashes_test' );

[lakshyajeet]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/9011

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 136.0.0.0
• OS: macOS
• Theme: Twenty Nineteen 3.1
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Testing instructions

• Apply the patch.
• Use Shortcode to display the text (sample code below).
• Add shortcode in a post: [sanitize_title_shortcode]
• Text displayed %abc

Expected Results

• The text abc should be displayed.

Actual Results

• The text abc is displayed.
• ✅ Issue resolved with patch.

Supplemental Artifacts

Code used:

function sanitize_title_with_dashes_shortcode_test() {
        $title = '---abc---';
        $sanitized_title = sanitize_title_with_dashes( $title );
        return $sanitized_title;
}
add_shortcode( 'sanitize_title_shortcode', 'sanitize_title_with_dashes_shortcode_test' );

[SirLouen]

6.9.0 Milestone proposed during today's <bug-scrub>
​More information

cc @azaozz

[azaozz]

Replying to SirLouen:

6.9.0 Milestone proposed during today's <bug-scrub>

Thanks @SirLouen and @lakshyajeet for testing this patch. However seems testing has to be done for the rest of the cases. I think Nacin nailed it 11 years ago:

As long as this doesn't break anything, I'm game.

Seems this wasn't clear back then and is still not clear.