Commit 24ce9113c12 for woocommerce
commit 24ce9113c12c0e77ffa329ba4871bc4d9655ace0
Author: Brandon Kraft <public@brandonkraft.com>
Date: Tue Jun 30 11:52:12 2026 -0500
Use direct filesystem for uploads-directory operations (logs, imports, transient files) (#64666)
Route uploads-directory file operations (logger, CSV importer, product feed, transient files, and the wc_update_870 migration) through a new FilesystemUtil::get_wp_filesystem_direct() that always returns a WP_Filesystem_Direct instance, so they no longer fail when FS_METHOD is set to an FTP-based method without complete credentials. FS_METHOD-respecting paths (plugin/theme installer, translation packs, MaxMind GeoIP) are unchanged.
Fixes #64654
diff --git a/plugins/woocommerce/changelog/wooplug-6656-uploads-direct-filesystem b/plugins/woocommerce/changelog/wooplug-6656-uploads-direct-filesystem
new file mode 100644
index 00000000000..5453ebce4de
--- /dev/null
+++ b/plugins/woocommerce/changelog/wooplug-6656-uploads-direct-filesystem
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Use a direct filesystem instance for log, import, product feed, and transient file operations inside the uploads directory, so they no longer fail on sites that have FS_METHOD set to an FTP-based method without complete credentials.
diff --git a/plugins/woocommerce/includes/wc-update-functions.php b/plugins/woocommerce/includes/wc-update-functions.php
index 0b8f37560c1..1971606d70e 100644
--- a/plugins/woocommerce/includes/wc-update-functions.php
+++ b/plugins/woocommerce/includes/wc-update-functions.php
@@ -37,6 +37,7 @@ use Automattic\WooCommerce\Internal\ProductAttributesLookup\LookupDataStore;
use Automattic\WooCommerce\Internal\ProductDownloads\ApprovedDirectories\Register as Download_Directories;
use Automattic\WooCommerce\Internal\ProductDownloads\ApprovedDirectories\Synchronize as Download_Directories_Sync;
use Automattic\WooCommerce\Internal\Utilities\DatabaseUtil;
+use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
use Automattic\WooCommerce\Utilities\StringUtil;
use Automattic\WooCommerce\Blocks\Options as BlockOptions;
use Automattic\WooCommerce\Blocks\Utils\BlockTemplateUtils;
@@ -2832,17 +2833,21 @@ function wc_update_860_remove_recommended_marketing_plugins_transient() {
* @return void
*/
function wc_update_870_prevent_listing_of_transient_files_directory() {
- global $wp_filesystem;
-
$default_transient_files_dir = untrailingslashit( wp_upload_dir()['basedir'] ) . '/woocommerce_transient_files';
if ( ! is_dir( $default_transient_files_dir ) ) {
return;
}
- require_once ABSPATH . 'wp-admin/includes/file.php';
- \WP_Filesystem();
- $wp_filesystem->put_contents( $default_transient_files_dir . '/.htaccess', 'deny from all' );
- $wp_filesystem->put_contents( $default_transient_files_dir . '/index.html', '' );
+ // Use a direct filesystem: the transient files directory is inside wp-content/uploads and is
+ // web-server writable, so honoring FS_METHOD is unnecessary and breaks on FTP-without-credentials setups.
+ try {
+ $wp_filesystem = FilesystemUtil::get_wp_filesystem_direct();
+ $wp_filesystem->put_contents( $default_transient_files_dir . '/.htaccess', 'deny from all' );
+ $wp_filesystem->put_contents( $default_transient_files_dir . '/index.html', '' );
+ } catch ( \Exception $exception ) {
+ // Best-effort: the directory remains usable without the no-listing files, but log so the failure leaves a trace.
+ error_log( 'WooCommerce: wc_update_870 could not write transient files directory protection files: ' . $exception->getMessage() ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
+ }
}
/**
diff --git a/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/File.php b/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/File.php
index ff0c0415223..6993e610872 100644
--- a/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/File.php
+++ b/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/File.php
@@ -234,7 +234,7 @@ class File {
*/
public function is_readable(): bool {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$is_readable = $filesystem->is_file( $this->path ) && $filesystem->is_readable( $this->path );
} catch ( Exception $exception ) {
return false;
@@ -250,7 +250,7 @@ class File {
*/
public function is_writable(): bool {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$is_writable = $filesystem->is_file( $this->path ) && $filesystem->is_writable( $this->path );
} catch ( Exception $exception ) {
return false;
@@ -375,7 +375,7 @@ class File {
*/
public function get_modified_timestamp() {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$timestamp = $filesystem->mtime( $this->path );
} catch ( Exception $exception ) {
return false;
@@ -391,7 +391,7 @@ class File {
*/
public function get_file_size() {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
if ( ! $filesystem->is_readable( $this->path ) ) {
return false;
@@ -412,7 +412,7 @@ class File {
*/
protected function create(): bool {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$created = $filesystem->touch( $this->path );
$modded = $filesystem->chmod( $this->path );
} catch ( Exception $exception ) {
@@ -501,7 +501,7 @@ class File {
$new_path = str_replace( $old_filename, $new_filename, $this->path );
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$moved = $filesystem->move( $this->path, $new_path, true );
} catch ( Exception $exception ) {
return false;
@@ -524,7 +524,7 @@ class File {
*/
public function delete(): bool {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$deleted = $filesystem->delete( $this->path, false, 'f' );
} catch ( Exception $exception ) {
return false;
diff --git a/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileExporter.php b/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileExporter.php
index 024639fc354..ee9f19ca71a 100644
--- a/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileExporter.php
+++ b/plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileExporter.php
@@ -51,7 +51,7 @@ class FileExporter {
*/
public function emit_file() {
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$is_readable = $filesystem->is_file( $this->path ) && $filesystem->is_readable( $this->path );
} catch ( Exception $exception ) {
$is_readable = false;
diff --git a/plugins/woocommerce/src/Internal/Admin/Logging/Settings.php b/plugins/woocommerce/src/Internal/Admin/Logging/Settings.php
index 08b9e3d07e8..2faab4160a8 100644
--- a/plugins/woocommerce/src/Internal/Admin/Logging/Settings.php
+++ b/plugins/woocommerce/src/Internal/Admin/Logging/Settings.php
@@ -12,7 +12,6 @@ use Automattic\WooCommerce\Proxies\LegacyProxy;
use Exception;
use WC_Admin_Settings;
use WC_Log_Handler_DB, WC_Log_Handler_File, WC_Log_Levels;
-use WP_Filesystem_Direct;
/**
* Settings class.
@@ -81,11 +80,12 @@ class Settings {
if ( true === $result ) {
// Create infrastructure to prevent listing contents of the logs directory.
try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
+ $filesystem = FilesystemUtil::get_wp_filesystem_direct();
$filesystem->put_contents( $dir . '.htaccess', 'deny from all' );
$filesystem->put_contents( $dir . 'index.html', '' );
- } catch ( Exception $exception ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch
- // Creation failed.
+ } catch ( Exception $exception ) {
+ // Best-effort: the directory exists and stays usable without the no-listing files, but log a trace.
+ error_log( 'WooCommerce: could not write log directory protection files: ' . $exception->getMessage() ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
}
}
}
@@ -295,16 +295,10 @@ class Settings {
$directory = self::get_log_directory();
$status_info = array();
- try {
- $filesystem = FilesystemUtil::get_wp_filesystem();
- if ( $filesystem instanceof WP_Filesystem_Direct ) {
- $status_info[] = __( '✅ Ready', 'woocommerce' );
- } else {
- $status_info[] = __( '⚠️ The file system is not configured for direct writes. This could cause problems for the logger.', 'woocommerce' );
- $status_info[] = __( 'You may want to switch to the database for log storage.', 'woocommerce' );
- }
- } catch ( Exception $exception ) {
- $status_info[] = __( '⚠️ The file system connection could not be initialized.', 'woocommerce' );
+ if ( wp_is_writable( $directory ) ) {
+ $status_info[] = __( '✅ Ready', 'woocommerce' );
+ } else {
+ $status_info[] = __( '⚠️ The log directory is not writable.', 'woocommerce' );
$status_info[] = __( 'You may want to switch to the database for log storage.', 'woocommerce' );
}
@@ -317,10 +311,6 @@ class Settings {
)
);
- if ( ! wp_is_writable( $directory ) ) {
- $location_info[] = __( '⚠️ This directory does not appear to be writable.', 'woocommerce' );
- }
-
$location_info[] = sprintf(
// translators: %s is an amount of computer disk space, e.g. 5 KB.
__( 'Directory size: %s', 'woocommerce' ),
diff --git a/plugins/woocommerce/src/Internal/TransientFiles/TransientFilesEngine.php b/plugins/woocommerce/src/Internal/TransientFiles/TransientFilesEngine.php
index b62b82c1b58..723836a098b 100644
--- a/plugins/woocommerce/src/Internal/TransientFiles/TransientFilesEngine.php
+++ b/plugins/woocommerce/src/Internal/TransientFiles/TransientFilesEngine.php
@@ -6,6 +6,7 @@ use \DateTime;
use \Exception;
use \InvalidArgumentException;
use Automattic\WooCommerce\Internal\RegisterHooksInterface;
+use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
use Automattic\WooCommerce\Proxies\LegacyProxy;
use Automattic\WooCommerce\Utilities\TimeUtil;
@@ -107,10 +108,9 @@ class TransientFilesEngine implements RegisterHooksInterface {
throw new Exception( "Can't create directory: $transient_files_directory" );
}
- // Create infrastructure to prevent listing the contents of the transient files directory.
- require_once ABSPATH . 'wp-admin/includes/file.php';
- \WP_Filesystem();
- $wp_filesystem = $this->legacy_proxy->get_global( 'wp_filesystem' );
+ // Prevent listing the directory contents. Use a direct filesystem: this dir is web-server-writable
+ // under wp-content/uploads, so honoring FS_METHOD is unnecessary and breaks FTP-without-creds setups.
+ $wp_filesystem = FilesystemUtil::get_wp_filesystem_direct();
$wp_filesystem->put_contents( $transient_files_directory . '/.htaccess', 'deny from all' );
$wp_filesystem->put_contents( $transient_files_directory . '/index.html', '' );
@@ -158,9 +158,9 @@ class TransientFilesEngine implements RegisterHooksInterface {
}
$filepath = $transient_files_directory . '/' . $filename;
- require_once ABSPATH . 'wp-admin/includes/file.php';
- \WP_Filesystem();
- $wp_filesystem = $this->legacy_proxy->get_global( 'wp_filesystem' );
+ // Use a direct filesystem because the transient files directory is inside
+ // wp-content/uploads and is web-server writable. See get_transient_files_directory().
+ $wp_filesystem = FilesystemUtil::get_wp_filesystem_direct();
if ( false === $wp_filesystem->put_contents( $filepath, $file_contents ) ) {
throw new Exception( "Can't create file: $filepath" );
}
diff --git a/plugins/woocommerce/src/Internal/Utilities/FilesystemUtil.php b/plugins/woocommerce/src/Internal/Utilities/FilesystemUtil.php
index 7776082f18d..1c88c00761d 100644
--- a/plugins/woocommerce/src/Internal/Utilities/FilesystemUtil.php
+++ b/plugins/woocommerce/src/Internal/Utilities/FilesystemUtil.php
@@ -7,6 +7,7 @@ use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Proxies\LegacyProxy;
use Exception;
use WP_Filesystem_Base;
+use WP_Filesystem_Direct;
/**
* FilesystemUtil class.
@@ -33,6 +34,14 @@ class FilesystemUtil {
*/
private const FTP_INIT_COOLDOWN_MINUTES = 2;
+ /**
+ * Memoized direct filesystem. Only ever holds a genuine WP_Filesystem_Direct;
+ * the defensive fallback is never cached so a later call can retry direct.
+ *
+ * @var WP_Filesystem_Base|null
+ */
+ private static ?WP_Filesystem_Base $cached_direct_filesystem = null;
+
/**
* Wrapper to retrieve the class instance contained in the $wp_filesystem global, after initializing if necessary.
*
@@ -51,6 +60,61 @@ class FilesystemUtil {
return $wp_filesystem;
}
+ /**
+ * Get a direct filesystem instance, bypassing the configured FS_METHOD.
+ *
+ * This is appropriate for paths that are guaranteed to be writable by the
+ * web server process (such as anything inside the uploads directory). Using
+ * the configured FS_METHOD for those paths is unnecessary and breaks on
+ * sites where FS_METHOD is set to an FTP-based method without complete
+ * credentials, even though the target paths are directly writable.
+ *
+ * Defensive fallback: if WP_Filesystem_Direct cannot be loaded or instantiated,
+ * fall back to {@see self::get_wp_filesystem()} with a _doing_it_wrong notice.
+ *
+ * @since 11.0.0
+ *
+ * @return WP_Filesystem_Base Normally a WP_Filesystem_Direct instance.
+ * @throws Exception If both the direct class and the configured FS_METHOD
+ * filesystem fail to initialize (the fallback path).
+ */
+ public static function get_wp_filesystem_direct(): WP_Filesystem_Base {
+ if ( null !== self::$cached_direct_filesystem ) {
+ return self::$cached_direct_filesystem;
+ }
+
+ // require_once is a no-op if the class is already loaded, so no class_exists guard is needed.
+ require_once ABSPATH . 'wp-admin/includes/class-wp-filesystem-base.php';
+ require_once ABSPATH . 'wp-admin/includes/class-wp-filesystem-direct.php';
+
+ if ( class_exists( WP_Filesystem_Direct::class ) ) {
+ try {
+ // WP_Filesystem_Direct::chmod()/put_contents() use the FS_CHMOD_* constants when no mode is
+ // passed; core only defines them in WP_Filesystem(), which we skip, so mirror them here.
+ if ( ! defined( 'FS_CHMOD_DIR' ) ) {
+ define( 'FS_CHMOD_DIR', ( fileperms( ABSPATH ) & 0777 | 0755 ) );
+ }
+ if ( ! defined( 'FS_CHMOD_FILE' ) ) {
+ define( 'FS_CHMOD_FILE', ( fileperms( ABSPATH . 'index.php' ) & 0777 | 0644 ) );
+ }
+
+ self::$cached_direct_filesystem = new WP_Filesystem_Direct( null );
+ return self::$cached_direct_filesystem;
+ } catch ( \Throwable $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch -- Fall through to the fallback below.
+ }
+ }
+
+ _doing_it_wrong(
+ __METHOD__,
+ esc_html__( 'WP_Filesystem_Direct could not be loaded. Falling back to the configured FS_METHOD; operations on the uploads directory may fail if FS_METHOD is misconfigured.', 'woocommerce' ),
+ '11.0.0'
+ );
+
+ // Deliberately uncached: the fallback may be a non-direct (FTP) instance, and caching it would pin
+ // every later "direct" caller to FS_METHOD; leaving it uncached lets a later call retry direct.
+ return self::get_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.
*
@@ -111,6 +175,10 @@ class FilesystemUtil {
* Recursively creates a directory (if it doesn't exist) and adds an empty index.html and a .htaccess to prevent
* directory listing.
*
+ * Always uses a direct filesystem for the file operations, since the
+ * caller is responsible for choosing a path that is writable by the web
+ * server process.
+ *
* @since 9.3.0
*
* @param string $path Directory to create.
@@ -118,7 +186,7 @@ class FilesystemUtil {
* @throws \Exception In case of error.
*/
public static function mkdir_p_not_indexable( string $path, bool $allow_file_access = false ): void {
- $wp_fs = self::get_wp_filesystem();
+ $wp_fs = self::get_wp_filesystem_direct();
if ( $wp_fs->is_dir( $path ) ) {
return;
@@ -218,11 +286,15 @@ class FilesystemUtil {
/**
* Validate that a file path is a valid upload path.
*
+ * Uses a direct filesystem instance rather than honoring FS_METHOD: this
+ * check only inspects paths under ABSPATH or the uploads directory, both
+ * of which the web server can read directly.
+ *
* @param string $path The path to validate.
* @throws \Exception If the file path is not a valid upload path.
*/
public static function validate_upload_file_path( string $path ): void {
- $wp_filesystem = self::get_wp_filesystem();
+ $wp_filesystem = self::get_wp_filesystem_direct();
// File must exist and be readable.
$is_valid_file = $wp_filesystem->is_readable( $path );
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/SettingsTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/SettingsTest.php
index 6346c63cb32..e6af7a35a1a 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/SettingsTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Logging/SettingsTest.php
@@ -410,4 +410,25 @@ class SettingsTest extends WC_Unit_Test_Case {
Constants::clear_single_constant( 'WC_LOG_THRESHOLD' );
}
+
+ /**
+ * @testdox The filesystem settings status panel reports Ready when the log directory is writable.
+ *
+ * The complementary "not writable" branch is intentionally not asserted here:
+ * making wp_is_writable() return false requires either (a) running as a
+ * non-root user with a chmod-restricted directory, which is unreliable in
+ * containerized test environments that run PHP CLI as root, or (b) mocking
+ * a non-proxied core function. Inverting the conditional or breaking the
+ * "Ready" branch is the regression most likely to ship; this test catches
+ * that.
+ */
+ public function test_render_form_filesystem_status_ready_when_writable(): void {
+ // The default upload-based log directory is writable in the test environment.
+ ob_start();
+ $this->sut->render_form();
+ $content = ob_get_clean();
+
+ $this->assertStringContainsString( '✅ Ready', $content );
+ $this->assertStringNotContainsString( 'The log directory is not writable.', $content );
+ }
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/TransientFiles/TransientFilesEngineTest.php b/plugins/woocommerce/tests/php/src/Internal/TransientFiles/TransientFilesEngineTest.php
index 1e770bdeda4..026745552d0 100644
--- a/plugins/woocommerce/tests/php/src/Internal/TransientFiles/TransientFilesEngineTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/TransientFiles/TransientFilesEngineTest.php
@@ -113,10 +113,13 @@ class TransientFilesEngineTest extends \WC_REST_Unit_Test_Case {
* @testdox create_transient_file throws an exception if the transient file can't be created.
*/
public function test_create_transient_file_throws_if_file_cant_be_created() {
+ // Point the engine at a directory that does not exist on disk and bypass mkdir
+ // so put_contents (native @file_put_contents under WP_Filesystem_Direct) fails.
+ $nonexistent_base = sys_get_temp_dir() . '/wc-transient-nonexistent-' . wp_generate_uuid4();
$this->register_legacy_proxy_function_mocks(
array(
- 'wp_upload_dir' => fn() => array( 'basedir' => '/wordpress/uploads' ),
- 'realpath' => fn( $path ) => '/real' . $path,
+ 'wp_upload_dir' => fn() => array( 'basedir' => $nonexistent_base ),
+ 'realpath' => fn( $path ) => $path,
'is_dir' => fn() => true,
'random_bytes' => fn( $length ) => implode( array_map( 'chr', array( 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ) ) ),
'gmdate' => fn( $format, $date = null ) =>
@@ -124,22 +127,8 @@ class TransientFilesEngineTest extends \WC_REST_Unit_Test_Case {
),
);
- // phpcs:disable Squiz.Commenting.FunctionComment.Missing
- $fake_wp_filesystem = new class() {
- public function put_contents( string $file, string $contents, $mode = false ): bool {
- return false;
- }
- };
- // phpcs:enable Squiz.Commenting.FunctionComment.Missing
-
- $this->register_legacy_proxy_global_mocks(
- array(
- 'wp_filesystem' => $fake_wp_filesystem,
- )
- );
-
$this->expectException( \Exception::class );
- $this->expectExceptionMessage( "Can't create file: /real/wordpress/uploads/woocommerce_transient_files/2023-12-02/000102030405060708090a0b0c0d0e0f" );
+ $this->expectExceptionMessage( "Can't create file: " . $nonexistent_base . '/woocommerce_transient_files/2023-12-02/000102030405060708090a0b0c0d0e0f' );
$this->sut->create_transient_file( 'foobar', '2023-12-02' );
}
@@ -247,43 +236,40 @@ class TransientFilesEngineTest extends \WC_REST_Unit_Test_Case {
* @testdox get_transient_files_directory creates the default base directory if it doesn't exist and the woocommerce_transient_files_directory filter is not used.
*/
public function test_get_transient_files_directory_creates_default_directory_if_it_does_not_exist() {
- $created_dir = null;
+ // Use a real temp upload base so put_contents (which now goes through
+ // WP_Filesystem_Direct → native @file_put_contents) can actually write.
+ $upload_base = sys_get_temp_dir() . '/wc-transient-create-' . wp_generate_uuid4();
+ $transient_dir = $upload_base . '/woocommerce_transient_files';
$this->register_legacy_proxy_function_mocks(
array(
- 'wp_upload_dir' => fn() => array( 'basedir' => '/wordpress/uploads' ),
- 'realpath' => fn( $path ) => false,
- 'wp_mkdir_p' => function( $directory ) use ( &$created_dir ) {
- $created_dir = $directory;
- return true; },
- )
- );
-
- // phpcs:disable Squiz.Commenting
- $fake_wp_filesystem = new class() {
- public $created_files = array();
-
- public function put_contents( string $file, string $contents, $mode = false ): int {
- $this->created_files[ $file ] = $contents;
- return strlen( $contents );
- }
- };
- // phpcs:enable Squiz.Commenting
-
- $this->register_legacy_proxy_global_mocks(
- array(
- 'wp_filesystem' => $fake_wp_filesystem,
+ 'wp_upload_dir' => fn() => array( 'basedir' => $upload_base ),
+ // First realpath() call returns false to force the create branch; rely on
+ // the real wp_mkdir_p afterward so the second realpath() (unmocked) succeeds.
+ 'realpath' => function ( $path ) {
+ static $first_call = true;
+ if ( $first_call ) {
+ $first_call = false;
+ return false;
+ }
+ return realpath( $path );
+ },
+ 'wp_mkdir_p' => fn( $directory ) => wp_mkdir_p( $directory ),
)
);
- $this->sut->get_transient_files_directory();
- $this->assertEquals( '/wordpress/uploads/woocommerce_transient_files', $created_dir );
-
- $expected_created_files = array(
- '/wordpress/uploads/woocommerce_transient_files/.htaccess' => 'deny from all',
- '/wordpress/uploads/woocommerce_transient_files/index.html' => '',
- );
- $this->assertEquals( $expected_created_files, $fake_wp_filesystem->created_files );
+ try {
+ $result = $this->sut->get_transient_files_directory();
+
+ $this->assertEquals( realpath( $transient_dir ), $result );
+ $this->assertDirectoryExists( $transient_dir );
+ $this->assertFileExists( $transient_dir . '/.htaccess' );
+ $this->assertEquals( 'deny from all', file_get_contents( $transient_dir . '/.htaccess' ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
+ $this->assertFileExists( $transient_dir . '/index.html' );
+ $this->assertEquals( '', file_get_contents( $transient_dir . '/index.html' ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
+ } finally {
+ self::rmdir_recursive( $upload_base, true );
+ }
}
/**
diff --git a/plugins/woocommerce/tests/php/src/Internal/Utilities/FilesystemUtilTest.php b/plugins/woocommerce/tests/php/src/Internal/Utilities/FilesystemUtilTest.php
index 260cf374884..7d8ba284093 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Utilities/FilesystemUtilTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Utilities/FilesystemUtilTest.php
@@ -7,11 +7,26 @@ use Automattic\Jetpack\Constants;
use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
use WC_Unit_Test_Case;
use WP_Filesystem_Base;
+use WP_Filesystem_Direct;
/**
* FilesystemUtilTest class.
*/
class FilesystemUtilTest extends WC_Unit_Test_Case {
+ /**
+ * Tracked temp files to clean up after each test.
+ *
+ * @var string[]
+ */
+ private $temp_files = array();
+
+ /**
+ * Tracked temp directories to clean up after each test.
+ *
+ * @var string[]
+ */
+ private $temp_dirs = array();
+
/**
* Set up before running any tests.
*
@@ -32,13 +47,60 @@ class FilesystemUtilTest extends WC_Unit_Test_Case {
* @return void
*/
public function tearDown(): void {
+ foreach ( $this->temp_files as $temp_file ) {
+ if ( file_exists( $temp_file ) ) {
+ wp_delete_file( $temp_file );
+ }
+ }
+ foreach ( $this->temp_dirs as $temp_dir ) {
+ if ( is_dir( $temp_dir ) ) {
+ // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_rmdir -- Test cleanup of an empty directory we created in this process.
+ rmdir( $temp_dir );
+ }
+ }
+ $this->temp_files = array();
+ $this->temp_dirs = array();
+
unset( $GLOBALS['wp_filesystem'] );
$this->reset_legacy_proxy_mocks();
Constants::clear_constants();
+ $this->reset_direct_filesystem_cache();
parent::tearDown();
}
+ /**
+ * Clear FilesystemUtil's memoized direct filesystem so each test starts
+ * from a cold cache. Without this, a warm cache makes the FS_METHOD-bypass
+ * test pass trivially from cache instead of exercising construction.
+ *
+ * @return void
+ */
+ private function reset_direct_filesystem_cache(): void {
+ $property = new \ReflectionProperty( FilesystemUtil::class, 'cached_direct_filesystem' );
+ $property->setAccessible( true );
+ $property->setValue( null, null );
+ }
+
+ /**
+ * Create a real temp file inside a directory and track it for cleanup.
+ *
+ * @param string $dir Directory to create the file in.
+ * @return string The absolute path of the file.
+ */
+ private function make_temp_file( string $dir ): string {
+ if ( ! is_dir( $dir ) ) {
+ wp_mkdir_p( $dir );
+ $this->temp_dirs[] = $dir;
+ }
+ $path = tempnam( $dir, 'fsutil_' );
+ if ( false === $path ) {
+ throw new \RuntimeException( esc_html( "Could not create a temp file in {$dir}." ) );
+ }
+ $this->temp_files[] = $path;
+ return $path;
+ }
+
/**
* @testdox Check that the get_wp_filesystem method returns an appropriate class instance.
*/
@@ -162,130 +224,140 @@ class FilesystemUtilTest extends WC_Unit_Test_Case {
}
/**
- * @testdox 'validate_upload_file_path' returns without throwing an exception if the file path is valid.
+ * @testdox 'get_wp_filesystem_direct' returns a WP_Filesystem_Direct instance regardless of FS_METHOD.
*/
- public function test_validate_upload_file_path_success() {
- $this->expectNotToPerformAssertions();
+ public function test_get_wp_filesystem_direct_returns_direct_even_with_ftp_method(): void {
+ Constants::set_constant( 'FS_METHOD', 'ftpext' );
+ Constants::set_constant( 'FTP_HOST', 'ftp.example.com' );
- global $wp_filesystem;
- $original_wp_filesystem = $wp_filesystem;
- $mock_wp_filesystem = $this->createMock( WP_Filesystem_Base::class );
- $mock_wp_filesystem->method( 'is_readable' )->willReturn( true );
- $mock_wp_filesystem->method( 'abspath' )->willReturn( ABSPATH );
- $wp_filesystem = $mock_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ $result = FilesystemUtil::get_wp_filesystem_direct();
- FilesystemUtil::validate_upload_file_path( ABSPATH . 'test.txt' );
-
- $wp_filesystem = $original_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ $this->assertInstanceOf( WP_Filesystem_Direct::class, $result );
+ $this->assertSame( 'direct', $result->method );
}
/**
- * @testdox 'validate_upload_file_path' throws an exception if the filesystem cannot be initialized.
+ * @testdox 'get_wp_filesystem_direct' returns the same cached instance on repeated calls.
*/
- public function test_validate_upload_file_path_failure_on_initialize_wp_filesystem() {
- Constants::set_constant( 'FS_METHOD', null );
+ public function test_get_wp_filesystem_direct_caches_instance(): void {
+ $first = FilesystemUtil::get_wp_filesystem_direct();
+ $second = FilesystemUtil::get_wp_filesystem_direct();
- $this->expectException( 'Exception' );
-
- FilesystemUtil::validate_upload_file_path( ABSPATH . 'test.txt' );
+ $this->assertSame( $first, $second );
}
/**
- * @testdox 'validate_upload_file_path' throws an exception if the file path is not readable.
+ * @testdox 'get_wp_filesystem_direct' returns an instance whose method is 'direct' when used to write to a known-writable temp dir.
*/
- public function test_validate_upload_file_path_failure_on_not_readable() {
- $this->expectException( 'Exception' );
+ public function test_get_wp_filesystem_direct_writes_through_native_php(): void {
+ $dir = sys_get_temp_dir() . '/wc-fsutil-write-' . wp_generate_uuid4();
+ wp_mkdir_p( $dir );
+ $this->temp_dirs[] = $dir;
- global $wp_filesystem;
- $original_wp_filesystem = $wp_filesystem;
- $mock_wp_filesystem = $this->createMock( WP_Filesystem_Base::class );
- $mock_wp_filesystem->method( 'is_readable' )->willReturn( false );
- $mock_wp_filesystem->method( 'abspath' )->willReturn( ABSPATH );
- $wp_filesystem = $mock_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ $path = $dir . '/sentinel.txt';
+ $value = 'hello-' . wp_generate_uuid4();
+ try {
+ $wp_fs = FilesystemUtil::get_wp_filesystem_direct();
+ $result = $wp_fs->put_contents( $path, $value );
+ } finally {
+ if ( file_exists( $path ) ) {
+ $this->temp_files[] = $path;
+ }
+ }
- FilesystemUtil::validate_upload_file_path( ABSPATH . 'test.txt' );
+ $this->assertTrue( $result );
+ // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- Local file we just wrote, not a remote URL.
+ $this->assertSame( $value, file_get_contents( $path ) );
- $wp_filesystem = $original_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ // A successful put_contents() with no explicit mode proves the FS_CHMOD_*
+ // constants were available; assert them directly to lock in the fix for
+ // the "Undefined constant FS_CHMOD_FILE" fatal that motivated this method.
+ $this->assertTrue( defined( 'FS_CHMOD_DIR' ) );
+ $this->assertTrue( defined( 'FS_CHMOD_FILE' ) );
}
/**
- * @testdox 'validate_upload_file_path' throws an exception if the file path is not in the upload directory.
+ * @testdox 'validate_upload_file_path' returns without throwing for a real file inside ABSPATH.
*/
- public function test_validate_upload_file_path_failure_on_not_in_directory() {
- $this->expectException( 'Exception' );
+ public function test_validate_upload_file_path_success(): void {
+ $this->expectNotToPerformAssertions();
- global $wp_filesystem;
- $original_wp_filesystem = $wp_filesystem;
- $mock_wp_filesystem = $this->createMock( WP_Filesystem_Base::class );
- $mock_wp_filesystem->method( 'is_readable' )->willReturn( true );
- $mock_wp_filesystem->method( 'abspath' )->willReturn( ABSPATH );
- $wp_filesystem = $mock_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ // Use an existing readable core file so the test does not depend on
+ // ABSPATH being writable.
+ FilesystemUtil::validate_upload_file_path( ABSPATH . 'index.php' );
+ }
- FilesystemUtil::validate_upload_file_path( '/etc/test.txt' );
+ /**
+ * @testdox 'validate_upload_file_path' throws an exception if the file path is not readable.
+ */
+ public function test_validate_upload_file_path_failure_on_not_readable(): void {
+ $this->expectException( 'Exception' );
- $wp_filesystem = $original_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ FilesystemUtil::validate_upload_file_path( ABSPATH . 'definitely-does-not-exist-' . wp_generate_uuid4() . '.txt' );
}
/**
- * @testdox 'validate_upload_file_path' returns without throwing an exception if the file path is in the upload directory.
+ * @testdox 'validate_upload_file_path' throws when the file is outside ABSPATH and the uploads directory.
*/
- public function test_validate_upload_file_path_success_with_upload_dir() {
- $this->expectNotToPerformAssertions();
+ public function test_validate_upload_file_path_failure_on_not_in_directory(): void {
+ $this->expectException( 'Exception' );
- $callback = fn() => array(
- 'path' => '/uploads/',
- 'basedir' => '/uploads/',
- 'error' => false,
- );
- add_filter( 'upload_dir', $callback );
+ $outside_dir = sys_get_temp_dir() . '/wc-fsutil-outside-' . wp_generate_uuid4();
+ $path = $this->make_temp_file( $outside_dir );
- global $wp_filesystem;
- $original_wp_filesystem = $wp_filesystem;
- $mock_wp_filesystem = $this->createMock( WP_Filesystem_Base::class );
- $mock_wp_filesystem->method( 'is_readable' )->willReturn( true );
- $mock_wp_filesystem->method( 'abspath' )->willReturn( ABSPATH );
- $wp_filesystem = $mock_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ // Make sure the temp file is genuinely outside ABSPATH and uploads.
+ $abspath_real = wp_normalize_path( realpath( ABSPATH ) );
+ $path_real = wp_normalize_path( realpath( $path ) );
+ $this->assertStringStartsNotWith( $abspath_real, $path_real );
- FilesystemUtil::validate_upload_file_path( '/uploads/test.txt' );
-
- $wp_filesystem = $original_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
- remove_filter( 'upload_dir', $callback );
+ FilesystemUtil::validate_upload_file_path( $path );
}
/**
- * @testdox 'validate_upload_file_path' returns without throwing an exception if the file path has a file:// protocol.
+ * @testdox 'validate_upload_file_path' returns without throwing for a real file inside the uploads directory.
*/
- public function test_validate_upload_file_path_success_with_file_protocol() {
+ public function test_validate_upload_file_path_success_with_upload_dir(): void {
$this->expectNotToPerformAssertions();
- global $wp_filesystem;
- $original_wp_filesystem = $wp_filesystem;
- $mock_wp_filesystem = $this->createMock( WP_Filesystem_Base::class );
- $mock_wp_filesystem->method( 'is_readable' )->willReturn( true );
- $mock_wp_filesystem->method( 'abspath' )->willReturn( ABSPATH );
- $wp_filesystem = $mock_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
-
- FilesystemUtil::validate_upload_file_path( 'file://' . ABSPATH . 'test.txt' );
+ $upload_dir = wp_get_upload_dir();
+ $path = $this->make_temp_file( $upload_dir['basedir'] );
- $wp_filesystem = $original_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ FilesystemUtil::validate_upload_file_path( $path );
}
/**
- * @testdox 'validate_upload_file_path' returns without throwing an exception if the file path has a protocol other than file://.
+ * @testdox 'validate_upload_file_path' accepts a file:// protocol prefix on a real path inside ABSPATH.
*/
- public function test_validate_upload_file_path_success_with_other_protocol() {
+ public function test_validate_upload_file_path_success_with_file_protocol(): void {
$this->expectNotToPerformAssertions();
- global $wp_filesystem;
- $original_wp_filesystem = $wp_filesystem;
- $mock_wp_filesystem = $this->createMock( WP_Filesystem_Base::class );
- $mock_wp_filesystem->method( 'is_readable' )->willReturn( true );
- $mock_wp_filesystem->method( 'abspath' )->willReturn( 's3://mock-bucket/' );
- $wp_filesystem = $mock_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ // Use an existing readable core file so the test does not depend on
+ // ABSPATH being writable.
+ FilesystemUtil::validate_upload_file_path( 'file://' . ABSPATH . 'index.php' );
+ }
- FilesystemUtil::validate_upload_file_path( 's3://mock-bucket/test.txt' );
+ /**
+ * @testdox 'file_is_in_directory' keeps stream-wrapper (e.g. s3://) containment intact for upload paths.
+ *
+ * Exercises the non-file:// protocol branch of the containment check. The
+ * public validate_upload_file_path() gates on is_readable() first, which a
+ * real direct filesystem cannot satisfy for an unregistered s3:// path in a
+ * unit test, so the protocol branch is verified directly. This restores the
+ * coverage previously provided by the (now removed) abspath()-mocking test
+ * and locks in the no-regression claim for WordPress VIP / S3-Uploads sites.
+ */
+ public function test_file_is_in_directory_handles_stream_wrapper_protocol(): void {
+ $method = new \ReflectionMethod( FilesystemUtil::class, 'file_is_in_directory' );
+ $method->setAccessible( true );
- $wp_filesystem = $original_wp_filesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+ // A path inside an s3:// uploads basedir is contained.
+ $this->assertTrue(
+ $method->invoke( null, 's3://mock-bucket/test.txt', 's3://mock-bucket/' )
+ );
+ // A path under a different bucket is rejected.
+ $this->assertFalse(
+ $method->invoke( null, 's3://other-bucket/test.txt', 's3://mock-bucket/' )
+ );
}
/**