WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 2 months ago

#52326 closed defect (bug) (fixed)

`get_post_status()` is can be incorrect for attachments.

Reported by: peterwilsoncc Owned by: peterwilsoncc
Milestone: 5.7 Priority: normal
Severity: normal Version: 2.2
Component: Media Keywords: has-unit-tests has-patch has-dev-note
Focuses: Cc:

Description (last modified by peterwilsoncc)

get_post_status() can return an incorrect status for attachments in the following circumstances:

  • Attachment with inherit status from future post: returns 'publish', ought to be 'future'
  • Attachment with status draft and no parent: returns 'publish', ought to be 'draft' (can't be done in admin, only via code)
  • Attachment with inherit status and post_parent set to nonexistent post: returns false, ought to be 'publish'.

I've attached unit tests to demonstrate.

Attachments (1)

52326.diff (3.8 KB) - added by peterwilsoncc 4 months ago.

Download all attachments as: .zip

Change History (13)

@peterwilsoncc
4 months ago

#3 @peterwilsoncc
4 months ago

In PR #881:

  • Unit tests
  • A valid attachment ID can no longer return false as the status
  • attachment with inherit status always inherits parent posts status unless trashed (need to confirm this is correct)
  • trashed posts return the status immediately prior to the post been trashed using the meta data
  • if the meta data is not set/the parent post does not exist the attachment defaults to publish.
  • attachments are now passed through the get_post_status filter, they bypassed it before.

#4 @peterwilsoncc
4 months ago

  • Description modified (diff)
  • Keywords needs-dev-note added
  • Milestone changed from Awaiting Review to 5.7
  • Owner set to peterwilsoncc
  • Status changed from new to assigned
  • Version set to 2.2

Putting this on the milestone as it's going to be helpful for #5272.

Will need a comment in the general dev note (rather than a dedicated one) that the get_post_status filter now runs for attachments too.

trash changes were introduced in 4.0.0 but various other bugs have been around since the function was introduced in 2.2.0.

#5 @peterwilsoncc
4 months ago

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

In 49985:

Media: Ensure get_post_status() returns correct result for attachments.

Prevent get_post_status() returning false for attachments if the parent post has been deleted. The returned attachment post status is now passed through the get_post_status filter.

Add tests for get_post_status().

Props peterwilsoncc, timothyblynjacobs for review.
Fixes #52326.

#6 @SergeyBiryukov
4 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

// Attachment permitted statuses: 'inherit', , see wp_insert_post().

Did this list mean to include something other than inherit? I assume publish, but wanted to double-check :)

#7 @peterwilsoncc
4 months ago

@SergeyBiryukov Failed cut and paste when I moved it to the elseif below. Fix incoming.

#8 @peterwilsoncc
4 months ago

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

In 49986:

Media: Ensure get_post_status() does not contain half a comment.

Removes a misplaced comment, follow up to [49985].

Props SergeyBiryukov.
Fixes #52326.

#9 follow-up: @joyously
4 months ago

A valid attachment ID can no longer return false as the status
attachment with inherit status always inherits parent posts status unless trashed (need to confirm this is correct)
if the meta data is not set/the parent post does not exist the attachment defaults to publish

publish seems like the wrong default for an attachment with no parent. Where is it published? I would suggest draft in this case.

Is this marked for a dev note? There could be plugins checking for false.

#10 in reply to: ↑ 9 @peterwilsoncc
4 months ago

Replying to joyously:

publish seems like the wrong default for an attachment with no parent. Where is it published? I would suggest draft in this case.

For trashed parents, attachments have worked like this for a long time (at least as far back as 4.4.0). An attachment uploaded with one post as the parent may be published on another and WP Core offers no protections against using files in the uploads folder.

Is this marked for a dev note? There could be plugins checking for false.

Yes, this is marked for a dev note.

#12 @SergeyBiryukov
2 months ago

  • Summary changed from `get_post_status()` is can be incrorrect for attachments. to `get_post_status()` is can be incorrect for attachments.

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


2 months ago

Note: See TracTickets for help on using tickets.