Cancelling a plugin update from an uploaded .zip file leaves the plugin .zip in the uploads directory

[psykro]

When using the new feature which allows a plugin to be updated by uploading a zip file, if the user chooses to cancel the update, the uploaded plugin file is left in the relevant uploads directory.

Over time this might cause that directory to become full. As the zip file is not listed in the media library, the user has no idea it needs to be deleted.

Ideally this file should be listed in the Media library, for manual clean up, and/or automatically deleted if the user clicks cancel.

[desrosj]

To clarify, the uploaded zip file is listed in the Media Library, but only when using the list view. It does not show when using grid view.

[psykro]

Replying to desrosj:

To clarify, the uploaded zip file is listed in the Media Library, but only when using the list view. It does not show when using grid view.

Ah, thanks @desrosj, I wasn't aware of that. In that case I will close this ticket.

[desrosj]

I think it's still worth having a discussion about. Especially where grid view is the default Media Library view, and plugins/themes can be quite large.

I also can't think of a use case where having the zip in the library would be useful.

[psykro]

Replying to desrosj:

I think it's still worth having a discussion about. Especially where grid view is the default Media Library view, and plugins/themes can be quite large.

I also can't think of a use case where having the zip in the library would be useful.

One way could be to remove it if the user clicks cancel.

However that leaves the option open for if the user clicks anything else (back button, menu item).

The only way I can think of to mitigate that would be to set a flag (option) somewhere and trigger an admin notice, warning of a failed plugin/theme update.

[joyously]

I was wondering why it is entered into the database at all, but I assume there's a technical reason for that. It seems like the file would be in the upgrade folder, not the uploads folder.

[azaozz]

if the user chooses to cancel the update, the uploaded plugin file is left in the relevant uploads directory.

Yes, that happens for all packages if the installing/updating was interrupted for some reason. There's a cron job to do cleanup: ​https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-file-upload-upgrader.php#L87 in two hours.

However these can be deleted immediately when the user has clicked "Cancel and go back". Patch coming up.

I assume there's a technical reason for that.

Right. The packages are handled by creating a "private attachment" (to reuse the file upload functionality). Perhaps can look into not listing these attachments in the Media Library "list view". They will disappear in couple of hours anyway.

[psykro]

Replying to azaozz:

Yes, that happens for all packages if the installing/updating was interrupted for some reason. There's a cron job to do cleanup: ​https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-file-upload-upgrader.php#L87 in two hours.
However these can be deleted immediately when the user has clicked "Cancel and go back". Patch coming up.

Forgive me for jumping the gun here then :-) I've just been following this for 3 of it's 11 years, and I was excited to test it out before the 5.5 release.

[azaozz]

Replying to psykro:

Forgive me for jumping the gun here

Ah, no problems, thanks for reporting this! :)

There will probably be more left-over files now, as users would cancel updates or navigate away from the "update from uploaded zip" screen.

There is another possible edge case: when a user leaves the update from uploaded zip screen open for more than two hours, the file will be "cleaned up". Can perhaps add some js to block the screen when left unused for a while, and ask the user to upload the file again.

[azaozz]

In 50612.diff:

• Fix the "Cancel and go back" button to do a cleanup (delete the attachment and file).
• In the Media Library list view, hide attachments for recently uploaded install/update packages when scheduled to be deleted in the next two hours.
• Hide the "Replace current with uploaded" button in 1 hour and 59 minutes. The attachment/file will be deleted in 2 hours.

Think this fixes the edge cases, please test :)

[noisysocks]

I tested 50612.diff​ locally by applying the patch, changing the timers to be 5 minutes instead of 2 hours, and then running through the upgrade flow. I was able to confirm that the attachment is eventually removed, that it doesn't show in the media page, and that the Replace current with uploaded button is eventually disabled.

Code looks good. My only minor comment is: Why is the check for window.wp.a11y necessary when we know we're including wp-a11y on update.php?

I'd say this is good to go! 👍

[whyisjake]

Looks good in my testing too.

[whyisjake]

In 48417:

Upgrade/Install: Ensure cleanup after canceled update.

Ensure that the uploaded zip is hidden from the media library, where a task will remove failed installs after two hours.

Fixes #50612.

Props psykro, desrosj, joyously, azaozz, noisysocks, whyisjake.

[joyously]

There was a comment in Slack: ​https://wordpress.slack.com/archives/C02RQBWTW/p1594281462266200

Today I worked all night and took some time to test WP 5.5 plugin and themes update feature using zip files. It works with plugins, but I couldn't update any themes using the same method, has anyone tested/reported this yet?

[SergeyBiryukov]

In 48427:

Docs: Improve description for the JS function that hides the update button for expired plugin or theme uploads.

Add missing @since tag.

Follow-up to [48417].

See #50612.

[psykro]

Replying to joyously:

There was a comment in Slack: ​https://wordpress.slack.com/archives/C02RQBWTW/p1594281462266200

Today I worked all night and took some time to test WP 5.5 plugin and themes update feature using zip files. It works with plugins, but I couldn't update any themes using the same method, has anyone tested/reported this yet?

I can confirm that I have tested this, rolled back Twenty Twenty, upgraded successfully. Also tested clean up of file if user elects to Cancel, as well as cron clean up of file, of user navigates away

[azaozz]

Replying to psykro:

Thanks for testing and confirming.