WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 3 months ago

#21970 new defect (bug)

404 error when a post has the same slug as with a deleted (trash) page.

Reported by: janintia Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4.2
Component: Permalinks Keywords: has-patch needs-unit-tests 4.0-early
Focuses: Cc:

Description

I don't know if this is a bug/issue but I'll share this anyway.

A user creates a new Page, for example, with a title "New Garden". But decides to delete it/"Move to Trash" without deleting it permanently. Then creates a new Post with the same title "New Garden". Viewing that post will result to a "404" page not found.

Attachments (5)

21970.patch (808 bytes) - added by SergeyBiryukov 2 years ago.
21970.diff (812 bytes) - added by kovshenin 2 years ago.
Check the page post status before verifying a match
21970.2.diff (1.6 KB) - added by SergeyBiryukov 2 years ago.
21970.3.diff (1.6 KB) - added by SergeyBiryukov 2 years ago.
21970.tests.diff (1.1 KB) - added by igmoweb 3 months ago.
Unit test

Download all attachments as: .zip

Change History (25)

comment:1 @SergeyBiryukov2 years ago

  • Severity changed from trivial to normal

Reproduced with /%postname%/ permalink structure.

comment:2 @SergeyBiryukov2 years ago

  • Component changed from General to Permalinks
  • Keywords needs-patch added

@SergeyBiryukov2 years ago

comment:3 @SergeyBiryukov2 years ago

  • Keywords has-patch added; needs-patch removed

Related: #21298

Perhaps get_page_by_path() should not return trashed pages.

comment:4 @DrewAPicture2 years ago

Related: #11863 - Trashed items interfere with page/post slug generation

comment:5 @SergeyBiryukov2 years ago

#22210 was marked as a duplicate.

@kovshenin2 years ago

Check the page post status before verifying a match

comment:6 @kovshenin2 years ago

In 21970.diff, verifying that the page has a visible post status (public, private or protected). I first thought of checking against the read_post cap, but that might create a really confusing scenario, where a logged in user will see the page, while a logged out user will see the post.

@SergeyBiryukov2 years ago

comment:7 @SergeyBiryukov2 years ago

  • Keywords needs-unit-tests added

21970.diff looks good to me.

21970.2.diff also handles the similar block in url_to_postid().

comment:8 @SergeyBiryukov2 years ago

  • Milestone changed from Awaiting Review to 3.6

@SergeyBiryukov2 years ago

comment:10 @SergeyBiryukov2 years ago

21970.3.diff makes the condition syntax a bit more obvious.

comment:11 follow-up: @sc0ttkclark23 months ago

A similar issue happens with get_page_by_title() (pulls from trash), I may be submitting a patch for that separately as well depending on dev feedback on whether core is open to the adjustment.

comment:12 @sc0ttkclark23 months ago

  • Cc lol@… added

comment:13 in reply to: ↑ 11 @SergeyBiryukov23 months ago

Replying to sc0ttkclark:

A similar issue happens with get_page_by_title() (pulls from trash)

Related: #20350

comment:14 @markjaquith20 months ago

  • Milestone changed from 3.6 to Future Release

Punting this as we're focusing on 3.6 blockers now.

comment:15 @SergeyBiryukov20 months ago

#24817 was marked as a duplicate.

comment:16 @SergeyBiryukov13 months ago

  • Milestone changed from Future Release to 3.9

comment:17 @nacin11 months ago

  • Milestone changed from 3.9 to Future Release

This needs unit tests like whoa.

comment:18 @samuelsidler11 months ago

  • Keywords 4.0-early added

comment:19 @SergeyBiryukov4 months ago

#30047 was marked as a duplicate.

@igmoweb3 months ago

Unit test

comment:20 @igmoweb3 months ago

Added Unit Tests.

I didn't know where to place this one so I decided for tests/post/objects.php. Now that I read it again sounds weird for me :).

Note: See TracTickets for help on using tickets.