WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9539 closed defect (bug) (fixed)

Image/Page naming collisions

Reported by: truthmedia Owned by: Denis-de-Bernardy
Milestone: 2.8 Priority: normal
Severity: major Version: 2.7.1
Component: Permalinks Keywords: has-patch tested commit
Focuses: Cc:

Description

This bug is discussed briefly in another related ticket, but should really be in it's own ticket. Related comments found here: http://core.trac.wordpress.org/ticket/6437

There seems to be a bug in WP 2.7.1 which allows images and other media to be assigned the same name/slug as pages. This can result in an image being shown to the visitor instead of the page they were intended to see.

Steps to duplicate: On a vanilla WP install with the URL rewriting enabled, attach an image called test.jpg to the About page. Then create a sub-page under About called "Test". When attempting to view /about/test/ you will see the image, rather than the "Test" page.

The same problem exists when uploading images directly to WordPress through the media library controls as opposed to attaching an image to a post. In this case, the image parent is set to '0' and the image test.jpg is viewable directly off the root of the site at /test/ and can override existing pages with the same name on the same level.

Attachments (2)

9539.diff (3.2 KB) - added by Denis-de-Bernardy 5 years ago.
full patch, including fix for #9726
9539.2.diff (1.8 KB) - added by Denis-de-Bernardy 5 years ago.
partial patch, in case #9726 gets committed before this

Download all attachments as: .zip

Change History (14)

comment:1 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; media slug naming collisions removed

comment:2 hakre5 years ago

  • Keywords reporter-feedback added

in case of collision: which one to display: the page or the media?

comment:3 Denis-de-Bernardy5 years ago

Personally, I'd say the 2.8 upgrader should fix collisions after #6437 gets fixed...

comment:4 ryan5 years ago

Updating a page will cause it to get a new slug if there is a conflict.

comment:5 Denis-de-Bernardy5 years ago

  • Keywords reporter-feedback removed

yup.

suggested fix, as hakre seemed interested in tackling it: in the 2.8 upgrade functions, add a collision check query that goes something like:

SELECT p1.*
FROM $wpdb->posts as p1
JOIN $wpdb->posts as p2
ON p1.post_name = p2.post_name
WHERE p1.post_type = 'attachment'
AND p2.post_type = 'page'
AND p1.post_parent = p2.post_parent

this'll fetch all of the conflicting attachments using an index. we'd then run each one through the unique slug function, and no conflicts should remain.

it'll arguably change the slug of attachments, but better that than page slugs.

comment:6 hakre5 years ago

+1 for that. sounds reasonable. post slugs seem to be the most important slugs on a webblog.

comment:7 hakre5 years ago

i am not so into upgrading. denis, can you propose a patch?

comment:8 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested commit added; needs-patch removed

done.

found #9726 while I was testing. included two versions of the patch in case it gets committed first.

#9727 might be worth looking into as well. it didn't seem to make any difference when I tried adding special characters to the image's title, but I'm not 100% certain that a few "quotes get added/stripped" bugs and so on aren't hiding behind #9727.

comment:9 Denis-de-Bernardy5 years ago

  • Keywords commit removed

woops... just noticed that the db_version is 12000 instead of the needed 11200.

Denis-de-Bernardy5 years ago

full patch, including fix for #9726

Denis-de-Bernardy5 years ago

partial patch, in case #9726 gets committed before this

comment:10 Denis-de-Bernardy5 years ago

  • Keywords commit added

fixed

comment:11 Denis-de-Bernardy5 years ago

  • Owner changed from ryan to Denis-de-Bernardy
  • Status changed from new to accepted

comment:12 azaozz5 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

Seems fixed in [11467] for future attachments. Don't think we should force the changing of the affected attachment slugs as this would break external links, affect plugins, etc. The code that does this can be made into a plugin for users that need it.

Note: See TracTickets for help on using tickets.