WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 5 months ago

#30009 assigned defect (bug)

_wp_put_post_revision checks for errors incorrectly

Reported by: rmccue Owned by: rmccue
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 2.6
Component: Revisions Keywords: has-patch dev-feedback
Focuses: Cc:
PR Number:

Description

_wp_put_post_revision uses wp_insert_post, then checks is_wp_error to pass it up the stack (lines 259-261). However, wp_insert_post only returns a WP_Error if the second param is true. In this case, it is not.

Introduced in r7907.

Attachments (4)

30009.diff (497 bytes) - added by rmccue 5 years ago.
Specify we want a WP_Error on insertion error
30009.2.diff (1.3 KB) - added by rmccue 5 years ago.
Correct some coding standards while we're in here
30009.3.diff (1.5 KB) - added by adamsilverstein 5 years ago.
update docblock
30009.4.diff (482 bytes) - added by adamsilverstein 4 years ago.

Download all attachments as: .zip

Change History (8)

@rmccue
5 years ago

Specify we want a WP_Error on insertion error

@rmccue
5 years ago

Correct some coding standards while we're in here

#1 follow-up: @rmccue
5 years ago

  • Keywords has-patch added
  • Owner set to rmccue
  • Status changed from new to assigned

Attached patches specify that we want a WP_Error back from wp_insert_post, pretty straight forward patch.

30009.diff fixes the problem, 30009.2.diff goes a bit further and corrects coding standards in the function while we're in there.

@adamsilverstein
5 years ago

update docblock

#2 in reply to: ↑ 1 @adamsilverstein
5 years ago

looks good. I updated the doc block slightly in 30009.3.diff to reflect the return value.

I tried to write a unit test for this and couldn't actually get wp_insert_post to return an error - it only does so when get_post( post_id ) fails (returns null) and that check already happens earlier in _wp_put_post_revision. For this reason, I believe we could remove the entire error check conditional, we are returning $revision_id in any case...

Good catch in any case, the check as is will never work!

Replying to rmccue:

Attached patches specify that we want a WP_Error back from wp_insert_post, pretty straight forward patch.

30009.diff fixes the problem, 30009.2.diff goes a bit further and corrects coding standards in the function while we're in there.

#3 @adamsilverstein
4 years ago

  • Keywords dev-feedback added

30009.4.diff

  • Refresh against trunk

Lets keep this patch simple and stick to adding the true value to wp_insert_post so it will potentially return the WP_Error we are looking out for.

Note: See TracTickets for help on using tickets.