Make WordPress Core

Opened 6 years ago

Last modified 2 months ago

#47552 new defect (bug)

post_name when inserting is not guaranteed to be unique

Reported by: domslee's profile domslee Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-test-info needs-patch needs-unit-tests
Focuses: Cc:

Description

Hi all,

In wp_insert_post, there is a gap between the allocation of the post_name (as wp_unique_post_slug), and the insertion ($wpdb->insert) of the post. This is problematic because another WordPress instance can insert a post with the same post_name in this gap, which would result in two posts being inserted with the same post name

See here for an example: https://github.com/domsleee/wp_insert_post_duplicate

I suppose a solution may be to obtain a table lock, then find a unique slug and insert the post, and then release the table lock... but this doesn't seem very performant. What do you guys think?

Attachments (2)

wpdev-docker-images-47552.patch (1.3 KB) - added by SirLouen 2 months ago.
The patch for wpdev-docker-images
docker-compose-test-47552.patch (743 bytes) - added by SirLouen 2 months ago.
Docker Compose changes to run sysv

Download all attachments as: .zip

Change History (5)

#1 @SergeyBiryukov
6 years ago

  • Component changed from General to Posts, Post Types

#2 follow-up: @domslee
6 years ago

Should we consider a UNIQUE constraint on (post_name, post_type, post_status)?

@SirLouen
2 months ago

The patch for wpdev-docker-images

@SirLouen
2 months ago

Docker Compose changes to run sysv

#3 in reply to: ↑ 2 @SirLouen
2 months ago

  • Keywords has-test-info needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

Reproduction Report

Description

✅ This report validates whether the issue can be reproduced.

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.29
  • Server: nginx/1.29.1
  • Database: mysqli (Server: 8.4.6 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 140.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.3
  • MU Plugins: None activated
  • Plugins:
    • BBB Testing Dolly
    • Test Reports 1.2.0

Testing Instructions

  1. If running wordpress-develop you will need a couple tweaks and the wpdev-docker-images WordPress repo. Check these two patches (wpdev-docker-images patch and docker compose patch to set everything up.
  2. Add the plugin in the OP
  3. Run with WP CLI cli wp my_plugin
  4. 🐞 Two posts are created with the exact same post_name

Actual Results

  1. ✅ Error condition occurs (reproduced).

Additional Information

Replying to domslee:

Should we consider a UNIQUE constraint on (post_name, post_type, post_status)?

There could be matching names as long as they are hierarchically in different levels. So we could have two post_type = page, two post_status = publish and two post_name identical. At best this could be done only if post_type = post (or for non hierarchical post types). But definitely not as a SQL UNIQUE key. I think that this specific casuistry is what brings all this havoc with many reports having this trouble under this scenario.

To test this I've added two files, one to add for a php 8.2 image in wpdev-docker-images and little tweak for the current docker-compose.yml to run the semaphores. Main problem here is that the default images are not well prepared to run scripts with extra libraries. I've added these two files, in case I have to refer back to this ticket, and I'm able to test this without having to spend the time I have spent to learn how to set up this in the wordpress-develop environment`.

So finally, after some testing and reviewing of this, I can confirm that the possibility to do this is there.

I'm unsure if #61996 is related because happens the same, but in page type, and playing around with parent-child relationships (i mean real WP parental relationship, not thread relationships like in the CLI plugin). Also this is being tricker to test, because since its running under wp-cli conditions, I'm unable to run with xdebug to clearly see the order of the operations in wp_insert_post

Definitely not an easy patch here at first glance. I would like to hear more opinions. @peterwilsoncc can you take a look at this when you have a minute?

Last edited 2 months ago by SirLouen (previous) (diff)
Note: See TracTickets for help on using tickets.