Make WordPress Core

#58940 closed defect (bug) (fixed)

site-health.php page Fatal error on version: 6.3-RC2

Reported by: utsav72640's profile utsav72640 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: has-patch
Focuses: administration Cc:

Description

When I navigate to "wp-admin/site-health.php", I encounter an error indicating that the Fatal error: Uncaught TypeError: ftp_nlist(): Argument #1 ($ftp) must be of type resource, null given in /Applications/XAMPP/xamppfiles/htdocs/Releasecandidate/wp-admin/includes/class-wp-filesystem-ftpext.php:438

I have attached both screenshot, Can you please check and let me know if there are any question or issue.

Attachments (5)

Screenshot 2023-07-31 at 11.07.49 AM.png (228.7 KB) - added by utsav72640 11 months ago.
Screenshot 2023-07-31 at 11.08.11 AM.png (277.0 KB) - added by utsav72640 11 months ago.
Capture d’écran 2023-07-31 à 11.18.42.png (451.7 KB) - added by audrasjb 11 months ago.
58940.diff (1.7 KB) - added by SergeyBiryukov 11 months ago.
58940.2.diff (1.7 KB) - added by SergeyBiryukov 11 months ago.

Download all attachments as: .zip

Change History (40)

#1 @audrasjb
11 months ago

  • Milestone changed from Awaiting Review to 6.3

#2 @audrasjb
11 months ago

Hello and thanks for the report @utsav72640!

I can't reproduce the issue on my side. Does it occur on a fresh install?

#3 @utsav72640
11 months ago

Hello @audrasjb

Yes i have fresh installed WP. Now can you please check attached both screenshot because sometimes PHP compatibility issue will be there.

#4 @utsav72640
11 months ago

Hello @audrasjb

I have shared one video URL, Can you please check because may be some help for you

https://www.awesomescreenshot.com/video/19552473?key=8a8323cb1dd560e79a8cea08892089ab

#5 follow-up: @audrasjb
11 months ago

Ah, it appears I'm able to reproduce the issue when using define( 'FS_METHOD', 'ftpext' ); in my wp-config file.

Could you please check you wp-config file to confirm you have this declaration?

#6 @audrasjb
11 months ago

Probably related: #33058 / [55556].

What do you think @SergeyBiryukov?

#7 in reply to: ↑ 5 @utsav72640
11 months ago

Hello @audrasjb
Not found define( 'FS_METHOD', 'ftpext' );
Can you please check this video URL, https://www.awesomescreenshot.com/video/19553737?key=b8b8480d1bb3092f22be748bd259af51

Replying to audrasjb:

Ah, it appears I'm able to reproduce the issue when using define( 'FS_METHOD', 'ftpext' ); in my wp-config file.

Could you please check you wp-config file to confirm you have this declaration?

#8 follow-up: @audrasjb
11 months ago

I saw your video, but I can't reproduce the issue without defining FS_METHOD to ftpext.

#9 in reply to: ↑ 8 @utsav72640
11 months ago

Hello @audrasjb
I have try with this but still not working for me. Check this video URL, https://www.awesomescreenshot.com/video/19554350?key=85463301426364789953521297b357f9

Replying to audrasjb:

I saw your video, but I can't reproduce the issue without defining FS_METHOD to ftpext.

This ticket was mentioned in PR #4937 on WordPress/wordpress-develop by @rajinsharwar.


11 months ago
#10

  • Keywords has-patch added; needs-refresh removed

Update class-wp-filesystem-ftpext.php to check FTP connection issues.

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

#11 @rajinsharwar
11 months ago

  • Keywords needs-dev-note added; needs-testing removed

As far as I understand, this constant might not support all servers' ftp out there. So, what we can do is check if we have null $this->link, each time we call it. Also, in connect function, we can check if the hostname, username or password is empty or not, and return false, (Because we can't make a connection without any of these). Currently, we are only adding the errors in the error class if the hostname, username, or password is empty. PR added, any suggestions are greatly appreciated.

#12 @utsav72640
11 months ago

Thanks for the PR @rajinsharwar

Now can you please check once from you end, @audrasjb & @SergeyBiryukov

#13 @costdev
11 months ago

  • Keywords reporter-feedback added; needs-dev-note removed
  • Version trunk deleted

@utsav72640 If you aren't intentionally using an FTP filesystem method, you'll likely also be unable to upload media or install/update plugins and themes without being prompted for a username, password and host. This issue is almost always due to file ownership and permissions issues.

For example, in my Ubuntu / Apache setup, I often have to run chown -R www-data:www-data wordpress-directory after downloading and extracting a new copy of WordPress, and sometimes after upgrading via CLI / checking out certain Git branches.

First, try defining FS_METHOD as direct:

define( 'FS_METHOD', 'direct' );

If that doesn't resolve the issue, check which user owns the WordPress directory, files and sub-directories. It's likely to be the incorrect user.

Let us know if the above helps resolve the issue 🙂


  • Removing needs-dev-note, as this isn't a verified bug in Core, and bugs would only really get dev notes if they are actually intentional BC breaks.
  • There's no indication that this is a bug introduced in trunk/6.3 - removing trunk as the Version property. [55556] is related to ::exists() return values, whereas the warnings/fatal error are related to the FTP filesystem connection not being initialized. This makes sense given that the user had no intention of using FTP, hence my suggestions above to use the direct filesystem method and check ownership/permissions.
Last edited 11 months ago by costdev (previous) (diff)

#14 @utsav72640
11 months ago

Hello @costdev

I am simply setup 6.3-RC2 WP in my local system. And also i have given 777 permission to my whole folder in htdocs so here no any issue with FTP points, we just inform you we have installed fresh 6.3-RC2 install in our local system and when When I navigate to "wp-admin/site-health.php", I encounter an error indicating that the Fatal error. Attached to this message are screenshots and a video URL demonstrating the issue. I would greatly appreciate any assistance in resolving this problem. Thank you.

#15 @SergeyBiryukov
11 months ago

Hi there, thanks for the ticket!

I was able to reproduce the issue with define( 'FS_METHOD', 'ftpext' ).

As noted above, [55556] / #33058 does not appear to be related, however the error comes from the Site Health test added in [55720] / #51857, so this was indeed introduced in 6.3.

I don't think the WP_Filesystem_FTPext class should be patched here, I believe this is an issue with the test itself, specifically that we don't check the result of the WP_Filesystem() call, or whether the $wp_filesystem object is ready for use and does not have any errors. In my case, the issue was caused by missing credentials.

Looking closer, _wp_delete_all_temp_backups() appears to have the same issue.

58940.diff attemps to use the stored credentials if available, and displays an error message otherwise.

#16 follow-up: @rajinsharwar
11 months ago

The patch looks good to me @SergeyBiryukov. But, I am not quite sure why I am able to reproduce the fatal error in WP 6.2.2 if the Site Health test was introduced in 6.3. Might be something that can be confirmed if others face the same issue too in WP 6.2.2

#17 in reply to: ↑ 16 ; follow-up: @SergeyBiryukov
11 months ago

Replying to rajinsharwar:

I am not quite sure why I am able to reproduce the fatal error in WP 6.2.2 if the Site Health test was introduced in 6.3.

Thanks for testing! Could you share the call stack for the error in WP 6.2.2? Is the error message the same?

I could only reproduce the error in WP 6.3 so far.

#18 @costdev
11 months ago

@utsav72640 I am simply setup 6.3-RC2 WP in my local system. And also i have given 777 permission to my whole folder in htdocs so here no any issue with FTP points, we just inform you we have installed fresh 6.3-RC2 install in our local system and when When I navigate to "wp-admin/site-health.php", I encounter an error indicating that the Fatal error. Attached to this message are screenshots and a video URL demonstrating the issue. I would greatly appreciate any assistance in resolving this problem. Thank you.

In case it seemed otherwise, I'd like to confirm that I read all of this ticket's information carefully before I posted my first comment.

When WordPress can't access files, it tries to use FTP. It will either use stored credentials, or ask you for credentials. This is what Sergey is patching.

However, that is not what's causing your problem. On your system, WordPress should not be trying to use FTP. It only tries to do this when the FS_METHOD is not set to direct, and when the owner OR permissions are incorrect. Please note that WordPress checks both, so 777 on htdocs isn't enough.

Last edited 11 months ago by costdev (previous) (diff)

#19 @mukesh27
11 months ago

You're welcome, @utsav72640, for raising the ticket, and thanks to @SergeyBiryukov for providing the patch.

Regarding the error, was it introduced in the 6.3 cycle? If not, we can consider moving this ticket to the next milestone, as in my opinion, there won't be a need to update anything in the Release Candidates (RCs). What do you think?

#20 in reply to: ↑ 17 @rajinsharwar
11 months ago

Replying to SergeyBiryukov:

Replying to rajinsharwar:

I am not quite sure why I am able to reproduce the fatal error in WP 6.2.2 if the Site Health test was introduced in 6.3.

Thanks for testing! Could you share the call stack for the error in WP 6.2.2? Is the error message the same?

I could only reproduce the error in WP 6.3 so far.

Ops, my bad @SergeyBiryukov. The issue doesn't seem to replicate in 6.2.2 (My branch was set to 6.3 RC2 lol). @mukesh27 I guess it's confirmed that this issue was introduced in 6.3.

#21 @rajinsharwar
11 months ago

I suggest updating the output message to something like "Unable to connect to the filesystem. Please confirm your credentials or make sure you have defined the correct FS Method in the wp-config.php file". This would give the admin a much clearer idea of what may have gone wrong.

#22 @SergeyBiryukov
11 months ago

The current string ("Unable to connect to the filesystem. Please confirm your credentials.") is used in a few other places in core, so we should ideally be consistent here, especially now that 6.3 is in string freeze, where we generally avoid introducing new strings unless absolutely necessary (e.g. to fix a typo). Perhaps the string should be clarified for all instances in a future release?

Last edited 11 months ago by SergeyBiryukov (previous) (diff)

#23 follow-up: @peterwilsoncc
11 months ago

58940.diff fixes the error on the site health page (a warning in my case because I am using an earlier version of PHP).

However, I don't think a critical error is quite correct from a Site Health perspective. Including define( 'FS_METHOD', 'ftpext' ); without the credentials doesn't prevent plugins from being updated: during the update there is a prompt to enter the credentials. Updating works provided they are correct.

A site owner may do that intentionally to avoid storing credentials in plain text on the server.

#24 in reply to: ↑ 23 ; follow-up: @SergeyBiryukov
11 months ago

Replying to peterwilsoncc:

However, I don't think a critical error is quite correct from a Site Health perspective. Including define( 'FS_METHOD', 'ftpext' ); without the credentials doesn't prevent plugins from being updated: during the update there is a prompt to enter the credentials. Updating works provided they are correct.

Good point, should we reduce the severity to recommended? I'm not sure if the test can be skipped entirely if there are no stored credentials, but that might be another option to look into.

#25 in reply to: ↑ 24 @peterwilsoncc
11 months ago

Replying to SergeyBiryukov:

should we reduce the severity to recommended? I'm not sure if the test can be skipped entirely if there are no stored credentials, but that might be another option to look into.

If no credentials are stored another critical error is already displayed:

Background updates are not working as expected

...[snip]...

❌ Your installation of WordPress prompts for FTP credentials to perform updates. (Your site is performing updates over FTP due to file ownership. Talk to your hosting company.)

It might be best to revert the new test for now and considered it covered already. If needs be, the new test can be rethought during the 6.4 release cycle to ensure it runs error free (in the PHP sense).

#26 follow-up: @SergeyBiryukov
11 months ago

The new Site Health test checks that all of the upgrade directories are writable, which does not appear to be covered by any other test. Since rollbacks were introduced in this release, I think the test is important enough to keep in 6.3 as well to help diagnose any potential issues. I'm also concerned about a similar issue in _wp_delete_all_temp_backups().

I think I would suggest 58940.2.diff for 6.3 consideration, it lowers the severity to recommended, and we can perhaps fine-tune the test further in 6.3.1 or 6.4.

#27 in reply to: ↑ 26 @audrasjb
11 months ago

Replying to SergeyBiryukov:

I think I would suggest 58940.2.diff for 6.3 consideration, it lowers the severity to recommended, and we can perhaps fine-tune the test further in 6.3.1 or 6.4.

I support this suggestion. Let's lower the criticity for now, and improve the check in the next minor or in 6.4 👍

This ticket was mentioned in Slack in #core by flixos90. View the logs.


11 months ago

#29 @SergeyBiryukov
11 months ago

In 56341:

Upgrade/Install: Pass stored credentials to WP_Filesystem() where appropriate.

With the introduction of temporary backups of plugins and themes before updating, a new Site Health test was added to verify that plugin and theme temporary backup directories are writable or can be created.

When using a non-direct filesystem, the Site Health test did not include the required credentials, leading to a fatal error as the connection was not initialized properly.

This commit attemps to use the stored credentials if available, and displays a message otherwise.

Includes a similar fix in a function that performs a cleanup of the temporary backup directory.

Follow-up to [55720].

Props utsav72640, rajinsharwar, costdev, mukesh27, peterwilsoncc, audrasjb, SergeyBiryukov.
See #58940.

#30 @SergeyBiryukov
11 months ago

In 56342:

Upgrade/Install: Pass stored credentials to WP_Filesystem() where appropriate.

With the introduction of temporary backups of plugins and themes before updating, a new Site Health test was added to verify that plugin and theme temporary backup directories are writable or can be created.

When using a non-direct filesystem, the Site Health test did not include the required credentials, leading to a fatal error as the connection was not initialized properly.

This commit attemps to use the stored credentials if available, and displays a message otherwise.

Includes a similar fix in a function that performs a cleanup of the temporary backup directory.

Follow-up to [55720].

Props utsav72640, rajinsharwar, costdev, mukesh27, peterwilsoncc, audrasjb, SergeyBiryukov.
Reviewed by audrasjb, SergeyBiryukov.
Merges [56341] to the 6.3 branch.
See #58940.

#31 @SergeyBiryukov
11 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core-test by utsav72640. View the logs.


11 months ago

@SergeyBiryukov commented on PR #4937:


11 months ago
#33

Thanks for the PR! This should be resolved in r56341.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


11 months ago

#35 @SergeyBiryukov
11 months ago

  • Keywords reporter-feedback removed
  • Resolution set to fixed
  • Status changed from accepted to closed

I think this is resolved for 6.3, feel free to create a new ticket if the Site Health test for the upgrade directories can be improved further. Thanks everyone!

Note: See TracTickets for help on using tickets.