From b1dade3c039391eee1242b06f20e9ae89f7c2483 Mon Sep 17 00:00:00 2001 From: Andrew Ozz Date: Fri, 18 Oct 2024 23:35:02 +0000 Subject: [PATCH] Upgrade/Install: Return WP_Error when source files cannot be found. Fixes a fatal error in `array_keys()` (PHP 8.0+) as `$wp_filesystem->dirlist()` will return `false` when the source directory doesn't exist or becomes unreadable for some reason. Props: verygoode, lifelightweb, da5f656f, costdev, afragen, azaozz Fixes #61114 git-svn-id: https://develop.svn.wordpress.org/trunk@59257 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/includes/class-wp-upgrader.php | 17 +++- tests/phpunit/tests/admin/wpUpgrader.php | 101 ++++++++++++++++++++ 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/src/wp-admin/includes/class-wp-upgrader.php b/src/wp-admin/includes/class-wp-upgrader.php index 67996aef3f..a9784682eb 100644 --- a/src/wp-admin/includes/class-wp-upgrader.php +++ b/src/wp-admin/includes/class-wp-upgrader.php @@ -204,6 +204,7 @@ class WP_Upgrader { $this->strings['mkdir_failed'] = __( 'Could not create directory.' ); $this->strings['incompatible_archive'] = __( 'The package could not be installed.' ); $this->strings['files_not_writable'] = __( 'The update cannot be installed because some files could not be copied. This is usually due to inconsistent file permissions.' ); + $this->strings['dir_not_readable'] = __( 'A directory could not be read.' ); $this->strings['maintenance_start'] = __( 'Enabling Maintenance mode…' ); $this->strings['maintenance_end'] = __( 'Disabling Maintenance mode…' ); @@ -558,7 +559,13 @@ class WP_Upgrader { $remote_source = $args['source']; $local_destination = $destination; - $source_files = array_keys( $wp_filesystem->dirlist( $remote_source ) ); + $dirlist = $wp_filesystem->dirlist( $remote_source ); + + if ( false === $dirlist ) { + return new WP_Error( 'source_read_failed', $this->strings['fs_error'], $this->strings['dir_not_readable'] ); + } + + $source_files = array_keys( $dirlist ); $remote_destination = $wp_filesystem->find_folder( $local_destination ); // Locate which directory to copy to the new folder. This is based on the actual folder holding the files. @@ -605,7 +612,13 @@ class WP_Upgrader { // Has the source location changed? If so, we need a new source_files list. if ( $source !== $remote_source ) { - $source_files = array_keys( $wp_filesystem->dirlist( $source ) ); + $dirlist = $wp_filesystem->dirlist( $source ); + + if ( false === $dirlist ) { + return new WP_Error( 'new_source_read_failed', $this->strings['fs_error'], $this->strings['dir_not_readable'] ); + } + + $source_files = array_keys( $dirlist ); } /* diff --git a/tests/phpunit/tests/admin/wpUpgrader.php b/tests/phpunit/tests/admin/wpUpgrader.php index 3d38631bfa..3e4da4082b 100644 --- a/tests/phpunit/tests/admin/wpUpgrader.php +++ b/tests/phpunit/tests/admin/wpUpgrader.php @@ -948,6 +948,107 @@ class Tests_Admin_WpUpgrader extends WP_UnitTestCase { ); } + /** + * Tests that `WP_Upgrader::install_package()` returns a WP_Error object + * when the source directory's file list cannot be retrieved. + * + * @ticket 61114 + * + * @covers WP_Upgrader::install_package + */ + public function test_install_package_should_return_wp_error_when_source_directory_file_list_cannot_be_retrieved() { + self::$instance->generic_strings(); + + self::$upgrader_skin_mock + ->expects( $this->once() ) + ->method( 'feedback' ) + ->with( 'installing_package' ); + + self::$wp_filesystem_mock + ->expects( $this->once() ) + ->method( 'dirlist' ) + ->willReturn( false ); + + $args = array( + 'source' => '/', + 'destination' => '/', + ); + + $actual = self::$instance->install_package( $args ); + + $this->assertWPError( + $actual, + 'WP_Upgrader::install_package() did not return a WP_Error object' + ); + + $this->assertSame( + 'source_read_failed', + $actual->get_error_code(), + 'Unexpected WP_Error code' + ); + } + + /** + * Tests that `WP_Upgrader::install_package()` returns a WP_Error object + * when the source directory is filtered and its file list cannot be retrieved. + * + * @ticket 61114 + * + * @covers WP_Upgrader::install_package + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_install_package_should_return_wp_error_when_a_filtered_source_directory_file_list_cannot_be_retrieved() { + define( 'FS_CHMOD_DIR', 0755 ); + + self::$instance->generic_strings(); + + self::$upgrader_skin_mock + ->expects( $this->once() ) + ->method( 'feedback' ) + ->with( 'installing_package' ); + + $first_source = array( + 'subdir' => array( + 'name' => 'subdir', + 'type' => 'd', + 'files' => array( 'subfile.php' ), + ), + ); + + self::$wp_filesystem_mock + ->expects( $this->exactly( 2 ) ) + ->method( 'dirlist' ) + ->willReturn( $first_source, false ); + + $args = array( + 'source' => '/', + 'destination' => '/', + ); + + // Filter the source to something else. + add_filter( + 'upgrader_source_selection', + static function () { + return '/not_original_source/'; + } + ); + + $actual = self::$instance->install_package( $args ); + + $this->assertWPError( + $actual, + 'WP_Upgrader::install_package() did not return a WP_Error object' + ); + + $this->assertSame( + 'new_source_read_failed', + $actual->get_error_code(), + 'Unexpected WP_Error code' + ); + } + /** * Tests that `WP_Upgrader::install_package()` adds a trailing slash to * the source directory of a single file.