Make WordPress Core

Opened 10 years ago

Closed 8 years ago

#31138 closed defect (bug) (maybelater)

Backup of a plugin / theme directory.

Reported by: aercolino's profile aercolino Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.1
Component: Upgrade/Install Keywords: has-patch
Focuses: Cc:

Description

Original Summary: Backup of a plugin / theme directory.

A plugin of mine allows users to add custom files to the plugin directory. I am almost ready with my shining new version BUT I cannot let users update with the magnificent update now link. Let me spend a couple of words of appreciation to those who made that possible. I LOVE IT. Easily the best feature of the last years, together with the same functionality for all the blog update. wow.

I've looked around but I couldn't see any right way to allow my users to effortlessly migrate their stuff. In fact I looked into the code and saw that the previous version of a package is implacably deleted.

I have developed a patch for it (against release 4.1). It's quite simple. From the WP_Upgrader::install_package method, before actually deleting the $remote_destination, it is backed up. If that completed successfully, then a transient is set to store backup metadata. Later a pair of functions (backup_prefix and get_transients_like) allow plugin / theme developers to get to all available backups of their package.

/**
 * Get the prefix of the backup name, compatible with transient names.
 *
 * The backup name is used for both a transient (which stores backup metadata) and a temporary directory (which
 * stores backup files and directories).
 *
 * Plugin/theme names get truncated after 21 characters and it's possible (but quite improbable) that two different
 * packages have exactly the same first 21 characters. In such a case there would be a prefix clash. However, to
 * tell them apart, you can check the original plugin/theme directory name which is stored into the transient.
 *
 * Notice that many backups can potentially exist for each plugin/theme at the same time. The number of backups is at
 * most the number of times the user updated to the latest version, which can occur many times in a row against one
 * new version (due to user's retries) or many successive versions (due to frequent releases).
 *
 * Example for copying back in place user's customizations.
 * <code>
 * $plugin_dir = dirname(__FILE__);
 * $prefix = backup_prefix($plugin_dir, 'plugin');
 * $backups = get_transients_like("%$prefix%");
 * if (count($backups)) {
 *   $last = array_pop($backups);
 *   copy_dir($last['backup_dir'] . '/custom_files', $plugin_dir . '/custom_files');
 * }
 * </code>
 *
 * @param string $package_dir Plugin/theme directory.
 * @param string $type        Either 'plugin' or 'theme'.
 *
 * @return string The prefix.
 */
function backup_prefix( $package_dir, $type ) {
  • Please review everything.
  • Do I need to add some tests?

Will you add this patch please? I think it will be useful to everyone, not only to plugin / theme developers but most importantly to users, which will get this added protection for their custom files.

Attachments (4)

backup_package_functions.patch (2.4 KB) - added by aercolino 10 years ago.
backup_package_option.patch (2.8 KB) - added by aercolino 10 years ago.
backup_package_class-wp-upgrader.patch (4.2 KB) - added by aercolino 10 years ago.
backup_package_option.2.patch (2.8 KB) - added by aercolino 10 years ago.

Download all attachments as: .zip

Change History (23)

#1 follow-up: @toscho
10 years ago

  • Keywords close added

Always use the upload directory for custom files. This is the only location with guaranteed write access and a built-in collision prevention for a multisite.

#2 in reply to: ↑ 1 @aercolino
10 years ago

  • Keywords close removed

Replying to toscho:

Always use the upload directory for custom files. This is the only location with guaranteed write access and a built-in collision prevention for a multisite.

Good tip. Unfortunately, you are talking about how to implement custom files before you release a plugin, taking into account this WordPress bug. I'm talking instead about how to recover custom files many years after you released a plugin that used its same directory to store custom files (not sure about the write access: what did you mean? Is not WP_CONTENT_DIR always usable for plugins and themes?).

However here it is why I consider this a WordPress bug.

Universal mantra (1): Always make a backup of the stuff you are going to replace.
Universal mantra (2): Always keep your stuff together.

Why isn't WP_Upgrader making a backup of the packages before replacing them? Why is it WordPress forcing third party developers and users to have closely related stuff in different places?

This no-feature, the lack of a transparent backup, is a bug that appeared with the (fantastic) update now functionality. I mean, not only it didn't implement a backup, but before it was released, a transparent backup wasn't needed. In fact, each user was aware (before or after help requests) of the proper procedure to install / update a plugin.

Do you remember? First you had to download it to your PC, then you had to open an FTP connection to your site, then, if it was an update, you had to make a backup and then delete the old directory contents or, if it was a new install, you had to create a new directory, and lastly you could upload all plugin's files to that directory.

Did I say I love the update now link?

Last edited 10 years ago by aercolino (previous) (diff)

#3 @aercolino
10 years ago

  • Keywords has-patch added

#4 follow-up: @toscho
10 years ago

The plugin directory doesn’t have to be in the wp-content directory, it can sit in a completely separated directory, even with a separate domain. The uploads directory is—and always was—the only place where it is safe to write files.

This is not a bug. It is a design decision made many years ago.

#5 in reply to: ↑ 4 @aercolino
10 years ago

Replying to toscho:

The plugin directory doesn’t have to be in the wp-content directory, it can sit in a completely separated directory, even with a separate domain. The uploads directory is—and always was—the only place where it is safe to write files.

Fine, but that is unrelated to this issue.
The point is how to get to users's files that were in the directory of a plugin before it was automatically updated by WordPress.

This is not a bug. It is a design decision made many years ago.

Not because it is an old decision it cannot be a bug in hindsight.

#6 @aercolino
10 years ago

The problem of recovering user's files stored in a plugin or theme directory has one solution and some workarounds.

Solution
Make WP_Upgrader create a temporary backup before deleting (this is what my patch does).
I think this is the best way to do it. And I mean the best for everyone: The WordPress application, brand, community, plugin and theme developers and lastly end users. Users just click the update now link the way they are used to and it all magically works as it currently does.

Workaround 1 -- bad
Tell the end user that before updating they have to backup their files themselves.
This could be done with an upgrade notice. I just saw an upgrade notice from another plugin. It's clear that's not viable for this workaround. Even if it was more prominent, it's not guaranteed to work. Users don't read, neither when they are forced to, unless the message is very short, very prominent and unavoidable.

  • How can I test an upgrade notice for my yet to be released new plugin version?

Workaround 2 -- worse
Make the update now link unavailable altogether. This would force the user to update manually and ask themselves about why it was not possible to update automatically, which in the end should make them read the update instructions, and then I could have a bad surrogate of Workaround 1.

Workaround 3 -- worst
Put the new version in a different directory. This is possible but would make the update pointless. It would be a completely new plugin. It's really a bad workaround, IMO. Unless there is a way to preserve the identity of the plugin, but I doubt it.

---
Please tell me how to solve this.

Last edited 10 years ago by aercolino (previous) (diff)

#7 @dd32
10 years ago

Ultimately @aercolino, your plugin users are going to have a bad upgrade path no matter what WordPress does, or doesn't do.

  1. If WordPress 4.2 added some kind of save files during an update, your plugin would need to be updated first to specify what to backup = custom files deleted
  2. If WordPress 4.2 added some kind of way to have files retained during update automatically without the plugin doing anything (which might I add, would be impossible since plugins legitimately remove files during updates, ie. 1.1 won't have file.php while 1.0 did) = custom files deleted if a user updates the plugin before WordPress (which I should add, should be the standard way of updating a site)

Realistically, all plugins should never store data in it's own folder, and should store it in a custom folder in WP_CONTENT_DIR, or if it has to be writable by the webserver, possibly a subdirectory of the uploads folder.

You could add a Upgrade Notice and Changelog specifying that users should backup their data or move it to a different location before a plugin update, that's your best bet, unfortunately though many users will simply update (or it'll happen automatically on some hosts/configurations) without ever reading that. I can't think of an easy way to issue an update notice that won't cause file deletion, I think if your repo even only contains a readme.txt then the ZIP would still end up overwriting the plugin (although that could be worth testing). Unfortunately testing the update mechanisms isn't easy, the best way is probably using the 3rd-party script that pulls an update from Github.


This is not a bug. It is a design decision made many years ago.

Not because it is an old decision it cannot be a bug in hindsight.

It can definitely be seen as a bug, or as a feature, depending on how you look at it. If I was implementing it from scratch again, I'd probably have used something slightly different, but the end result would still be the same, to remove all the existing files, and replace it with a new set.
This has been a problem/concern for Themes for a long time, people will edit the CSS/PHP files directly, then update the theme, and be shocked their customizations were lost. The proper method in that case is to have the modifications in a child theme.

#8 @aercolino
10 years ago

@dd32,

the end result would still be the same, to remove all the existing files, and replace it with a new set.

That is the way, no doubt. The point is that never ever we remove anything before making a backup. Or making sure that what we remove is exactly the same thing we already have somewhere else, which is what a backup guarantees.

This has been a problem/concern for Themes for a long time, ...

With changes to the code of a plugin / theme, the problem of how to make them persist after an update is quite different from what I'm discussing here and, I might say, much more complex to solve.

  • BUT, if WordPress doesn't automatically make any backup, we can be sure that the new plugin / theme version has no way to know if such changes were present in the first place.
  • INSTEAD, if WordPress automatically makes a backup, the new version can make the UX much much more pleasurable.
    For example, version 2 could compute a hash of the backup and compare it to what it should be, and if they differ it can take the steps it pleases. It could ask the user: "Hey admin, version 2 of Super Fantastic Theme speaking. I noticed you made some changes to version 1. If you proceed with the update your changes are not lost but you will have to apply them again by yourself. You can find your version 1 in the directory ... If you Cancel the update, your version 1 will be restored. [Proceed] [Cancel]".
    Ultimately, plugin / theme authors would have all they need to decide how to update.
  1. If WordPress 4.2 added some kind of way to have files retained during update automatically without the plugin doing anything

Sorry but I can't understand where is the problem... Here is how the update flow looks like after applying my patch.

  1. WordPress 4.2 implements automatic backup of a plugin / theme files. (This is my patch.)
  2. Users get notified that WordPress 4.2 has been released and it implements automatic backup to improve UX when updating. (This is only meant to be a bell-ringer later, when updating plugins / themes.)
  3. The new version of my plugin states that it requires WordPress 4.2. (I realize this is not perfect because users could update it before updating WordPress, however it does offer another bell-ringer to users.)
  4. Users get notified of the new version and click the update now link.
  5. WordPress 4.2 makes a backup of all the files in the directory of my plugin, then deletes the directory and puts there all the new files.
  6. The user or WordPress activates my plugin again and version 2 activation code will run.
  7. It will detect that this is an update and it will find the backup WordPress made.
    • It will copy user's files from the backup directory back in the plugin directory.
      (This is only my use case. Any plugin / theme author can do whatever they see fit.)

I don't remember how the Requires at least feature currently works, but I think that that is also quite a key piece of the puzzle. I mean, plugin / theme authors are very interested in making the minimum supported version of WordPress as low as possible. So it would make a lot of sense to strongly (actively?) discourage users from installing / updating to a plugin which requires a version greater than what they are using.


I looked into how the update works for WordPress core and we already allow new code to get executed in place. https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-upgrader.php#L2021-L2033

At some point I thought that something along those lines could be done for plugins / themes, for example by using an install.php script (nicely symmetrical to uninstall.php) and copy / include / execute it in a similar fashion, but then I realized that that would entail many changes to the current flow, and we'd have to educate about many details.

I'm now convinced that the most flexible and unobtrusive solution is to automatically make a backup of the plugin directory. Notice that it is a temporary backup. Some tuning would be required there, but the idea is that a scheduled garbage collector (not included in my patch) could get rid of expired stuff on a regular basis. Expired backup directories are easy to find: they are those without a corresponding (non-expired) transient with a name equal to the directory.

#9 @toscho
10 years ago

What happens with the old backup files? Will they stay forever?

What happens when the user runs out of disc space? This is a common problem for backup plugins. Think bulk updates.

If we implement this, plugin author will think it is allowed now to put custom files into the plugin directory. This would encourage the wrong behavior.

#10 @aercolino
10 years ago

@tosho,

What happens with the old backup files? Will they stay forever? ... Think bulk updates.

These backups are temporary by definition. Read what I put at the end of my previous comment. It can be tuned at will (also look into the patch, if you want). I wouldn't automatically remove them after activation for example, because it is safer to let some time / new posts pass.

This would encourage the wrong behavior.

What makes you say so? Why is it the wrong behavior?

  1. Before automatic updates, it was one of the right behaviors.
  2. After automatic updates but before automatic backups (current situation), it is wrong only because of this bug.
  3. After automatic backups, it will be again one of the right behaviors.

#11 @aercolino
10 years ago

I've not implemented a garbage collector. However, once it is available, we could expose the temporary directory and backup functionalities into a reusable API. Also notice that a GC for this could be as simple as a line in the admin panel informing the user that WordPress can free X bytes from expired backups, and a free space link.

example of WordPress tmp API

Last edited 10 years ago by aercolino (previous) (diff)

#12 follow-up: @aercolino
10 years ago

Would someone please tell me how to go for the second workaround I mentioned above?

Workaround 2 -- worse
Make the update now link unavailable altogether.

At the moment it seems the only viable alternative. Either that or refrain from sharing the update.

It's a pity there is little interest in fixing this now, and I'm surprised that it wasn't fixed before.

#13 @lgladdy
10 years ago

I saw your post on #29820 and figured it made more sense to reply here.

I'm actually against this from a mentality point of view. I think a better question is should the editors carry better warnings about those changes being lost when upgrades occur.

Plugin and theme updates can often be for security reasons and critical - you don't want users to be able to go back to a vulnerable version, even if that means losing changes (that they should be aware won't stick around)

Every time I've wanted to change a plugin, i've made a copy and appended something to the name to stop it auto-updating - thus taking on responsibility for maintaining it. I deploy sites to production with disable_file_mods to prevent the editors working.

People should be backing things up, either manually, through their host or through a service like vaultpress, as is recommended anyway!

#14 in reply to: ↑ 12 ; follow-up: @lgladdy
10 years ago

Replying to aercolino:

Would someone please tell me how to go for the second workaround I mentioned above?

Workaround 2 -- worse
Make the update now link unavailable altogether.

At the moment it seems the only viable alternative. Either that or refrain from sharing the update.

It's a pity there is little interest in fixing this now, and I'm surprised that it wasn't fixed before.

disallow_file_mods will prevent any filesystem modification on the front-end.

http://codex.wordpress.org/Editing_wp-config.php#Disable_Plugin_and_Theme_Update_and_Installation

#15 @aercolino
10 years ago

@lgladdy,

updates can often be for security reasons and critical - you don't want users to be able to go back

These are automatic backups before deleting plugin and theme directories and copying there the new versions. They are not only automatic but also transparent to the user. It means it's up to plugin and theme programmers to decide if the automatic backup has to survive the update action or not. Having the automatic backup, plugin and theme programmers can remove them ASAP, if they think that's better for whatever reason. NOT having the automatic backup... nothing is possible.

This is an internal feature that allows plugin and theme programmers to offer a smoother user experience.

#16 in reply to: ↑ 14 @aercolino
10 years ago

Replying to lgladdy:

disallow_file_mods will prevent any filesystem modification on the front-end.
http://codex.wordpress.org/Editing_wp-config.php#Disable_Plugin_and_Theme_Update_and_Installation

Thanks, I didn't know those settings. Unfortunately they are unrelated to the question I asked, I think... :-(

#17 @aercolino
10 years ago

See also #29820: Smooth installation and updating of plugins and themes.

Last edited 10 years ago by aercolino (previous) (diff)

#18 @aercolino
10 years ago

So two weeks went on... Meanwhile, does someone know the answer to this question?

  • Why the automatic updates feature never automatically did a backup before automatically doing a delete?

I mean, it looks like I'm talking about something really difficult or impossible in WordPress for some hidden reason...
Or the reason is not hidden at all and I'm just... blind?

#19 @dd32
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Two years on from being reported, and 6-7 years since plugin/theme updates have been a standard part of WordPress life, automatic backup of old versions of plugins and themes still hasn't had any traction.

I'm marking this as maybelater as if a committer believes in it, we can and should do something, but in the meantime this is best left to backup plugins instead.

Note: See TracTickets for help on using tickets.