WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 12 months ago

Last modified 12 months ago

#47704 closed task (blessed) (fixed)

PHP 7.4: Fix issues regarding accessing null/bool/etc as if they were an array

Reported by: jrf Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.3
Component: General Keywords: has-patch has-unit-tests php74 has-dev-note
Focuses: Cc:

Description

The change in PHP 7.4

PHP 7.4 introduces a "Trying to access array offset on value of type ..." warning for accessing null/bool/int/float/resource (everything but array, string and object) as if it were an array.

Previously PHP already threw a warning when you would attempt to use an offset that is of an invalid type. Now PHP will also throw a warning when a container is of an invalid type.

The undocumented, default behavior, of a silent NULL return, is still maintained.

Refs:

The problem

While code which relies on this undocumented behaviour of PHP returning null when attempting to array access a null/bool/int/float/resource, will continue to work as expected, each and every instance of this will now throw a PHP E_WARNING.

This is already causing the unit tests to fail when run against PHP 7.4 as can be seen in the latest Travis build against master: https://travis-ci.com/WordPress/wordpress-develop/jobs/215901645

The solution

When using array access on the outcome of functions with mixed return values, i.e. array|false and the likes, more type checking needs to be done before attempting to access the return value as if it were an array to prevent this warning.

Patches

For those unit tests currently failing because of this issue, I will be attaching patches.

This does not imply that all instances of this issue will be solved by merging these patches as not all code is 100% unit tested, so there may well be more problem code which has not surfaced yet.

I expect more issues to be reported - both in Core as well as in plugins and themes - once PHP 7.4 is out and starting to be used for real.

Finding additional issues

At this time, AFAIK there are no (static) analysis tools available to find these issues.

I can already tell you now that tools such as PHPCompatibility will not be able to detect these issues.

Attachments (10)

47704-WP_REST_Request-get_content.patch (1.4 KB) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix WP_REST_Request::get_content_type() - fixes 810 errors, 2 failures and 32 unnecessarily skipped tests
47704-add_theme_support.patch (2.4 KB) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix add_theme_support() - fixes 15 errors
47704-WP_Customize_Nav_Menu_Item_.patch (1.7 KB) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix WP_Customize_Nav_Menu_Item_Setting::filter_wp_get_nav_menu_items() - fixes 1 error
47704-wp_update_user.patch (1.1 KB) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix wp_update_user() - fixes 2 errors
47704-get_metadata.patch (894 bytes) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix get_metadata() - fixes 1 error
47704-wpdb-strip_invalid_text.patch (960 bytes) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix wpdb::strip_invalid_text() - fixes 3 errors
47704-WP_Customize_Manager-regist.patch (1.6 KB) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix WP_Customize_Manager::register_controls() - fixes 12 errors
47704-Tests_Query_GeneratePostdat.patch (908 bytes) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix Tests_Query_GeneratePostdata::test_setup_by_fake_post() - fixes 1 faulty unit test
47704-Tests_TermExists-test_term_.patch (927 bytes) - added by jrf 15 months ago.
PHP 7.4/array-access: Fix Tests_TermExists::test_term_exists_taxonomy_nonempty_parent_0_should_return_false_for_child_term() - fixes 1 faulty unit test
47704-maintenance-nag.diff (602 bytes) - added by desrosj 12 months ago.

Download all attachments as: .zip

Change History (25)

@jrf
15 months ago

PHP 7.4/array-access: Fix WP_REST_Request::get_content_type() - fixes 810 errors, 2 failures and 32 unnecessarily skipped tests

@jrf
15 months ago

PHP 7.4/array-access: Fix add_theme_support() - fixes 15 errors

@jrf
15 months ago

PHP 7.4/array-access: Fix WP_Customize_Nav_Menu_Item_Setting::filter_wp_get_nav_menu_items() - fixes 1 error

@jrf
15 months ago

PHP 7.4/array-access: Fix wp_update_user() - fixes 2 errors

@jrf
15 months ago

PHP 7.4/array-access: Fix get_metadata() - fixes 1 error

@jrf
15 months ago

PHP 7.4/array-access: Fix wpdb::strip_invalid_text() - fixes 3 errors

@jrf
15 months ago

PHP 7.4/array-access: Fix WP_Customize_Manager::register_controls() - fixes 12 errors

@jrf
15 months ago

PHP 7.4/array-access: Fix Tests_Query_GeneratePostdata::test_setup_by_fake_post() - fixes 1 faulty unit test

@jrf
15 months ago

PHP 7.4/array-access: Fix Tests_TermExists::test_term_exists_taxonomy_nonempty_parent_0_should_return_false_for_child_term() - fixes 1 faulty unit test

#1 @jrf
15 months ago

A build on a branch including all attached patches can be found here: https://travis-ci.com/WordPress/wordpress-develop/jobs/215960581

The remaining three failing unit tests are caused by the same issue existing in the WordPress Importer plugin. Once the issue in the plugin is fixed and a new version of the plugin tagged, those three remaining unit tests should start passing again.

#2 @jrf
15 months ago

  • Keywords has-unit-tests added

#3 @jrf
15 months ago

PR to fix the issue in the WordPress Importer plugin: https://github.com/WordPress/wordpress-importer/pull/53

#4 @pento
15 months ago

In 45639:

Code Modernisation: Fix known instances of array access on data types that can't be accessed as arrays.

PHP 7.4 addes a warning when trying access a null/bool/int/float/resource (everything but array, string and object) as if it were an array.

This change fixes all of these warnings visible in unit tests.

Props jrf.
See #47704.

#5 @pento
15 months ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to 5.3
  • Type changed from defect (bug) to task (blessed)

[45639] includes all of the patches to date. I'm going to leave this ticket open as a task for the 5.3 cycle, in case we run into more that need fixing.

#6 @jrf
14 months ago

Thanks @pento! and yes, leaving the ticket open was the idea ;-)

#7 @jrf
14 months ago

  • Keywords php74 added

#8 @jorbin
12 months ago

Seeing:

Trying to access array offset on value of type bool in /wp-admin/includes/update.php on line 710

#9 @desrosj
12 months ago

Ha, beat me to it @jorbin! Was just testing and also found this one.

I also did some testing with each of the default themes and did not find any occurrences in those.

#10 @jrf
12 months ago

@desrosj Pacth reviewed. LGTM.

Version 0, edited 12 months ago by jrf (next)

#11 @desrosj
12 months ago

In 46185:

PHP 7.4: Fix another instance of array access on a datatype that cannot be accessed as an array.

PHP 7.4 adds a warning when trying access a null/bool/int/float/resource (everything but array, string and object) as if it were an array.

Follow up of [45639].

Props desrosj, jrf.
See #47704.

#12 @desrosj
12 months ago

  • Keywords close added

Marking this as close because I think most of the occurrences of this have been fixed.

#13 @jrf
12 months ago

  • Keywords close removed

@desrosj This is not an issue which can be detected by static analysis, so the only issues we've fixed so far are issues which caused warnings in the unit tests.

As WP does not have 100% code coverage with the unit tests, I think we should leave this open as I expect more issues will be discovered once more people start testing WP on PHP 7.4 for real.

#14 @ocean90
12 months ago

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

Closing as fixed for now. Please feel free to reopen this ticket or create new tickets for new discovered issues.

Note: See TracTickets for help on using tickets.