Make WordPress Core

Opened 2 years ago

Last modified 9 months ago

#56431 new defect (bug)

Fatal Error on Update Page When a Plugin's Icon is Not Set

Reported by: scottdeluzio's profile scott.deluzio Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: needs-patch needs-testing dev-feedback reporter-feedback
Focuses: Cc:

Description

When on the Dashboard > Updates page, if there is a plugin whose icon is not set, a fatal error is generated, and no plugins are listed in the update section.

PHP Fatal error: Uncaught Error: Cannot use object of type stdClass as array in /wp-admin/update-core.php:509

I have often seen this with plugins that are not hosted on wordpress.org. Mostly this is when you have a paid plugin that doesn't have an icon set.

I think the fix should be as follows:
wp-admin/update-core.php line 509:
if ( ! empty( $plugin_data->update->icons[ $preferred_icon ] ) ) {
should change to:
if ( is_array( $plugin_data->update->icons ) && ! empty( $plugin_data->update->icons[ $preferred_icon ] ) ) {

Attachments (2)

plugin-update-error.jpg (137.9 KB) - added by scott.deluzio 2 years ago.
Update screen with error and no plugins showing.
plugin-update-error-fixed.jpg (90.2 KB) - added by scott.deluzio 2 years ago.
Update screen with error fixed and all available updates showing.

Download all attachments as: .zip

Change History (15)

@scott.deluzio
2 years ago

Update screen with error and no plugins showing.

@scott.deluzio
2 years ago

Update screen with error fixed and all available updates showing.

#1 @costdev
2 years ago

  • Component changed from Plugins to Upgrade/Install
  • Keywords needs-testing reporter-feedback added

Hi @scottdeluzio, can you confirm if this issue only appeared in WordPress 6.0.1, or if it existed in earlier versions of WordPress? If possible, can you test this on an older version of WordPress and report your findings?

#2 @scott.deluzio
2 years ago

  • Keywords reporter-feedback removed

I first noticed it when we updated our servers to PHP 8. I never noticed it before that update, so it may be an issue with how PHP 8 handles this.

Looking in the code for update-core.php the if ( ! empty( $plugin_data->update->icons[ $preferred_icon ] ) ) { code seems to exist at least as far back as 5.0 (probably further back, but I didn't look any further), which was long before we started using PHP 8. I would suspect that a similar error would have occurred had the server been using PHP 8 at the time. Then again, it could just be a coincidence that this error started showing up around the time that PHP 8 was introduced.

I, unfortunately, don't have a test environment set up at the moment to test this theory, but I'm hoping it will point to the solution.

#3 @SergeyBiryukov
2 years ago

  • Keywords php8 added

#4 @afragen
2 years ago

@scottdeluzio I think this is likely related to the updater code that this particular non-dot org plugin uses. Having written updaters for non-dot org plugins/themes, there's lots of semi-documented things.

Knowing the updater code would help find the real solution. I'm not sure this is a core bug. Just improper data type being sent to core.

#5 follow-ups: @costdev
2 years ago

  • Keywords dev-feedback added

In testing going as far back as WordPress 5.0, I can reproduce the Fatal Error on both PHP 7.4 and PHP 8.0 if I add this to wp-admin/update.php before the end of the foreach loop in get_plugin_updates():

$upgrade_plugins[ $plugin_file ]->update->icons = new stdClass();

Core expects an associative array, and the plugin mentioned in this ticket does actually seem to have code to enforce this. I guess something slipped through.

There are two potential points of failure that I can see:

  1. There are no icons set.
  2. The icons are not set to an associative array (an object, for example).

Wrapping the foreach() loop with this seems to prevent the Fatal Error in my testing:

if ( isset( $plugin_data->update->icons ) && is_array( $plugin_data->update->icons ) ) {}

However, whether this is an appropriate solution (or even something Core should mitigate) needs more feedback. Adding dev-feedback to get additional thoughts on this.

Removing the Version as this is at least back to WordPress 5.0 and the exact version is still to be confirmed.

#6 @costdev
2 years ago

  • Version 6.0.1 deleted

#7 in reply to: ↑ 5 @hellofromTonya
16 months ago

  • Keywords reporter-feedback added

Hmm, I don't how / why the following code in wp-admin/update-core.php throws a fatal error:

if ( ! empty( $plugin_data->update->icons[ $preferred_icon ] ) ) {

empty() is checking each part of the chain including that icons is an array and if it has $preferred_icon.

I set up 4 scenarios https://3v4l.org/kAVJ9 to see this in action. It worked as expected, with no fatal errors.

Hmm. What am I missing?

Hey @scottdeluzio, do you have a backtrace available? It might provide more clues.

#8 in reply to: ↑ 5 @hellofromTonya
16 months ago

  • Keywords php8 removed

Replying to costdev:

In testing going as far back as WordPress 5.0, I can reproduce the Fatal Error on both PHP 7.4 and PHP 8.0 if I add this to wp-admin/update.php before the end of the foreach loop in get_plugin_updates():

I've removed php8 keyword, as @costdev was able to show a fatal error scenario that happens on PHP version(s) older than PHP 8. It seems the reported issue is not strictly bound to only PHP 8.

#9 follow-up: @costdev
16 months ago

Hey @hellofromTonya! The issue is the icons property. If an extender messes up and uses an object, this will cause a fatal.

Normally we'd tell extenders to do it right and we'd move on. However, if I installed Plugin A 1.0.0 and version 2.0.0 mistakenly uses an object for its icons, then without that version even being installed, it can cause a fatal on WordPress sites.

I think we could both protect users from pending updates causing fatal errors, while also notifying the extenders via a _doing_it_wrong()/trigger_error() behind WP_DEBUG.

Last edited 16 months ago by costdev (previous) (diff)

#10 in reply to: ↑ 9 @hellofromTonya
16 months ago

In re-reading the description:

if there is a plugin whose icon is not set, a fatal error is generated, and no plugins are listed in the update section.

PHP Fatal error: Uncaught Error: Cannot use object of type stdClass as array in /wp-admin/update-core.php:509

How does a "plugin whose icon is not set" become an object, rather than an empty / not set value?

Replying to costdev:

Hey @hellofromTonya! The issue is the icons property. If an extender messes up and uses an object, this will cause a fatal.

  • Is it a matter of a developer setting the icons property to an object (i.e. instance of stdClass()?
  • Or is something to do with what happens when its "icon is not set" that it gets set to a stdClass().

The difference matters as the first question is the developer doing it wrong but the second question may be an issue in Core itself.

Normally we'd tell extenders to do it right and we'd move on. However, if I installed Plugin A 1.0.0 and version 2.0.0 mistakenly uses an object for its icons, then without that version even being installed, it can cause a fatal on WordPress sites.

Hmm, I see. Recovery from such a state requires help from the host or a developer. If a guard is added to avoid the fatal error and the inability to recover through the updates or plugins screen, then I think this needs thought around:

  • Should the update fail?
  • How should the user and developer be alerted to the "doing it wrong" issue? Feedback is still needed to inform of the problem.

Before implementing a change, I think an understanding of how icons property can be an instance of stdClass is needed.

#11 @costdev
16 months ago

@hellofromTonya

The difference matters as the first question is the developer doing it wrong but the second question may be an issue in Core itself.

Before implementing a change, I think an understanding of how icons property can be an instance of stdClass is needed.

I 100% agree.

  • Should the update fail?

This isn't so much about an update in progress, but rather when a plugin has an update available. AFAIK, the icons is from an API response. When update-core.php tries to output this information in the table, the fatal occurs, blocking all later output.

  • How should the user and developer be alerted to the "doing it wrong" issue? Feedback is still needed to inform of the problem.

If the icons value isn't set to stdClass by Core, we could put feedback behind WP_DEBUG, and use a default icon for the plugin that would otherwise trigger a fatal. The logic being that an icon issue in one plugin update shouldn't prevent a user from being able to see all available updates, bulk select them, and run them.

Last edited 16 months ago by costdev (previous) (diff)

#12 @afragen
16 months ago

This is apparently populated via wp_update_plugins() via a call to http://api.wordpress.org/plugins/update-check/1.1/.

These data are populated via update_plugins transient obtained from get_site_transient( 'update_plugins' ) and may be modified via filters such as set_site_transient_update_plugins and pre_set_site_transient_update_plugins.

I'm fairly certain this is not a core issue. It may be possible it's an API issue but most likely it's a developer issue.

#13 @costdev
9 months ago

#60065 was marked as a duplicate.

Note: See TracTickets for help on using tickets.