[wp-trac] [WordPress Trac] #26809: Errors from wp_update_post obscured in edit_post

[WordPress Trac]

#26809: Errors from wp_update_post obscured in edit_post
--------------------------+-----------------------------
 Reporter:  dllh          |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  General       |    Version:  trunk
 Severity:  normal        |   Keywords:
--------------------------+-----------------------------
 A sample reproduction of a bug that illustrates the general case I'd like
 to document:

 1. Activate a theme that has page templates (e.g. twentyeleven).
 2. Enable Jetpack and configure the Sharing module, which does things on
 `save_post` (or set up something else that does things on `save_post`).
 3. Write a post and select a template (e.g. `single-page.php`).
 4. Switch to a theme (e.g. twentythirteen) that doesn't have page template
 UX in the page editing screen.
 5. Toggle the "Show sharing buttons" setting in the relevant metabox (or
 do something else that will occur on `save_post` that's easily detected
 after saving).
 6. Save the post.

 Expected Result: If there's a failure saving any piece of the data, the
 user'll be notified, and possibly the whole save will be postponed until
 any errors are corrected.

 Actual Result: Changes to the main post fields are saved, but things that
 would have fired on `save_post` do not happen, resulting in a partial save
 of the data (e.g. sharing status is not saved). The code in `wp-
 includes/post.php` that checks for a valid page template returns before
 several actions (including `save_post`) fire because the valid template
 check fails. The failure is essentially silent because `edit_post()`
 doesn't check the return of `wp_update_post()`.

 In the particular case I outline above, the issue arises because post meta
 for the page template existed and lives in `$postarr` based on a save when
 the old theme was activated. When saved under a theme with no page
 template UX, the value from meta is used but is not a valid value for the
 new theme. So it correctly errors, but the early return causes later
 actions not to fire properly, wreaking havoc that's tricky to spot.

 Optimally, it seems like WP would at least check the return of
 `wp_update_post( $foo, true )` for `is_wp_error()` in `edit_post()` and
 would fail gracefully (or at least `wp_die()` as in other cases) if an
 error was returned, but then there are other issues as well, such as the
 fact that some meta values are saved in `edit_post()` before the
 `wp_update_post()` executes, and theoretically these'd need to be rolled
 back as well on failure.

 The template check in `wp-includes/post.php` should also be moved to
 higher up in the code than where it currently lives so that the check
 would bail prior to the database update.

 I imagine that there might be much larger architectural issues that would
 subsume any changes to the way `edit_post()` works, so rather than
 providing a fix for the specific issue that'd likely be rejected anyway, I
 wanted to raise the general issue for discussion/rejection/whatever.
 Possibly relevant: #21963.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/26809>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress blogging software