Upgrade/Install: Return an WP_Error when files cannot be detected.

[costdev]

Previously, WP_Upgrader::install_package() attempted to get keys from a WP_Filesystem_*::dirlist() result, expecting it to be an array. However, WP_Filesystem_*::dirlist() may also return false. The subsequent array_keys() function call produces a Warning (PHP < 8.0) and Fatal Error (PHP 8.0+) because the first argument must be of array type.

This change checks for a false return value from WP_Filesystem_*::dirlist() and returns a WP_Error from WP_Upgrader::install_package().

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

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props costdev, afragen, azaozz.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[costdev]

@azaozz @afragen This does a specific check for false to ensure a targeted error message is displayed. By returning early, we also save on processing time if the upgrade can't continue anyway.

If/when you have bandwidth, let me know your thoughts here on the PR 🙂

[costdev]

I see that this is currently milestoned for WordPress 6.7. While I totally agree that this can have a serious impact, this doesn't leave a lot of time for testing. The added unit tests certainly help, just be mindful of our timeframe here and the need to be confident that the error handling is safe through testing.

If this PR has consensus as the way we're going forward here, then we'll need to get the Test Team on this as soon as possible. Until/unless this PR has that consensus, let's not add to their already heavy workload. 🙂

[afragen]

@costdev I think this looks good.

It seems to be a deprecation due to a PHP version and not so much from WP 6.7. What I don't understand is I don't see this issue on my sites, running 6.6.2 or beta/RC and PHP 8.3.x.

[costdev]

Not a deprecation, but rather that it went from a Warning to a Fatal Error in PHP 8.0. Ref

Autovivification of false was deprecated, but not until PHP 8.1.

[azaozz]

Imho this looks good.

it went from a Warning to a Fatal Error in PHP 8.0. Ref
...doesn't leave a lot of time for testing

WP 6.7 is scheduled for release on November 12, 2024. Thinking four weeks is ample time to test this. Also this fix doesn't change the way the code works. Passing false to array_keys() would return an empty array in earlier PHP versions, right? There are still no files, the operation fails, and an error is eventually shown.

[azaozz]

LGTM