Modify product import file check to use the WP filesystem API (#51540)

* Modify product import file check to use the WP filesystem API.

Otherwise it doesn't work on environments that don't have
a direct filesystem like e.g. WordPress VIP.

* Add the FilesystemUtil::get_wp_filesystem_method_or_direct method
This commit is contained in:
Néstor Soriano 2024-10-04 08:50:54 +02:00 committed by GitHub
parent 7c48112d4a
commit ddfa425062
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 125 additions and 27 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: fix
Modify product import file check to use the WP filesystem API

View File

@ -5,6 +5,8 @@
* @package WooCommerce\Admin\Importers
*/
use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
use Automattic\WooCommerce\Internal\Utilities\URL;
use Automattic\WooCommerce\Utilities\I18nUtil;
if ( ! defined( 'ABSPATH' ) ) {
@ -113,37 +115,18 @@ class WC_Product_CSV_Importer_Controller {
* @throws \Exception When file validation fails.
*/
protected static function check_file_path( string $path ): void {
$is_valid_file = false;
$wp_filesystem = FilesystemUtil::get_wp_filesystem();
if ( ! empty( $path ) ) {
$path = realpath( $path );
$is_valid_file = false !== $path;
}
// File must be readable.
$is_valid_file = $is_valid_file && is_readable( $path );
// File must exist and be readable.
$is_valid_file = $wp_filesystem->is_readable( $path );
// Check that file is within an allowed location.
if ( $is_valid_file ) {
$normalized_path = wp_normalize_path( $path );
$in_valid_location = false;
$valid_locations = array();
$valid_locations[] = ABSPATH;
$upload_dir = wp_get_upload_dir();
if ( false === $upload_dir['error'] ) {
$valid_locations[] = $upload_dir['basedir'];
$is_valid_file = self::file_is_in_directory( $path, $wp_filesystem->abspath() );
if ( ! $is_valid_file ) {
$upload_dir = wp_get_upload_dir();
$is_valid_file = false === $upload_dir['error'] && self::file_is_in_directory( $path, $upload_dir['basedir'] );
}
foreach ( $valid_locations as $valid_location ) {
$normalized_location = wp_normalize_path( realpath( $valid_location ) );
if ( 0 === stripos( $normalized_path, trailingslashit( $normalized_location ) ) ) {
$in_valid_location = true;
break;
}
}
$is_valid_file = $in_valid_location;
}
if ( ! $is_valid_file ) {
@ -155,6 +138,19 @@ class WC_Product_CSV_Importer_Controller {
}
}
/**
* Check if a given file is inside a given directory.
*
* @param string $file_path The full path of the file to check.
* @param string $directory The path of the directory to check.
* @return bool True if the file is inside the directory.
*/
private static function file_is_in_directory( string $file_path, string $directory ): bool {
$file_path = (string) new URL( $file_path ); // This resolves '/../' sequences.
$file_path = preg_replace( '/^file:\\/\\//', '', $file_path );
return 0 === stripos( wp_normalize_path( $file_path ), trailingslashit( wp_normalize_path( $directory ) ) );
}
/**
* Get all the valid filetypes for a CSV file.
*

View File

@ -4,6 +4,7 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\Internal\Utilities;
use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Proxies\LegacyProxy;
use Exception;
use WP_Filesystem_Base;
@ -31,6 +32,35 @@ class FilesystemUtil {
return $wp_filesystem;
}
/**
* Get the WP filesystem method, with a fallback to 'direct' if no FS_METHOD constant exists and there are not FTP related options/credentials set.
*
* @return string|false The name of the WP filesystem method to use.
*/
public static function get_wp_filesystem_method_or_direct() {
$proxy = wc_get_container()->get( LegacyProxy::class );
if ( ! self::constant_exists( 'FS_METHOD' ) && false === $proxy->call_function( 'get_option', 'ftp_credentials' ) && ! self::constant_exists( 'FTP_HOST' ) ) {
return 'direct';
}
$method = $proxy->call_function( 'get_filesystem_method' );
if ( $method ) {
return $method;
}
return 'direct';
}
/**
* Check if a constant exists and is not null.
*
* @param string $name Constant name.
* @return bool True if the constant exists and its value is not null.
*/
private static function constant_exists( string $name ): bool {
return Constants::is_defined( $name ) && ! is_null( Constants::get_constant( $name ) );
}
/**
* Recursively creates a directory (if it doesn't exist) and adds an empty index.html and a .htaccess to prevent
* directory listing.
@ -75,7 +105,7 @@ class FilesystemUtil {
require_once ABSPATH . 'wp-admin/includes/file.php';
$method = get_filesystem_method();
$method = self::get_wp_filesystem_method_or_direct();
$initialized = false;
if ( 'direct' === $method ) {

View File

@ -3,6 +3,7 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\Tests\Internal\Utilities;
use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
use WC_Unit_Test_Case;
use WP_Filesystem_Base;
@ -29,6 +30,8 @@ class FilesystemUtilTest extends WC_Unit_Test_Case {
*/
public function tearDown(): void {
unset( $GLOBALS['wp_filesystem'] );
$this->reset_legacy_proxy_mocks();
Constants::clear_constants();
parent::tearDown();
}
@ -58,4 +61,69 @@ class FilesystemUtilTest extends WC_Unit_Test_Case {
remove_filter( 'filesystem_method', $callback );
}
/**
* @testdox 'get_wp_filesystem_method_or_direct' returns 'direct' if no FS_METHOD constant, not 'ftp_credentials' option and not FTP_HOST constant exist.
*/
public function test_get_wp_filesystem_method_with_no_fs_method_nor_ftp_constant() {
Constants::set_constant( 'FS_METHOD', null );
$this->register_legacy_proxy_function_mocks(
array(
'get_option' => fn( $name, $default_value = false ) => 'ftp_credentials' === $name ? false : get_option( $name, $default_value ),
'get_filesystem_method' => function () {
throw new \Exception( 'Unexpected call to get_filesystem_method' ); },
)
);
Constants::set_constant( 'FTP_HOST', null );
$this->assertEquals( 'direct', FilesystemUtil::get_wp_filesystem_method_or_direct() );
}
/**
* @testdox 'get_wp_filesystem_method_or_direct' invokes 'get_filesystem_method' if the FS_METHOD constant, the 'ftp_credentials' option or the FTP_HOST constant exist.
*
* @testWith ["method", false, null]
* [null, "credentials", null]
* [null, false, "host"]
*
* @param string|null $fs_method_constant_value The value of the FS_METHOD constant to test.
* @param string|false $ftp_credentials_option_value The value of the 'ftp_credentials' option to test.
* @param string|false $ftp_host_option_value The value of the FTP_HOST constant to test.
*/
public function test_get_wp_filesystem_method_with_fs_method_or_ftp_constant( $fs_method_constant_value, $ftp_credentials_option_value, $ftp_host_option_value ) {
Constants::set_constant( 'FS_METHOD', $fs_method_constant_value );
$this->register_legacy_proxy_function_mocks(
array(
'get_option' => fn( $name, $default_value = false ) => 'ftp_credentials' === $name ? $ftp_credentials_option_value : get_option( $name, $default_value ),
'get_filesystem_method' => fn() => 'method',
)
);
Constants::set_constant( 'FTP_HOST', $ftp_host_option_value );
$this->assertEquals( 'method', FilesystemUtil::get_wp_filesystem_method_or_direct() );
}
/**
* 'get_wp_filesystem_method_or_direct' returns 'direct' if the FS_METHOD constant, the 'ftp_credentials' option or the FTP_HOST constant exist, and 'get_filesystem_method' fails.
*
* @testWith ["method", false, null]
* [null, "credentials", null]
* [null, false, "host"]
*
* @param string|null $fs_method_constant_value The value of the FS_METHOD constant to test.
* @param string|false $ftp_credentials_option_value The value of the 'ftp_credentials' option to test.
* @param string|false $ftp_host_option_value The value of the FTP_HOST constant to test.
*/
public function test_get_wp_filesystem_method_with_fs_method_or_ftp_constant_and_no_wp_filesystem( $fs_method_constant_value, $ftp_credentials_option_value, $ftp_host_option_value ) {
Constants::set_constant( 'FS_METHOD', $fs_method_constant_value );
$this->register_legacy_proxy_function_mocks(
array(
'get_option' => fn( $name, $default_value = false ) => 'ftp_credentials' === $name ? $ftp_credentials_option_value : get_option( $name, $default_value ),
'get_filesystem_method' => fn() => false,
)
);
Constants::set_constant( 'FTP_HOST', $ftp_host_option_value );
$this->assertEquals( 'direct', FilesystemUtil::get_wp_filesystem_method_or_direct() );
}
}