WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 5 months ago

#25021 new defect (bug)

Improve sanitize_title_with_dashes % removal

Reported by: duck_ Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.5
Component: Formatting Keywords: has-patch needs-testing
Focuses: Cc:
PR Number:

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_ 6 years ago.

Download all attachments as: .zip

Change History (7)

#1 @duck_
6 years 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.

#2 @SergeyBiryukov
6 years ago

  • Keywords has-patch added

Related: #3329

#3 follow-up: @nacin
6 years 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)?

#5 in reply to: ↑ 3 @nacin
6 years 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.

#6 @chriscct7
4 years ago

  • Keywords needs-testing added
Note: See TracTickets for help on using tickets.