Make WordPress Core

Opened 13 years ago

Last modified 4 weeks ago

#13459 assigned defect (bug)

Conflict between post and page slugs/permalinks when permalink setting is set to /%postname%/

Reported by: jamescollins's profile jamescollins Owned by: petitphp's profile petitphp
Milestone: Future Release Priority: normal
Severity: normal Version: 2.9.2
Component: Permalinks Keywords: has-patch
Focuses: Cc:

Description

If Dashboard -> Settings -> Permalinks is set to /%postname%/, it is possible to create both a page and a post with the same slug. When viewing via the frontend, the page is always displayed.

I would have thought that WordPress should prevent a post and a page from having the same permalink.

Steps to reproduce:

  1. Create and publish a page with any slug. eg. http://domain.com/test/.
  2. Create and publish a blog post with the same slug. Wordpress says the permalink for the blog post is http://domain.com/test/, but when you visit that URL it displays the page instead.

I can reproduce this on my 2.9.2 install, as well as 3.0 trunk. I'm guessing the bug is present in earlier versions of WordPress as well.

Possibly related: #11863

Attachments (9)

13459.patch (1.2 KB) - added by SergeyBiryukov 11 years ago.
13459.2.patch (1.1 KB) - added by moraleida.me 10 years ago.
Exact same patch from SergeyBiryukov, reapplied to current codebase
13459-unit-tests-post.php.patch (1.0 KB) - added by moraleida.me 10 years ago.
post.php.patch (537 bytes) - added by datta.parad 9 years ago.
13459.diff (3.2 KB) - added by MikeHansenMe 9 years ago.
13459-w-test.diff (5.8 KB) - added by MikeHansenMe 9 years ago.
13459.2.diff (5.6 KB) - added by ericlewis 8 years ago.
13459.3.diff (6.2 KB) - added by ericlewis 8 years ago.
13459.4.diff (7.8 KB) - added by ericlewis 8 years ago.

Download all attachments as: .zip

Change History (91)

#1 @solarissmoke
13 years ago

  • Keywords dev-feedback added

I feel like there is another ticket for this, but can't find it.

In any case, I can't see any easy way to fix this, because pages are allowed to have non-unique slugs across hierarchies. One possibility might be to check for this particular permalink structure and modify the unique slug generation accordingly to ensure that no pages have that slug - but it would only work for new/updated posts - old ones would still have a conflict. Bit flimsy.

#2 follow-up: @stephencronin
12 years ago

This problem still occurs in 3.1.1.

It persists even after the Page is sent to Trash (the user gets a 404 instead of the Post), until the Page is emptied from the Trash.

The suggestion by solarissmoke is better than nothing. Ideally it would have to work both ways, ie if permalink is /%postname%/ then:

1) check that no posts have this slug when generating top level page slug
2) check that no top level pages have this slug when generating post slug

It may not cover everything:

a) it may not just be posts and pages we have to worry about (some people strip /category/ from URL for category pages for example);
b) there is a very small chance that /%category%/%postname%/ posts could clash with a second level page.

but they are real edge cases and as I said, it's better to do something than nothing.

#4 @SergeyBiryukov
11 years ago

#23166 was marked as a duplicate.

#5 in reply to: ↑ 2 @SergeyBiryukov
11 years ago

Replying to stephencronin:

It persists even after the Page is sent to Trash (the user gets a 404 instead of the Post), until the Page is emptied from the Trash.

#21970 would fix that.

#6 @ocean90
11 years ago

#23300 was marked as a duplicate.

#7 @markoheijnen
11 years ago

Seriously, till now we never fixed this. That's quite a shame.

#8 @ocean90
11 years ago

  • Keywords needs-patch added; dev-feedback removed

#9 @markoheijnen
11 years ago

#23300 was marked as a duplicate.

#10 follow-up: @SergeyBiryukov
11 years ago

  • Keywords dev-feedback added

wp_unique_post_slug() returns a unique slug within the given post type.

Seems like we have 3 options:

  1. Always prevent posts, pages (and CPTs?) from having the same slug (require unique slugs across all post types). Since having the same slug is actually fine with most permalink structures, this sounds like an unnecessary restriction.
  2. Only do the above for the /%postname%/ permalink structure. However, if the structure changes to /%postname%/ later (after the page and the post are created), we'll still end up with a conflict.
  3. Leave this to a plugin, since wp_unique_post_slug() is filterable (#14111).

I personally think that the permalink conflict is to be expected in this case, so the 3rd option sounds acceptable to me.

#11 in reply to: ↑ 10 @SergeyBiryukov
11 years ago

Replying to SergeyBiryukov:

wp_unique_post_slug() returns a unique slug within the given post type.

Correction: As noted in comment:1, pages are allowed to have non-unique slugs across hierarchies:
http://core.trac.wordpress.org/browser/tags/3.5/wp-includes/post.php#L3103

#12 @markoheijnen
11 years ago

I disagree with leaving this to a plugin. The problem is only for post vs pages when the permalink structure is "/%postname%/".

It seems for me that the 2nd option is the way to go. Obviously changing to the structure will ending up with a conflict but that can be plugin territory.

#13 @SergeyBiryukov
11 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 3.6

13459.patch is an attempt to implement option 2.

#14 @SergeyBiryukov
11 years ago

  • Keywords needs-unit-tests added

@moraleida.me
10 years ago

Exact same patch from SergeyBiryukov, reapplied to current codebase

#15 @moraleida.me
10 years ago

Simple unit test added to cover the case when the same title is used to create a post and a page AND permalinks are set to /%postname%/

#16 @moraleida.me
10 years ago

  • Cc moraleida.me added

#17 @markjaquith
10 years ago

  • Milestone changed from 3.6 to Future Release

Just checking for '/%postname%/' isn't going to cover it completely. Will at least need to standardize trailing slashes on that check. Let's take a look at this early in 3.7.

#18 @context
10 years ago

  • Cc context added

#19 follow-up: @Ipstenu
10 years ago

It appears this is no longer restricted to JUST using /%postname%/ as your permalink.

On 3.8 and 3.9-alpha I can make both a page and a post with the same slug. Using /%year%/%postname%/

#20 in reply to: ↑ 19 @digiscience
10 years ago

For me this is a feature, not a bug. I can place pages into blog lists. Very useful, hope you'll never fix it.

#22 @jeremyfelt
10 years ago

#27297 was marked as a duplicate.

#23 @ryan
9 years ago

  • Owner ryan deleted
  • Status changed from new to assigned

#24 @SergeyBiryukov
9 years ago

#28900 was marked as a duplicate.

#25 @datta.parad
9 years ago

Actually needs to change parameter of get_post_types function, currently it only return "page" Post type.

#26 @SergeyBiryukov
9 years ago

#28960 was marked as a duplicate.

@MikeHansenMe
9 years ago

#27 @MikeHansenMe
9 years ago

It also depends which order the page/post is created. So we need to check the permalink structure in both hierarchical and non-hierarchical.

#28 @MikeHansenMe
9 years ago

  • Keywords needs-unit-tests removed

Added some tests to ensure it is working when different combinations of post/page are made.

#29 @SergeyBiryukov
9 years ago

#31704 was marked as a duplicate.

#30 @SergeyBiryukov
8 years ago

#32426 was marked as a duplicate.

#31 @SergeyBiryukov
8 years ago

#32795 was marked as a duplicate.

#32 @SergeyBiryukov
8 years ago

  • Milestone changed from Future Release to 4.4

See comment:1:ticket:32795 for potential options.

This ticket was mentioned in Slack in #core by sergey. View the logs.


8 years ago

#34 @SergeyBiryukov
8 years ago

  • Owner set to SergeyBiryukov
  • Status changed from assigned to reviewing

#35 @helen
8 years ago

#23166 was marked as a duplicate.

#36 @helen
8 years ago

#23166 was marked as a duplicate.

#37 @SergeyBiryukov
8 years ago

Since [34690], attachments now have pretty permalinks too.

This increases the chances of clashing with post slugs, see #24612 and comment:8:ticket:24612.

We should make sure that attachments and posts cannot have duplicate slugs if /%postname%/ structure is used.

#38 @SergeyBiryukov
8 years ago

For reference, some of this functionality may (or may not) have changed in #18962.

This ticket was mentioned in Slack in #core by sergey. View the logs.


8 years ago

#40 @QROkes
8 years ago

Subdirectory multisite could clash also and may be related with 12002 (Closed and included in 4.4 next release).

#41 @wonderboymusic
8 years ago

  • Keywords needs-refresh added; dev-feedback removed

@SergeyBiryukov the patch is exploding, do we need to do this in 4.4?

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


8 years ago

#43 @wonderboymusic
8 years ago

  • Milestone changed from 4.4 to Future Release

@ericlewis
8 years ago

@ericlewis
8 years ago

#44 @ericlewis
8 years ago

  • Keywords needs-refresh removed

I've added a few changes in attachment:13459.3.diff.

  • Consolidate the checks into a single query rather than two. The extra check we want to perform is to confirm uniqueness across more post types, which we can do with an IN.
  • Avoid post and attachment slug collisions as Sergey suggested.

@ericlewis
8 years ago

#45 @ericlewis
8 years ago

In attachment:13459.4.diff

  • Moved the tests to wpUniquePostSlug.php
  • Added tests for post slug/attachment slug collisions. A post slug can't collide with an attachment slug when the permalink structures are the same, but they can collide when the permalink structures are different.

#46 @SergeyBiryukov
8 years ago

Seems like '/%postname%/' === $wp_rewrite->permalink_structure won't catch a static prefix, e.g. '/blog/%postname%/'. Would that still cause a conflict if there's a 'Blog' page with subpages?

Perhaps the preg_match() check from [35679] could be used.

Last edited 8 years ago by SergeyBiryukov (previous) (diff)

#47 @ericlewis
8 years ago

Seems like '/%postname%/' === $wp_rewrite->permalink_structure won't catch a static prefix, e.g. '/blog/%postname%/'. Would that still cause a conflict if there's a 'Blog' page with subpages?
Perhaps the preg_match() check from [35679] could be used.

We can drop this into our logic for looking up a unique slug for a post (post_type = post). This would be complex to use for page slugs, because that query ensures uniqueness is tied to diffferent post_parent levels / URL path hunks.

Do you think that's a must-fix use-case for this issue, or an edge case? This is not the only collision of this flavor we will see, e.g. %tax_base%/%tax_term% collisions with a subpage.

#48 @ericlewis
8 years ago

  • Owner changed from SergeyBiryukov to ericlewis
  • Status changed from reviewing to accepted

#49 @ericlewis
8 years ago

@SergeyBiryukov said:

Seems like '/%postname%/' === $wp_rewrite->permalink_structure won't catch a static prefix, e.g. '/blog/%postname%/'. Would that still cause a conflict if there's a 'Blog' page with subpages?

I've created a plugin to approach the problem in a more holistic way — Cool slugs. It uses the rewrite API to determine if a URL namespace is already occupied. This has the potential to avoid all problems of this nature. What do you think?

cc @boonebgorges, who also has a natural affinity to wp_unique_post_slug().

#50 @ericlewis
8 years ago

  • Keywords needs-patch good-first-bug added; has-patch removed

#51 @ericlewis
8 years ago

  • Keywords has-patch added; needs-patch good-first-bug removed

This ticket was mentioned in Slack in #core by eric. View the logs.


8 years ago

#53 @Reiniepress
7 years ago

Can i use that "plugin" until someone decides to release the/a fix ?
(not sure I can ask here...)
Thanks.

Last edited 7 years ago by Reiniepress (previous) (diff)

This ticket was mentioned in Slack in #core by helen. View the logs.


7 years ago

#55 @ocean90
7 years ago

#38005 was marked as a duplicate.

#56 @peterwilsoncc
7 years ago

#38022 was marked as a duplicate.

#57 @mdgl
7 years ago

See also #34972. Since the changes for #1914 were applied, slug/permalink conflicts are more likely between unattached media items and pages, regardless of the post permalink settings.

#58 @SergeyBiryukov
6 years ago

#41848 was marked as a duplicate.

#59 @SergeyBiryukov
3 years ago

#52214 was marked as a duplicate.

#60 @SergeyBiryukov
2 years ago

#54031 was marked as a duplicate.

#61 @SergeyBiryukov
19 months ago

#55209 was marked as a duplicate.

#62 @wpmarkuk
19 months ago

I found this issue today too. Creating a page and a post with the same slug results in the post being inaccessible. This seems to be because both have the same slug and the page always wins.

If the permalinks are set to /%postname%/ the I am assuming that posts and pages are using the same "namespace".

I think the solution here, is that if the permalink structure is set like this, when WordPress saves the post slug, it needs to check both posts and pages for others with the same slug. I assume this it is only checking posts at this time. This was probably a legacy as to when permalinks didn't allow /%postname%/.

I don't know what happens when the slug is set, but I assume that there is a check for other posts with the same slug (maybe a query). If pages were included in this check it could solve the problem.

This ticket was mentioned in Slack in #core by bobbingwide. View the logs.


19 months ago

#64 @ericlewis
19 months ago

  • Owner ericlewis deleted
  • Status changed from accepted to assigned

#65 @peterwilsoncc
19 months ago

This was discussed in the dev-chat in Slack earlier. In an earlier comment additional conflicts are mentioned so it would be good to have each of these investigated too so the bug is fixed rather than half a bug.

There are a number of structure tags available on the permalinks screen, some notes on collisions for each would be valuable:

  • content slugs matching a custom post type's slug
  • pages with an apparent date structure: /2022/some-page (also consider months, days, etc)
  • including uncommon components such as author, category

Setting up a pull requests with some initially broken tests would be most helpful. If it's decided that some items aren't in scope, then follow up tickets can be created.

Any changes would need to be made to the function wp_unique_post_slug()

#66 follow-up: @wpmarkuk
19 months ago

I am not sure how to to this:

Setting up a pull requests with some initially broken tests would be most helpful.

Therefore, hopefully someone else can assist here.

#67 in reply to: ↑ 66 @webcommsat
19 months ago

I think this post by @desrosj might help @wpmarkuk https://make.wordpress.org/core/2020/02/21/working-on-trac-tickets-using-github-pull-requests/

[Updated] Thanks for being involved to help move this issue forward.
Replying to wpmarkuk:

I am not sure how to to this:

Setting up a pull requests with some initially broken tests would be most helpful.

Therefore, hopefully someone else can assist here.

Last edited 19 months ago by webcommsat (previous) (diff)

This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.


19 months ago

#69 @bobbingwide
19 months ago

I've documented my view of the problem at https://herbmiller.me/requirements-for-permalink-postname/

In the post I list several steps to reproduce the problem, what I believe to be the requirements to be satisfied and have tabulated some of the alternative permalink structures and the results obtained accessing duplicated entries.

I've briefly considered two different approaches to the problem. I'm coming to the conclusion that parse_request() needs to be changed. The logic associated with use_verbose_page_rules being the main sticking point when duplicates exist.

In my opinion any broken tests should cover the round trip, confirming that the post being requested is the one that's returned to the user.

If required I'll copy paste my post into this TRAC.

#70 @SergeyBiryukov
18 months ago

#55451 was marked as a duplicate.

#71 @SergeyBiryukov
18 months ago

#55472 was marked as a duplicate.

This ticket was mentioned in Slack in #core-test by bobbingwide. View the logs.


18 months ago

#73 @bobbingwide
18 months ago

I've written some round trip black box PHPUnit tests demonstrating two problems.

See https://github.com/bobbingwide/dupes where you'll find the tests and notes on how to run them. They rely on wp_remote_get() actually requesting the content for the permalink and then seeing if it got what it was asking for.

The tests currently produce two failures.

  1. The original problem, where the duplicate page is returned instead of the post.
  2. The inability to retrieve the post by its post ID, when there's a duplicate page.

There are other failing scenarios for which I've not yet written tests.

#74 @thelmachido
15 months ago

This bug was recently flagged on Gutenberg's Git Repo https://github.com/WordPress/gutenberg/issues/41720 after 6.0 was released.

We can reproduce the same when a test site is using the Classic Editor plugin which led us to believe that this might be core related.

This ticket was mentioned in Slack in #core by abhanonstopnews. View the logs.


15 months ago

This ticket was mentioned in Slack in #core by ocean90. View the logs.


15 months ago

#77 @fidoboy
14 months ago

Why not use a customizable base slug for unattached media items? Something like this:

www.domain.com/**media-item**/the-filename-here/

As far as I know, the biggest issue with this thing is specially on sites with thousands of media items and pages, it's very difficult or almost impossible to detect conflicts. Or may be that someone could develop a plugin to scan all library and pages to detect duplicates and alert.

The big problem with this is that is causing random 404 errors and the most of times it's very difficult to detect or to know that it's due to a duplicated permalink into the website structure. Most of times it's causing racing conditions and that's why sometimes a link work as expected and other times it didn't.

Last edited 14 months ago by fidoboy (previous) (diff)

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#79 @audrasjb
8 months ago

  • Owner set to petitphp

As per today's Old Ticket Triage session, @petitphp proposed to refresh the patch. Assiging him as ticket owner :)

This ticket was mentioned in Slack in #core by kebbet. View the logs.


6 months ago

#81 @verygoode
6 months ago

Still present in 6.2.

  1. Spin up a new 6.2 site (no plugins, core theme)
  2. Ensure permalink settings are set to Post name on wp-admin/options-permalink.php
  3. Create a new page titled permalink-test
  4. Create a new post titled permalink-test
  5. Observe that the unique post IDs share the same slug and conflict

#82 @SergeyBiryukov
4 weeks ago

#59243 was marked as a duplicate.

Note: See TracTickets for help on using tickets.