Plugin update results in mixing files of old and new versions

[Chouby]

As far as I know, during an update, WordPress completely deletes the plugin directory before unzipping the new plugin files.

However we had ​several cases​of users reporting ​a fatal error ​after the update to Polylang 3.3.1 which can be explained only by the fact that the old files were not removed and new files were just merged to the existing old plugin. Indeed the error is reported in a file that has been removed in the new version.

I initially thought that users were manually updating the plugin by overwriting files, but they confirm that they update the plugin from the WP backend or have auto-update activated.

The update probably runs well for most users. I could not reproduce the issue myself.

[costdev]

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 /tmp.
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 it in this ticket would be much appreciated!

[pbiron]

@costdev I can confirm that the steps you outline are correct.

@Chouby I can also confirm that a successful update to Polylang 3.3.1 does not result in the errors reported in the various support form posts you link to: I'm an active user of Polylang (thanx for a great plugin) and updated a number of sites.

This leads me to 2 questions:

1. what additional plugins are installed on the affected sites (of the users reporting problems)?
2. what WP_Filesystem->method is used on the affected sites (direct, ssh2, ftpext, or ftpsockets)?

[afragen]

@Chouby is it possible that some add-on plugin is trying to load this now removed module?

I think we need to know what additional plugins are installed on these sites. If this is the case, this error will happen until these add-ons have been updated. It also explains why the previous version works.

Though it appears that class-polylang.php:line 244 is still reporting the modules/lingotek directory.

[Chouby]

I'll try to collect more informations from users reporting the issue.

@afragen We are using a similar modules structure as Jetpack. Our modules are automatically loaded by the line you pointed. We just need a file load.php in a subdirectory of modules/ to load it. That explains why a file from the old version is loaded by the new plugin. The fatal itself is due to the fact that and old class from an old file is loaded and is not in the composer autoloader classmap as the class has been removed from the new version.

We can manually reproduce the error by:

1. install the version 3.3
2. overwrite the files with the version 3.3.1 without deleting the version 3.3

So we don't need an addon to attempt to load a non existing file. When we examine the backtrace provided by the users, it's all internal to Polylang. Reinstalling properly the plugin solved the issue.

[JasonHouge]

I am not sure if this is even related to the issue above,
but very recently across the various sites I manage all plugin auto updates have failed since updating to WP 6.1.1. So, I am left to manually perform them. Over the past night an even stranger thing started to happen where the directory contents for random plugins just disappear leaving empty directories. I've since disabled auto updates and I'm trying to determine what could possibly be causing it besides the latest WordPress core... Embarrassingly, I am the hosting provider, so there is no tech support for me to turn to and I'm afraid I'm not quite savvy enough to know exactly what I'm looking for.

[afragen]

@JasonHouge are they really random plugins with empty directories or were they plugins with updates?

If you’d like to try something you can install ​Faster Updates and ​Rollback Update Failure and see if these issues persist.

[JasonHouge]

Replying to afragen:

@JasonHouge are they really random plugins with empty directories or were they plugins with updates?

If you’d like to try something you can install ​Faster Updates and ​Rollback Update Failure and see if these issues persist.

It's funny you should list Rollback Update Failure, as that was one that was left empty this morning, and I see it was listed as updated 21 hours ago. So, it looks like perhaps they are not random.

[Chouby]

Just an update which unfortunately doesn't help much. I received 3 site healh for users reporting the issue. There are no plugins which are common to the 3 installs, except Polylang of course.

[barry.hughes]

I'm a little unsure if this is precisely the same issue, but we've also noticed the potential for code from two different versions of the same plugin to be loaded in the space of the same request. Specifically, it seems this can happen when updating a plugin by manually uploading a zip of the latest version.

To replicate:

1. Via the Plugins ‣ Add New ‣ Upload screen, upload foobar.1.0.zip and activate.
2. Then, using the same admin screen, upload foobar.2.0.zip ("Replace current with uploaded").
3. Either directly on the screen (depending on your error display settings) or else via your error log, you should observe a fatal error as follows:

Fatal error: Uncaught Error: Class "Bar" not found
in /.../wp-content/plugins/foobar/inc/foo.php on line 6

If you look at the code in those sample plugins you will see it is a pretty contrived example (the class loader is very simplistic), and a different autoloading strategy would solve the above error. Even so, it seems sub-optimal and potentially risky to mix code from two different versions in this way.

Things I'm unsure about:

• If this is essentially the same problem as Chouby reported, or different.
• If it is best solved from WordPress, or if it is something individual plugins should mitigate.

[barry.hughes]

Foobar 1.0

[barry.hughes]

Foobar 2.0

[afragen]

I think WP 6.2, due to be released tomorrow, should mitigate this type of issue. Can you possibly check again using WP6.2 and see if the issue persists?

[afragen]

Usually this is during the update process. Barry I’ll see if I can reproduce it.

[barry.hughes]

Thanks, Andy ... it still seems possible to replicate (using the uploaded sample plugins) with 6.2-RC3.

[barry.hughes]

Though, looking again at the forum posts Chouby shared (like ​this one—"I cannot access my wp-admin page"), perhaps this is different—because the problem I've described is transitory (it clears on the very next request), whereas that doesn't seem to be the case in some of those other reports.

[afragen]

@barryhughes I can certainly reproduce your results. When I step through in xDebug what I see happening is the "update" to v2.0 occurs but before the new files are transferred the v1.0 spl_autoload_register() runs, then the files switch and Bar::bar() is called, however, the newer version of the spl_autoload_register() has not yet run. This is why the error displays. Obviously the next pageload and hence the next run will be correct.

I think the primary issue can best be described as the the old plugin loading between the time it is removed and replaced with the new plugin.

I do think this might be a similar issue to @Chouby. As you suggest a more robust autoloader would solve this issue. Essentially an autoloader must be identical between the releases. The autoloader must load files in a specific directory or by interpolating the $class and the file name. The issue occurs when specific files are mentioned in the spl_autoload_register().

In the case of Polylang perhaps a transition from Composer's classmap autoloader to PSR-0 might work. I believe that specifically naming the files as occurs here, and loading them in the autoloader is the issue.

[barry.hughes]

Hey @afragen, thanks for checking on it!

As you suggest a more robust autoloader would solve this issue. Essentially an autoloader must be identical between the releases.

Yep, though, possibly worth calling out that essentially this same problem (mixing of code from two different versions) can be replicated with the standard Composer autoloader—or even with no autoloader whatsoever.