Make WordPress Core

Opened 9 months ago

Closed 7 weeks ago

Last modified 7 weeks ago

#57469 closed defect (bug) (invalid)

retrieve_widgets(): fatal error when a sidebar's widgets set to null (array is expected)

Reported by: kesselb's profile kesselb Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9
Component: Widgets Keywords: php80 close
Focuses: Cc:

Description (last modified by hellofromTonya)

When changing from PHP 7.4 to PHP 8.0 it's not possible to activate our theme.

Fatal error: Uncaught Error: array_merge(): Argument #3 must be of type array, null given in wp-includes/widgets.php on line 1342
array_merge()
wp-includes/widgets.php:1342

retrieve_widgets()
wp-includes/widgets.php:1287

_wp_sidebars_changed()
wp-includes/class-wp-hook.php:310

WP_Hook::apply_filters()
wp-includes/class-wp-hook.php:332

WP_Hook::do_action()
wp-includes/plugin.php:517

do_action()
wp-includes/theme.php:3395

check_theme_switched()
wp-includes/class-wp-hook.php:308

WP_Hook::apply_filters()
wp-includes/class-wp-hook.php:332

WP_Hook::do_action()
wp-includes/plugin.php:517

do_action()
wp-settings.php:617

require_once()
wp-config.php:103

require_once()
wp-load.php:50

require_once()
wp-admin/admin.php:34

require_once()
wp-admin/plugins.php:10

Line 1342:

<?php

$shown_widgets = array_merge( ...array_values( $sidebars_widgets ) );

$sidebars_widgets

array (
  'wp_inactive_widgets' => 
  array (
  ),
  'sidebar-standard' => 
  array (
  ),
  'sidebar-marken' => NULL,
  'sidebar-typen' => NULL,
  'sidebar-test' => NULL,
  'sidebar-zubehoer' => NULL,
  'sidebar-ratgeber' => NULL,
)

PHP 8.0+ throws a type error when a non-array value is given to array_merge.

I guess NULL is not a valid value for the sidebar widgets.
Maybe our theme wrote this weird state to the options field.

Patch: https://github.com/WordPress/wordpress-develop/pull/3848

Attachments (2)

Screenshot from 2023-01-15 21-45-32.png (173.2 KB) - added by kesselb 9 months ago.
Screenshot from 2023-01-15 21-46-30.png (77.4 KB) - added by kesselb 9 months ago.

Download all attachments as: .zip

Change History (10)

This ticket was mentioned in PR #3848 on WordPress/wordpress-develop by kesselb.


9 months ago
#1

  • Keywords has-patch has-unit-tests added

wp_map_sidebar_widgets is called in retrieve_widgets and is expected to return array<string, array>.

Before this change it was possible to return something like ['sidebar-1' => [], 'sidebar-2' => null].

array_merge in PHP 8.0 is stricter and throws an TypeError when null given.

PHP 7.4: array_merge(...[['hello', 'world'], null]) => null
PHP 8.0: array_merge(...[['hello', 'world'], null]) => Fatal error

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

#2 @audrasjb
9 months ago

  • Keywords php8 added
  • Milestone changed from Awaiting Review to 6.2
  • Version 6.1.1 deleted

Thanks for the ticket, the patch and the unit tests!

I left a comment in the PR.
Moving for 6.2 consideration.

#3 follow-up: @jrf
8 months ago

  • Keywords close added
  • Milestone 6.2 deleted

@kesselb Thanks for the ticket.

To be honest, this sounds like user error and something which should be fixed in the theme, not in WP Core.

If anything, a _doing_it_wrong should be thrown when receiving invalid data (as is the case with your use-case).

#4 @kesselb
8 months ago

Hi,

If anything, a _doing_it_wrong should be thrown when receiving invalid data (as is the case with your use-case).

Sounds good ;)

<?php
function check_sidebars_widgets_for_invalid_values($value)
{
        if (is_array($value)) {
                $value = array_filter($value, static function ($var) {
                        return $var !== null;
                });
        }
        return $value;
}

/**
 * Upgrade to PHP 8 fails on some pages with an uncaught error:
 *
 * array_merge(): Argument #3 must be of type array, null given in wp-includes/widgets.php on line 1342
 *
 * Starting with PHP 8 array_merge is stricter about null values.
 * This filter will remove sidebars with null as value.
 */
add_filter('option_sidebars_widgets', 'check_sidebars_widgets_for_invalid_values', 1, 1);

If anyone has a similar issue. The above filter prevent the fatal error.

#5 @hellofromTonya
7 weeks ago

  • Description modified (diff)
  • Keywords php80 added; php8 has-patch has-unit-tests removed
  • Summary changed from Uncaught Error: array_merge(): Argument #3 must be of type array, null given in wp-includes/widgets.php on line 1342 to retrieve_widgets(): fatal error when a sidebar's widgets set to null (array is expected)
  • Version set to 4.9

Changes in the ticket:

  • Updated the Summary to better reflect the issue being reported, i.e. for discoverability.
  • Updated the Version to 4.9.
  • Wrapped the PHP error in the Description.
  • Reset the keywords as the PR was closed.
  • Changed keyword to php80 to identify which PHP version increased the error's severity.

Error across PHP versions:
If null or a non-array is set for the widgets of a sidebar, then PHP throws an error (see it in action https://3v4l.org/i45nZ):

  • < 7.3: Warning: array_merge(): Argument #3 is not an array
  • PHP 7.3 to 7.4: Warning: array_merge(): Expected parameter 3 to be an array, null given
  • PHP 8+: Fatal error: Uncaught TypeError: array_merge(): Argument #3 must be of type array, null given.

Historical context:

  • [41555] / #39693 (during WP 4.9):
    • Introduced wp_map_sidebars_widgets().
    • retrieve_widgets():
      • Invoked wp_map_sidebars_widgets() to map $sidebars_widgets
      • Added $shown_widgets = call_user_func_array( 'array_merge', $sidebars_widgets );.
  • [48839] / #50913 (during WP 5.6):
    • retrieve_widgets(): changed call_user_func_array( 'array_merge', $sidebars_widgets ); to array_merge( ...array_values( $sidebars_widgets ) );.

#6 in reply to: ↑ 3 @hellofromTonya
7 weeks ago

  • Resolution set to invalid
  • Status changed from new to closed

Replying to jrf:

To be honest, this sounds like user error and something which should be fixed in the theme, not in WP Core.

I agree. The error flagging null as the wrong data type and informing it should an array has been present for a long time. This is a case of doing it wrong.

I'm all for defensive code to protect Core's source code. However, sometimes a native PHP error is appropriate.

When changing from PHP 7.4 to PHP 8.0 it's not possible to activate our theme.

I think in this case, not being able to activate a theme is not severe enough to warrant adding defensive guard and _doing_it_wrong().

A user using the theme with the issue upgrading to PHP 8+ would experience a fatal error, which would require manual action to restore the site. Changing Core's code for this scenario is dependent upon the frequency of this scenario happening. But I'm not seeing other reports to null for widgets causing a fatal error.

Thus, I tend to agree with @jrf to close this ticket as something to be fixed in the theme.

#7 @hellofromTonya
7 weeks ago

#58351 was marked as a duplicate.

#8 @hellofromTonya
7 weeks ago

#58351 reported the same issue but happened after switching themes:

In some subsites, that after switched themes I get the following PHP Fatal error.

@lenasterg As noted in this ticket, the root cause seems to be a problem in the theme. Can you confirm if switching back to the original theme or to a different theme resolved the issue?

Note: See TracTickets for help on using tickets.