Skip to content

Upgrade/Install: Do not reuse an unconnected filesystem in WP_Upgrader::maintenance_mode(). - #13257

Open
jigneshbhavani wants to merge 1 commit into
WordPress:trunkfrom
jigneshbhavani:fix/65942-maintenance-mode-unconnected-filesystem
Open

Upgrade/Install: Do not reuse an unconnected filesystem in WP_Upgrader::maintenance_mode().#13257
jigneshbhavani wants to merge 1 commit into
WordPress:trunkfrom
jigneshbhavani:fix/65942-maintenance-mode-unconnected-filesystem

Conversation

@jigneshbhavani

Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/65942

maintenance_mode() guards initialization with ! $wp_filesystem, which treats any object in the global as usable. WP_Filesystem() assigns that global at file.php:2213 before connecting and returns false at :2226 on a constructor error, so a failed call earlier in the request leaves an unconnected object behind. maintenance_mode() then calls abspath() on it, and under PHP 8 the transport fatals on a null link. It is called once per WP_Automatic_Updater::update(), so the whole update run aborts rather than the one item.

This is the branch [60107] did not cover on the same function, and the check mirrors fs_connect() in this class.

Design decision worth flagging: the guard treats any recorded error as unusable. WP_Filesystem_Base::$errors is set in the constructor and never cleared, and WP_Filesystem_FTPext::connect() deliberately tolerates connect and auth codes so a failed attempt can be retried on the same instance. An instance that failed once and then connected successfully still carries those codes, so this guard will discard it and reinitialize. That costs a reconnect in a rare case, and I preferred it to inspecting transport-specific link properties from the caller. fs_connect() is stricter still and hard-fails on the same condition.

Testing

Set FS_METHOD to ftpext, call WP_Filesystem() once with incomplete credentials, then run maintenance_mode( true ). Before the change that fatals in the transport; after it, the existing "Could not access filesystem." notice path from [60107] handles it.

…er::maintenance_mode()`.

A failed `WP_Filesystem()` call leaves an unconnected object in the global, which the
`! $wp_filesystem` guard accepted. Check for recorded errors as `fs_connect()` does.

Props bejignesh, hatterops.
See #65942.
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @hatterops.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props bejignesh.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@hatterops

Copy link
Copy Markdown

Thanks for picking it up and submitting a patch. Wanted to add some diagnostic detail from the investigation.

the crashing $wp_filesystem object in the system had:

is_object( $wp_filesystem ) → true
$wp_filesystem->errors->has_errors() → '''false''' (zero recorded errors — confirmed via print_r( $ftp->errors->errors, true ) on a freshly-constructed instance using the exact same credentials, which came back empty)
$wp_filesystem->link → '''undefined''' (not false, not a resource — genuinely never set), confirmed via var_export( $wp_filesystem->link ?? 'no-link-property', true ) at the moment of the real crash

Ruled out bad/missing FTP credentials specifically (valid FTP_HOST/FTP_USER/FTP_PASS, clean manual FTP login, OK LOGIN in vsftpd's log immediately before the crash) before landing on this. So in the case, connect() genuinely appears to have never been called at all on the object sitting in the global — not called-and-failed (which would leave errors populated, or link as false), just never invoked.

Given that, the proposed guard:
{{{#!php
if ( ! is_object( $wp_filesystem )
|| ( is_wp_error( $wp_filesystem->errors ) && $wp_filesystem->errors->has_errors() )
) { ... }
}}}
would evaluate ! true || false → false for the object I observed, so maintenance_mode() would still reuse it and still fatal the same way. This patch looks correct and worth having for the case it targets (a constructor that recorded a real error — bad config, missing extension, etc.), but it looks like a different failure mode from the one in the reported ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants