[wp-trac] [WordPress Trac] #57500: Plugin update results in mixing files of old and new versions

[WordPress Trac]

#57500: Plugin update results in mixing files of old and new versions
-----------------------------+------------------------------
 Reporter:  Chouby           |       Owner:  (none)
     Type:  defect (bug)     |      Status:  new
 Priority:  normal           |   Milestone:  Awaiting Review
Component:  Upgrade/Install  |     Version:
 Severity:  normal           |  Resolution:
 Keywords:  needs-testing    |     Focuses:
-----------------------------+------------------------------
Changes (by costdev):

 * keywords:   => needs-testing


Comment:

 Hi @Chouby, thanks for opening this ticket!

 Posting some information to aid discussion and investigation.

 The upgrade order of operations is:
 1. Download the new package to `/upgrade`.
 2. Unpack the new package to `/upgrade`.
 3. Delete the old version from `/plugins`.
 4. Copy the new version from `/upgrade` to `/plugins`.

 It seems the issue occurs between 3 and 4.

 -----

 For the deletion of the old version:
 1. `WP_Upgrader::install_package()` calls
 `WP_Upgrader::clear_destination()`.
 2. This attempts to delete the old version and, upon failure, returns a
 `WP_Error` object.
 3. Otherwise, the hooked `Plugin_Upgrader::delete_old_plugin()` runs to
 verify the deletion. It:
   - Checks if the delete result is already a `WP_Error` object and if so,
 returns the error immediately. If not, then it:
   - returns a `WP_Error` object if the arguments passed to it are
 incorrect.
   - returns the original value from `WP_Upgrader::clear_destination()` if
 the old plugin directory doesn't exist (i.e. there's nothing left to
 delete).
   - will check if the plugin directory still exists, if so, tries to
 delete the directory again, and returns a `WP_Error` object upon failure.
   - returns `true` if nothing failed along the way.
 4. Both `WP_Upgrader::clear_destination()` and
 `Plugin_Upgrader::delete_old_plugin()` return `true|WP_Error` (some docs
 may say `bool|WP_Error`, but there is no `false` outcome from what I can
 see).

 -----

 For the copying of the new version into place:
 1. If `WP_Upgrader::clear_destination()` returns a `WP_Error` object, then
 `WP_Upgrader::install_package()` returns the same object and the new
 version is not copied into place.
 2. Otherwise, the new version is copied into place.

 -----

 @afragen, @pbiron, please fact-check all of the above to ensure the
 discussion continues with accurate information. It's likely that if this
 is a Core issue, something above is not working as expected.

 -----

 While it seems quite a number of users have encountered this issue, this
 still needs testing to determine the exact reproduction steps, and to dig
 down into the point of failure. Adding `needs-testing`.

 @Chouby if there is any more information that comes through from support
 tickets that could help in this effort, posting i in this ticket would be
 much appreciated!

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/57500#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform