WordPress.org

Make WordPress Core

Opened 5 weeks ago

Last modified 4 weeks ago

#53002 reviewing defect (bug)

Plugin_Upgrader's bulk_upgrade returns incorrect results when an error is encountered

Reported by: pwtyler Owned by: SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.7
Component: Upgrade/Install Keywords: has-patch
Focuses: Cc:

Description

I initially thought this was a WP-CLI issue and dug in from there, but we're seeing that the bad results are coming from bulk_upgrade() in wp-admin/includes/class-plugin-upgrader.php.

When multiple plugins are updated and one succeeds and another fails, both end up treated as a success (Updated).

The logs printed show the expected output (error updating the second plugin) but the final output shows success for both updates.

Steps to reproduce (with WP-CLI)

$ wp plugin install akismet --version=4.0 --force
$ wp plugin install wordpress-importer --version=0.5 --force
$ chmod -w wp-content/plugins/wordpress-importer
$ wp plugin update --all
# > See "Success: Updated 2 of 2 plugins."
$ wp plugin list 
# > See "wordpress-importer" is still at version 0.5

Digging deeper, I can see the array of results created at line 350 of wp-admin/includes/class-plugin-upgrader.php is not reporting the correct information for the second of the two updates (the failed wordpress-importer update)— instead of a WP_Error Object, we're seeing a result for Akismet, updated previously.

--
Thanks to @nullvariable for the assistance tracking this down.

Change History (5)

This ticket was mentioned in Slack in #cli by pwtyler. View the logs.


5 weeks ago

This ticket was mentioned in PR #1188 on WordPress/wordpress-develop by pwtyler.


5 weeks ago

  • Keywords has-patch added

When bulk updating plugins or themes, the result of a previous update remained through the loop, and if a subsequent update failed (which should have then been null result), the failed update would end up with the result of the previous operation.

This PR unsets the result at the top of the bulk upgrade loop if it is not already null to prevent the results of the first plugin or theme's success from cascading through a subsequent failure and resulting in a false positive on an update result.

Trac ticket: https://core.trac.wordpress.org/ticket/53002

This ticket was mentioned in Slack in #core by pwtyler. View the logs.


4 weeks ago

#4 @SergeyBiryukov
4 weeks ago

  • Milestone changed from Awaiting Review to 5.8

#5 @SergeyBiryukov
4 weeks ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.