WordPress.org

Make WordPress Core

#50670 closed defect (bug) (fixed)

Upgrader implementation ignore UpgraderSkin interfaces

Reported by: schlessera Owned by:
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.5
Component: Plugins Keywords: reporter-feedback
Focuses: Cc:

Description

The new Upgrader implementations like Plugin_Upgrader don't stick to the interfaces they claim to use in terms of the WP_Upgrader_Skin and just directly access properties like $overwrite that are only declared for _some_ implementations.

This is a violation of the open/closed principle and cause code that already extends the UpgraderSkin to break. An example of this can be found here: https://travis-ci.org/github/wp-cli/wp-cli/jobs/708222192#L505-L511

The code should either first check whether the properties/methods that are not part of the interface are actually available or not, or it should only accept specific implementations, or an adapted interface.

Change History (3)

#1 @azaozz
14 months ago

  • Keywords needs-patch reporter-feedback added

Seems to happen only for themes. Caused by Theme_Upgrader missing a conditional that is in Plugin_Upgrader. See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-theme-upgrader.php#L97 and https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-plugin-upgrader.php#L89.

What would be the best fix here? Adding public $overwrite; to WP_Upgrader_Skin or the conditional, or both? :)

Last edited 14 months ago by azaozz (previous) (diff)

#2 @SergeyBiryukov
14 months ago

In 48493:

Upgrade/Install: Check if the theme installer skin's overwrite property exists in Theme_Upgrader::install_strings().

This ensures consistency with Plugin_Upgrader::install_strings() and resolves an issue caused by the property not existing in other upgrader implementations.

Props schlessera, azaozz.
See #50670.

#3 @SergeyBiryukov
14 months ago

  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from new to closed

I think this is fixed by [48493], feel free to reopen if any other changes are needed.

Note: See TracTickets for help on using tickets.