Make WordPress Core

Opened 8 years ago

Closed 3 years ago

Last modified 3 years ago

#35217 closed defect (bug) (invalid)

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

Reported by: basszje's profile basszje Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Upgrade/Install Keywords: close
Focuses: Cc:


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.

Change History (9)

#1 follow-up: @dd32
8 years ago

  • Component changed from Plugins to Upgrade/Install
  • Milestone changed from Awaiting Review to 4.5

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.

#2 @connorjburton
8 years ago

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

Last edited 8 years ago by connorjburton (previous) (diff)

#3 @kirasong
8 years ago

  • Milestone changed from 4.5 to Future Release

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.

#5 @poena
3 years ago

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 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().

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core-auto-updates by afragen. View the logs.

3 years ago

This ticket was mentioned in Slack in #core-auto-updates by francina. View the logs.

3 years ago

#8 in reply to: ↑ 1 @francina
3 years ago

  • Resolution set to invalid
  • Status changed from new to closed

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:

Thank you everyone for the discussion over the years.

#9 @francina
3 years ago

  • Keywords close added
Note: See TracTickets for help on using tickets.