_wp_put_post_revision checks for errors incorrectly

[rmccue]

_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.

[rmccue]

Specify we want a WP_Error on insertion error

[rmccue]

Correct some coding standards while we're in here

[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.

[adamsilverstein]

update docblock

[adamsilverstein]

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.

[adamsilverstein]

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.

[adamsilverstein]

Fine leaving closed, this was fixed in WordPress 5.0: ​https://github.com/WordPress/wordpress-develop/blob/c2390f96a212aa57afe076bfe098c5b1676f5ec6/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L365

[coreymckrill]

Just came here to report this same issue. Looks like 30009.4.diff​ would be a quick fix.

Trac ticket: https://core.trac.wordpress.org/ticket/30009

[adamsilverstein]

@SergeyBiryukov I added a test and am running all tests in ​https://github.com/WordPress/wordpress-develop/pull/1279.

If everything looks good, this is ready for commit. Before this fix is_wp_error on line 274 would never pass!

[adamsilverstein]

30009.5.diff​ includes a test to verify a WP_Error is returned now. I was able to trigger an error by passing an invalid post ID directly to the inner function.

[whyisjake]

In 51124:

Revisions: Check and return errors for insertions to revisions.

Fixes #30009.

Props rmccue, adamsilverstein, coreymckrill, whyisjake.