WP_Upgrader: writes temporary directory name to plugin folder if zip doesn't have directory

[basszje]

The issue described in ticket:30945 occurs now in 4.4 as well.

If you serve a plugin zip with the files in the root directory it writes the plugin to the directory including the random string generated by wp_tempnam in file.php. This did not happen in 4.3.

This means this will occur with every plugin not having a directory in the ZIP itself.

In the mentioned ticket it is stated WP expects a plugin to have a directory name. Then why is the plugin upgrader checking if there are multiple files? In this case wouldn't it be better to gracefully return an error?

This block is in class-wp-upgrader.php :

451                     if ( 1 == count( $source_files ) && $wp_filesystem->is_dir( trailingslashit( $args['source'] ) . $source_files[0] . '/' ) ) { //Only one folder? Then we want its contents.
452                             $source = trailingslashit( $args['source'] ) . trailingslashit( $source_files[0] );
453                     } elseif ( count( $source_files ) == 0 ) {
454                             return new WP_Error( 'incompatible_archive_empty', $this->strings['incompatible_archive'], $this->strings['no_files'] ); // There are no files?
455                     } else { // It's only a single file, the upgrader will use the folder name of this file as the destination folder. Folder name is based on zip filename.
456                             $source = trailingslashit( $args['source'] );
457                     }

The first condition happens on 'wrong' plugin formats, resulting in a faulty plugin directory (with temp string) in the plugins dir. The last condition is the one that always should be expected when the root of the plugin zip is a directory.

[dd32]

Zips without a folder are officially unsupported, but for user experience, we attempt to do as best as we can.

I guess we should take the suffixed data into account in this case though and strip the final 7 random characters off the foldername.

[connorjburton]

I just experienced this (and have been trying to figure it out for days now) and it's only because of this ticket I figured it out, some way of notifying the user that we are doing something wrong would be great, even if it throws an error or notice.

Edit: mine was a theme, but same thing

[kirasong]

Punting from 4.5 because this doesn't have a patch, and we're mid-beta.

Feel free to re-milestone if a viable patch emerges.

[SergeyBiryukov]

Related: #36373

[poena]

Using the testing instructions and code provided in #30476, (with updated remote link), I was able to reproduce this by remotely fetching a plugin zip file that had no directories inside of it.

The name of the zip file was hello-dolly.zip and the resulting folder name were in the likes of "hello-dolly-B3W8Xu", "hello-dolly-moENyt".

The random name is generated at wp_tempnam()​ via download_url()​.

See WP_Upgrader::download_package()​.

[francina]

Replying to dd32:

Zips without a folder are officially unsupported, but for user experience, we attempt to do as best as we can.

I will close this ticket since the developer hub has some best practices for plugin format and those should be followed: ​https://developer.wordpress.org/plugins/plugin-basics/best-practices/#folder-structure
Re: https://core.trac.wordpress.org/ticket/14781

Thank you everyone for the discussion over the years.