Attachments: avoid fatal errors when WP filesystem isn't available

[jeherve]

Fixes #2718

Proposed changes:

Let's introduce some failsafes when WP_Filesystem isn't fully available.

Other information:

• Have you written new tests for your changes, if applicable?

Testing instructions:

I'm not sure how to best test this. It may be an option to set a custom FS_METHOD constant in wp-config.php to override the default behavior and break things.

Changelog entry

• Automatically create a changelog entry from the details below.

Changelog Entry Details

Significance

• Patch
• Minor
• Major

Type

• Added - for new features
• Changed - for changes in existing functionality
• Deprecated - for soon-to-be removed features
• Removed - for now removed features
• Fixed - for any bug fixes
• Security - in case of vulnerabilities

Message

[Copilot]

Pull request overview

This PR adds defensive checks to prevent fatal errors when the WordPress filesystem (WP_Filesystem) isn't available, particularly on hosts using FTP-based filesystem configurations. The changes ensure graceful failure handling instead of fatal errors when filesystem operations cannot be performed.

• Adds checks for WP_Filesystem() initialization success before using the global $wp_filesystem object
• Returns appropriate error messages or silently fails for delete operations when filesystem is unavailable
• Ensures proper cleanup of temporary files even when filesystem initialization fails

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
includes/class-attachments.php	Added WP_Filesystem initialization checks in four methods: delete_ap_posts_directory, save_attachment, save_file, and delete_actors_directory to prevent fatal errors when filesystem operations fail
.github/changelog/fix-wp-filesystem-null-check	Added changelog entry documenting the patch-level fix for WP_Filesystem initialization checks

Comments suppressed due to low confidence (4)

includes/class-attachments.php:80

• The second check for $wp_filesystem being falsy is redundant. If WP_Filesystem() returns true (passes the first check on line 72), then $wp_filesystem global will be properly initialized. This redundant check adds unnecessary complexity without providing additional safety.

		if ( $wp_filesystem->is_dir( $activitypub_dir ) ) {
			$wp_filesystem->delete( $activitypub_dir, true );

includes/class-attachments.php:473

• The second check for $wp_filesystem being falsy is redundant. If WP_Filesystem() returns true (passes the first check on line 465), then $wp_filesystem global will be properly initialized. This redundant check adds unnecessary complexity without providing additional safety.

				/* translators: %s: file path */
				return new \WP_Error( 'file_not_found', sprintf( \__( 'File not found: %s', 'activitypub' ), $attachment_data['url'] ) );
			}

includes/class-attachments.php:616

• The second check for $wp_filesystem being falsy is redundant. If WP_Filesystem() returns true (passes the first check on line 606), then $wp_filesystem global will be properly initialized. This redundant check adds unnecessary complexity without providing additional safety.

		}

		// Move file to destination.
		if ( ! $wp_filesystem->move( $tmp_file, $file_path, true ) ) {

includes/class-attachments.php:1068

• The second check for $wp_filesystem being falsy is redundant. If WP_Filesystem() returns true (passes the first check on line 1060), then $wp_filesystem global will be properly initialized. This redundant check adds unnecessary complexity without providing additional safety.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pfefferle]

Is this still working, because it now inits the filesystem after setting the global!?