Logging: Add more robust method of filesystem initialization to handle configuration edge cases (#45914)

* Add FilesystemUtil class and unit tests

* Update logger to use FilesystemUtil

* phpcs cleanup

* Add changelog file

* Extract credentials gathering into a separate method

* Add notice when filesystem init fails

* Simplify getting filesystem credentials

* Tweak messaging in Settings when filesystem is not direct

* Remove unnecessary conditional in file deletion method

* Refactor test_clear

* Restore test_clear to its previous state

* Prevent MaxMind unit tests from polluting the state of WP_Filesystem

plugins/woocommerce/changelog/try-logging-filesystem-checks

@@ -0,0 +1,4 @@
+Significance: minor
+Type: update
+
+Add more robust method of filesystem initialization

plugins/woocommerce/src/Internal/Admin/Logging/FileV2/File.php

@@ -4,7 +4,8 @@ declare( strict_types = 1 );
 namespace Automattic\WooCommerce\Internal\Admin\Logging\FileV2;
 
 use Automattic\Jetpack\Constants;
-use WP_Filesystem_Direct;
+use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
+use Exception;
 
 /**
  * File class.
@@ -60,14 +61,6 @@ class File {
 	 * @param string $path The absolute path of the file.
 	 */
 	public function __construct( $path ) {
-		require_once ABSPATH . 'wp-admin/includes/file.php';
-
-		global $wp_filesystem;
-
-		if ( ! $wp_filesystem instanceof WP_Filesystem_Direct ) {
-			WP_Filesystem();
-		}
-
 		$this->path = $path;
 		$this->ingest_path();
 	}
@@ -237,27 +230,33 @@ class File {
 	/**
 	 * Check if the file represented by the class instance is a file and is readable.
 	 *
-	 * @global WP_Filesystem_Direct $wp_filesystem
-	 *
 	 * @return bool
 	 */
 	public function is_readable(): bool {
-		global $wp_filesystem;
+		try {
+			$filesystem  = FilesystemUtil::get_wp_filesystem();
+			$is_readable = $filesystem->is_file( $this->path ) && $filesystem->is_readable( $this->path );
+		} catch ( Exception $exception ) {
+			return false;
+		}
 
-		return $wp_filesystem->is_file( $this->path ) && $wp_filesystem->is_readable( $this->path );
+		return $is_readable;
 	}
 
 	/**
 	 * Check if the file represented by the class instance is a file and is writable.
 	 *
-	 * @global WP_Filesystem_Direct $wp_filesystem
-	 *
 	 * @return bool
 	 */
 	public function is_writable(): bool {
-		global $wp_filesystem;
+		try {
+			$filesystem  = FilesystemUtil::get_wp_filesystem();
+			$is_writable = $filesystem->is_file( $this->path ) && $filesystem->is_writable( $this->path );
+		} catch ( Exception $exception ) {
+			return false;
+		}
 
-		return $wp_filesystem->is_file( $this->path ) && $wp_filesystem->is_writable( $this->path );
+		return $is_writable;
 	}
 
 	/**
@@ -372,31 +371,38 @@ class File {
 	/**
 	 * Get the time of the last modification of the file, as a Unix timestamp. Or false if the file isn't readable.
 	 *
-	 * @global WP_Filesystem_Direct $wp_filesystem
-	 *
 	 * @return int|false
 	 */
 	public function get_modified_timestamp() {
-		global $wp_filesystem;
+		try {
+			$filesystem = FilesystemUtil::get_wp_filesystem();
+			$timestamp  = $filesystem->mtime( $this->path );
+		} catch ( Exception $exception ) {
+			return false;
+		}
 
-		return $wp_filesystem->mtime( $this->path );
+		return $timestamp;
 	}
 
 	/**
 	 * Get the size of the file in bytes. Or false if the file isn't readable.
 	 *
-	 * @global WP_Filesystem_Direct $wp_filesystem
-	 *
 	 * @return int|false
 	 */
 	public function get_file_size() {
-		global $wp_filesystem;
+		try {
+			$filesystem = FilesystemUtil::get_wp_filesystem();
 
-		if ( ! $wp_filesystem->is_readable( $this->path ) ) {
+			if ( ! $filesystem->is_readable( $this->path ) ) {
+				return false;
+			}
+
+			$size = $filesystem->size( $this->path );
+		} catch ( Exception $exception ) {
 			return false;
 		}
 
-		return $wp_filesystem->size( $this->path );
+		return $size;
 	}
 
 	/**
@@ -405,10 +411,13 @@ class File {
 	 * @return bool
 	 */
 	protected function create(): bool {
-		global $wp_filesystem;
-
-		$created = $wp_filesystem->touch( $this->path );
-		$modded  = $wp_filesystem->chmod( $this->path );
+		try {
+			$filesystem = FilesystemUtil::get_wp_filesystem();
+			$created    = $filesystem->touch( $this->path );
+			$modded     = $filesystem->chmod( $this->path );
+		} catch ( Exception $exception ) {
+			return false;
+		}
 
 		return $created && $modded;
 	}
@@ -463,8 +472,6 @@ class File {
 			return false;
 		}
 
-		global $wp_filesystem;
-
 		$created = 0;
 		if ( $this->has_standard_filename() ) {
 			$created = $this->get_created_timestamp();
@@ -489,7 +496,13 @@ class File {
 		$new_filename = str_replace( $search, $replace, $old_filename );
 		$new_path     = str_replace( $old_filename, $new_filename, $this->path );
 
-		$moved = $wp_filesystem->move( $this->path, $new_path, true );
+		try {
+			$filesystem = FilesystemUtil::get_wp_filesystem();
+			$moved      = $filesystem->move( $this->path, $new_path, true );
+		} catch ( Exception $exception ) {
+			return false;
+		}
+
 		if ( ! $moved ) {
 			return false;
 		}
@@ -503,13 +516,16 @@ class File {
 	/**
 	 * Delete the file from the filesystem.
 	 *
-	 * @global WP_Filesystem_Direct $wp_filesystem
-	 *
 	 * @return bool True on success, false on failure.
 	 */
 	public function delete(): bool {
-		global $wp_filesystem;
+		try {
+			$filesystem = FilesystemUtil::get_wp_filesystem();
+			$deleted    = $filesystem->delete( $this->path, false, 'f' );
+		} catch ( Exception $exception ) {
+			return false;
+		}
 
-		return $wp_filesystem->delete( $this->path, false, 'f' );
+		return $deleted;
 	}
 }

plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileController.php

@@ -481,10 +481,7 @@ class FileController {
 
 		$files = $this->get_files_by_id( $file_ids );
 		foreach ( $files as $file ) {
-			$result = false;
-			if ( $file->is_writable() ) {
-				$result = $file->delete();
-			}
+			$result = $file->delete();
 
 			if ( true === $result ) {
 				$deleted ++;

plugins/woocommerce/src/Internal/Admin/Logging/FileV2/FileExporter.php

@@ -3,8 +3,9 @@ declare( strict_types=1 );
 
 namespace Automattic\WooCommerce\Internal\Admin\Logging\FileV2;
 
+use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
+use Exception;
 use WP_Error;
-use WP_Filesystem_Direct;
 
 /**
  * FileExport class.
@@ -39,11 +40,6 @@ class FileExporter {
 	 *                                   part of the path.
 	 */
 	public function __construct( string $path, string $alternate_filename = '' ) {
-		global $wp_filesystem;
-		if ( ! $wp_filesystem instanceof WP_Filesystem_Direct ) {
-			WP_Filesystem();
-		}
-
 		$this->path               = $path;
 		$this->alternate_filename = $alternate_filename;
 	}
@@ -54,8 +50,14 @@ class FileExporter {
 	 * @return WP_Error|void Only returns something if there is an error.
 	 */
 	public function emit_file() {
-		global $wp_filesystem;
-		if ( ! $wp_filesystem->is_file( $this->path ) || ! $wp_filesystem->is_readable( $this->path ) ) {
+		try {
+			$filesystem  = FilesystemUtil::get_wp_filesystem();
+			$is_readable = ! $filesystem->is_file( $this->path ) || ! $filesystem->is_readable( $this->path );
+		} catch ( Exception $exception ) {
+			$is_readable = false;
+		}
+
+		if ( ! $is_readable ) {
 			return new WP_Error(
 				'wc_logs_invalid_file',
 				__( 'Could not access file.', 'woocommerce' )

plugins/woocommerce/src/Internal/Admin/Logging/Settings.php

@@ -8,10 +8,12 @@ use Automattic\WooCommerce\Internal\Admin\Logging\FileV2\File;
 use Automattic\WooCommerce\Internal\Admin\Logging\LogHandlerFileV2;
 use Automattic\WooCommerce\Internal\Admin\Logging\FileV2\FileController;
 use Automattic\WooCommerce\Internal\Traits\AccessiblePrivateMethods;
+use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
 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_Base;
+use WP_Filesystem_Direct;
 
 /**
  * Settings class.
@@ -78,14 +80,13 @@ class Settings {
 
 			if ( true === $result ) {
 				// Create infrastructure to prevent listing contents of the logs directory.
-				require_once ABSPATH . 'wp-admin/includes/file.php';
-				global $wp_filesystem;
-				if ( ! $wp_filesystem instanceof WP_Filesystem_Base ) {
-					WP_Filesystem();
+				try {
+					$filesystem = FilesystemUtil::get_wp_filesystem();
+					$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.
 				}
-
-				$wp_filesystem->put_contents( $dir . '.htaccess', 'deny from all' );
-				$wp_filesystem->put_contents( $dir . 'index.html', '' );
 			}
 		}
 
@@ -292,6 +293,20 @@ class Settings {
 		$location_info = array();
 		$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' );
+			$status_info[] = __( 'You may want to switch to the database for log storage.', 'woocommerce' );
+		}
+
 		$location_info[] = sprintf(
 			// translators: %s is a location in the filesystem.
 			__( 'Log files are stored in this directory: %s', 'woocommerce' ),
@@ -317,6 +332,11 @@ class Settings {
 				'id'    => self::PREFIX . 'settings',
 				'type'  => 'title',
 			),
+			'file_status'   => array(
+				'title' => __( 'Status', 'woocommerce' ),
+				'type'  => 'info',
+				'text'  => implode( "\n\n", $status_info ),
+			),
 			'log_directory' => array(
 				'title' => __( 'Location', 'woocommerce' ),
 				'type'  => 'info',
@@ -340,7 +360,7 @@ class Settings {
 		$table = "{$wpdb->prefix}woocommerce_log";
 
 		$location_info = sprintf(
-			// translators: %s is a location in the filesystem.
+			// translators: %s is the name of a table in the database.
 			__( 'Log entries are stored in this database table: %s', 'woocommerce' ),
 			"<code>$table</code>"
 		);

plugins/woocommerce/src/Internal/Utilities/FilesystemUtil.php

@@ -0,0 +1,64 @@
+<?php
+declare( strict_types = 1 );
+
+namespace Automattic\WooCommerce\Internal\Utilities;
+
+use Automattic\Jetpack\Constants;
+use Exception;
+use WP_Filesystem_Base;
+
+/**
+ * FilesystemUtil class.
+ */
+class FilesystemUtil {
+	/**
+	 * Wrapper to retrieve the class instance contained in the $wp_filesystem global, after initializing if necessary.
+	 *
+	 * @return WP_Filesystem_Base
+	 * @throws Exception Thrown when the filesystem fails to initialize.
+	 */
+	public static function get_wp_filesystem(): WP_Filesystem_Base {
+		global $wp_filesystem;
+
+		if ( ! $wp_filesystem instanceof WP_Filesystem_Base ) {
+			$initialized = self::initialize_wp_filesystem();
+
+			if ( false === $initialized ) {
+				throw new Exception( 'The WordPress filesystem could not be initialized.' );
+			}
+		}
+
+		return $wp_filesystem;
+	}
+
+	/**
+	 * Wrapper to initialize the WP filesystem with defined credentials if they are available.
+	 *
+	 * @return bool True if the $wp_filesystem global was successfully initialized.
+	 */
+	protected static function initialize_wp_filesystem(): bool {
+		global $wp_filesystem;
+
+		if ( $wp_filesystem instanceof WP_Filesystem_Base ) {
+			return true;
+		}
+
+		require_once ABSPATH . 'wp-admin/includes/file.php';
+
+		$method      = get_filesystem_method();
+		$initialized = false;
+
+		if ( 'direct' === $method ) {
+			$initialized = WP_Filesystem();
+		} elseif ( false !== $method ) {
+			// See https://core.trac.wordpress.org/changeset/56341.
+			ob_start();
+			$credentials = request_filesystem_credentials( '' );
+			ob_end_clean();
+
+			$initialized = $credentials && WP_Filesystem( $credentials );
+		}
+
+		return is_null( $initialized ) ? false : $initialized;
+	}
+}

plugins/woocommerce/tests/legacy/unit-tests/integrations/maxmind-geolocation/class-wc-tests-maxmind-integration.php

@@ -33,6 +33,18 @@ class WC_Tests_MaxMind_Integration extends WC_Unit_Test_Case {
 		add_filter( 'woocommerce_maxmind_geolocation_database_service', array( $this, 'override_integration_service' ) );
 	}
 
+	/**
+	 * Clean up after each test.
+	 */
+	public function tearDown(): void {
+		unset( $GLOBALS['wp_filesystem'] );
+
+		remove_filter( 'filesystem_method', array( $this, 'override_filesystem_method' ) );
+		remove_filter( 'woocommerce_maxmind_geolocation_database_service', array( $this, 'override_integration_service' ) );
+
+		parent::tearDown();
+	}
+
 	/**
 	 * Make sure that the database is not updated if no target database path is given.
 	 */

plugins/woocommerce/tests/php/src/Internal/Admin/Logging/SettingsTest.php

@@ -117,8 +117,6 @@ class SettingsTest extends WC_Unit_Test_Case {
 		$upload_dir = wp_upload_dir();
 		$path       = $upload_dir['basedir'] . '/wc-logs-test/';
 
-		$this->assertFalse( wp_is_writable( $path ) );
-
 		$callback = fn() => $path;
 		add_filter( 'woocommerce_log_directory', $callback );
 

plugins/woocommerce/tests/php/src/Internal/Utilities/FilesystemUtilTest.php

@@ -0,0 +1,61 @@
+<?php
+declare( strict_types = 1 );
+
+namespace Automattic\WooCommerce\Tests\Internal\Utilities;
+
+use Automattic\WooCommerce\Internal\Utilities\FilesystemUtil;
+use WC_Unit_Test_Case;
+use WP_Filesystem_Base;
+
+/**
+ * FilesystemUtilTest class.
+ */
+class FilesystemUtilTest extends WC_Unit_Test_Case {
+	/**
+	 * Set up before running any tests.
+	 *
+	 * @return void
+	 */
+	public static function setUpBeforeClass(): void {
+		parent::setUpBeforeClass();
+
+		unset( $GLOBALS['wp_filesystem'] );
+	}
+
+	/**
+	 * Tear down between each test.
+	 *
+	 * @return void
+	 */
+	public function tearDown(): void {
+		unset( $GLOBALS['wp_filesystem'] );
+
+		parent::tearDown();
+	}
+
+	/**
+	 * @testdox Check that the get_wp_filesystem method returns an appropriate class instance.
+	 */
+	public function test_get_wp_filesystem_success(): void {
+		$callback = fn() => 'direct';
+		add_filter( 'filesystem_method', $callback );
+
+		$this->assertInstanceOf( WP_Filesystem_Base::class, FilesystemUtil::get_wp_filesystem() );
+
+		remove_filter( 'filesystem_method', $callback );
+	}
+
+	/**
+	 * @testdox Check that the get_wp_filesystem method throws an exception when the filesystem cannot be initialized.
+	 */
+	public function test_get_wp_filesystem_failure(): void {
+		$this->expectException( 'Exception' );
+
+		$callback = fn() => 'asdf';
+		add_filter( 'filesystem_method', $callback );
+
+		FilesystemUtil::get_wp_filesystem();
+
+		remove_filter( 'filesystem_method', $callback );
+	}
+}