WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 3 months ago

#25021 new defect (bug)

Improve sanitize_title_with_dashes % removal

Reported by: duck_ Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 1.5
Component: Formatting Keywords: has-patch
Focuses: Cc:

Description

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]

Attachments (1)

25021.negative-lookahead.diff (799 bytes) - added by duck_ 8 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 duck_8 months ago

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.

comment:2 SergeyBiryukov8 months ago

  • Keywords has-patch added

Related: #3329

comment:3 follow-up: nacin8 months ago

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)?

comment:5 in reply to: ↑ 3 nacin3 months ago

  • Milestone changed from Awaiting Review to Future Release

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.

Note: See TracTickets for help on using tickets.