WP_Upgrader: Incorrect plugin folder after updating from remote (non-WP) URL

[miyarakira]

I have a plugin that fetches its updates via a remote URL - it's hosted on my own server, not WordPress.org. The plugin hooks into the site_transient_update_plugins filter to integrate with the native plugin update process.

The issue I've run into is that after performing an update, the plugin is in a new folder with an incorrect name. It happens under two conditions:

1) If the download URL is in the syntax http://example.com/updates/my-plugin.zip, then the plugin folder is renamed to my-plugin.tmp instead of my-plugin.

2) If the download URL is in the syntax http://example.com/updates/?action=download&slug=my-plugin (the server interprets this and returns the zip file), then the plugin folder is renamed to actiondownloadslugmy-plugin, based on the query string.

I've been tracing the root cause of the issue in wp-admin/includes/class-wp-upgrader.php, and it's happening in WP_Upgrader::run(). The plugin is downloaded into a temporary folder via download_package(). It uses download_url($package) which calls wp_tempnam($url) to create the temporary folder name. Then it's unpacked, and by the time it gets to install_package() the destination directory is different from the original plugin folder. It seems that before install_package(), the plugin folder should be renamed from the temporary one to the correct one, based on the information available in $package.

I checked how a plugin hosted on WordPress.org is updated. The options passed to WP_Upgrader::run() are the same, with the download URL pointing to a zip file. I wonder where it's going wrong in my case.. Could it be that the zip file itself is the problem? I was having another issue there where if the zip contained a root folder with all the files, the update process would throw an error that the plugin was "empty".

It would be great if someone could help me figure this out..

[dd32]

Can you provide example code which shows the case?

I suspect the problem is that the zip it's downloading does not match the WordPress.org plugin zip format. The plugin must be self-contained within 1 folder in the zip, named appropriately (so 'my-plugin' in your example).

[miyarakira]

zip file of a plugin that installs fine but updates with wrong folder name

[miyarakira]

Thank you for the response. I have attached the zip file in question. It installs fine via Plugins -> Add New -> Upload. If you change the plugin version to, say, 0.0.1 and perform an update, that's when the folder name changes.

[dd32]

Replying to miyarakira:

It installs fine via Plugins -> Add New -> Upload. If you change the plugin version to, say, 0.0.1 and perform an update, that's when the folder name changes.

Yep, it doesn't match the zip format WordPress expects for updates.
Currently it's like this:

plugin.zip
+ plugin.php
+ readme.txt

where it needs to be like:

plugin.zip
+ plugin/
  - plugin.php
  - readme.txt

We can do better and fix the case where it ends up as plugin.tmp quite easily, but then the other case of actiondownloadslugmy-plugin isn't fixable, unless you add the extra directory to the zip file.

[miyarakira]

I see.. Yes, I had to resort to the first format, because if I put all files in a root folder as in the second format, I encountered an error: "The package could not be installed. The plugin contains no files." I can provide a zip example of this as well.

As for the query string, I worked around this issue by modifying .htaccess in the update server, so that a request to updates/*.zip will get handled by the updater's index.php. That way I can specify the zip file in the download URL (as well as the real request in the query string).

It would be helpful if the folder name plugin.tmp could be fixed for zip files in the first format. I believe somewhere in WP_Upgrader::run() between unpack and install package?

[dd32]

Replying to miyarakira:

I see.. Yes, I had to resort to the first format, because if I put all files in a root folder as in the second format, I encountered an error: "The package could not be installed. The plugin contains no files." I can provide a zip example of this as well.

Works for me with a WordPress.org-provided zip, if you match that format it should work.

[miyarakira]

Example of a zip that can't be installed:"Plugin contains no files"..but it does

[miyarakira]

This enables plugin update into correct folder, for the first zip format

[miyarakira]

I attached an example of a zip file in the second format (I believe). It throws an error during install: "Plugin contains no files". But it does - you can extract the files just like the first format.

I also attached a draft version of a patch which enabled me to update the plugin, zipped in the first format (no root folder).

Inside install_package(), it checks if the destination folder is protected - and during plugin update, destination is the path to wp-content/plugins. Since that's protected, it rewrites the destination based on $source - the temporary folder - so it becomes wp-content/plugins/my-plugin.tmp/. During plugin update, there's an array of "hook_extra" passed to the function, and it contains the plugin attribute in the syntax: "my-plugin/my-plugin.php". So, if that's available, I get the plugin's top directory and use it instead of the temporary one in $source.

Not the most elegant way to do it at the moment, but it works: I can now update the plugin and it will stay in the correct folder.

[dd32]

I attached an example of a zip file in the second format (I believe). It throws an error during install: "Plugin contains no files". But it does - you can extract the files just like the first format.

The folder name in the ZIP is invalid, the files are stored as ../readme.txt instead of plugin-slug/readme.txt, which you can check on the command line with unzip -l file.zip, so the ../ is being striped during decompression.

Originally there's a reason why it respects the foldername in the zip archive, it was originally designed that if you'd installed a plugin in plugins/some-fancy-plugin-slug, but it was plugin-slug on WordPress.org, that it would remove plugins/some-fancy-plugin-slug and instead put it into plugins/plugin-slug.
Switching it to use the existing directory, when it's going to use a temporary folders filename during update seems like a valid solution though.

[miyarakira]

The folder name in the ZIP is invalid, the files are stored as ../readme.txt instead of plugin-slug/readme.txt, which you can check on the command line with unzip -l file.zip, so the ../ is being striped during decompression.

Aah, I see. I didn't know a way to inspect the structure of the zip file, so was only seeing the result after decompression.

During the plugin build, I'm using Gulp to zip everything and I'm limited as to the parameters I can pass to the zip process: the first format (no base folder) or second format specifying the base folder (my-plugin/). Apparently the latter is not working as I expected. In both cases though, I can decompress them and the result is the same: all files in a base folder with the correct slug. So it would be helpful if WordPress could accommodate one or the other format.

Originally there's a reason why it respects the foldername in the zip archive, it was originally designed that if you'd installed a plugin in plugins/some-fancy-plugin-slug, but it was plugin-slug on WordPress.org, that it would remove plugins/some-fancy-plugin-slug and instead put it into plugins/plugin-slug.
Switching it to use the existing directory, when it's going to use a temporary folders filename during update seems like a valid solution though.

That makes sense - it seems logical that before and after an update, the plugin should stay in the same folder. So perhaps a better patch would be to store the name of the original plugin directory, then when the temporary folder is being moved back, to restore its name?

[miyarakira]

I finally figured out how to package the zip file as WordPress expects.

This is not so relevant here as it pertains to gulp-zip, but the correct way was:

 return gulp.src(files, { base:'../' })
   .pipe(zip(slug+'.zip'))
   .pipe(gulp.dest(dest));

Setting the base directory to ../ (one above the plugin folder) zips all files under a root folder with the plugin name. I was able to confirm this using unzip -l plugin.zip and listing its contents, as well as installing and updating the plugin.

OK, so I believe this ticket can be closed without further ado. Thank you so much for the input! Today I filed two tickets, set up a dev version of WordPress via SVN so I can submit patches, and dug around in core like never before. Whew, that was sure a learning process.

[danielbachhuber]

Reported to WP-CLI too: ​https://github.com/wp-cli/wp-cli/issues/1619

Aside from the conversation about folder structures, I think wp_tempnam() shouldn't clobber extensions associated with archive formats and / or WP_Upgrader::run() should know to clean up .tmp from the directory name. The latter is likely a more minor decision.

[danielbachhuber]

I've put together a patch that fixes the .tmp issue. The commit message is here ​https://github.com/danielbachhuber/wordpress-develop/pull/1

[DrewAPicture]

@dd32: Could you possibly follow up on @danielbachhuber's patch? I was going to ask about the possibility of tests, but I'm not sure we can mock that environment. Perhaps you know.

[dd32]

I'd like to be able to mock the update environment, but we can't really right now.

[dd32]

In 31863:

Upgrader: When creating a temporary working directory, strip off .tmp as well as .zip incase we end up using that directory as the items main directory.
Fixes #30945

[dd32]

#30476 was marked as a duplicate.