[wp-trac] [WordPress Trac] #19292: Not found errors due to sanitization in sanitize_title_with_dashes

WordPress Trac wp-trac at lists.automattic.com
Tue Nov 22 06:27:24 UTC 2011


#19292: Not found errors due to sanitization in sanitize_title_with_dashes
--------------------------+------------------
 Reporter:  xknown        |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  3.3
Component:  General       |     Version:
 Severity:  blocker       |  Resolution:
 Keywords:  has-patch     |
--------------------------+------------------

Comment (by nacin):

 Discussed this further with dd32.

 The second (and worse) blocker here is as follows: Take a page (or any
 '''hierarchical''' post type) that was created in 3.0~3.2 that has a
 special character in the slug. (You can rig this to work without doing svn
 switches by hacking sanitize_title_with_dashes() to prevent the code
 inside the `'save' == $context` conditional from firing, then remove the
 hack.)

 At this point, you will be able to view the page under the bad URL. Now go
 into the page and save it. The page now has a good, clean URL. But the bad
 URL no longer works.

 To fix this, one of the following needs to occur: 1) a hack to prevent
 sanitization from happening to hierarchical post types that are being
 updated, or 2) we need to implement _wp_old_slug that works for pages.

 (2) Is actually not terrible to do as long as we don't care about parent
 changes (the focus of #4328), and focus squarely on slug changes. We can
 then introduce _wp_old_path to properly handle actual path changes in 3.4.
 On the other hand, I don't know what the heck would happen to a child
 page's original link when a parent page is changed. Yeah, so that won't
 really work.

 (1) is doable, but kind of messy. Let me explain:

 At first, I recommended to dd32 that sanitize_title() could be hacked to
 allow an integer for $context, and in that case, $context could then be
 assumed to be a post ID, and then $context would be set to 'save' only if
 the post type was not hierarchical.

 On one hand, this is backwards compatible. ($context was introduced
 specifically for 'save' versus 'query' in 3.1.) On the other hand, this is
 really low level and totally lame.

 '''The better way to do this''' — still hacky, but I'm warming up to it —
 is to modify every call to sanitize_title() that affects $post_name
 generation. This happens in only a few instances — three, and all in
 wp_insert_post(). (Also, these would need to be modified anyway to pass
 post IDs, as proposed and rejected above.)

 The modification is simple: If you're running and update AND it's a
 hierarchical post type, then don't run the sanitization on save context.
 This means that our feature will work for all new content, and old content
 that has _wp_old_slug. [attachment:19292.diff] reflects this.

 dd32 cooked up a slightly different patch that results in it only working
 for new content (or old content that is sanitized identically to new
 content), rather than falling back onto canonical redirections via
 _wp_old_slug. While conservative, our original intent of the rolling
 upgrade was that it would simply work, so I think I would rather go with
 the route I've described.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/19292#comment:5>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list