Make WordPress Core

Opened 4 years ago

Last modified 2 months ago

#48689 assigned defect (bug)

PHP warnings after updating to WP 5.3: ftp_nlist() and ftp_pwd() expect missing parameters

Reported by: hinjiriyo's profile Hinjiriyo Owned by: adamsilverstein's profile adamsilverstein
Milestone: Future Release Priority: normal
Severity: minor Version: 5.3
Component: Filesystem API Keywords: php80 needs-unit-tests has-patch needs-testing 2nd-opinion
Focuses: Cc:

Description

I updated several websites to WP 5.3 without any problems. But on one wesite I got these PHP warnings both in the backend and in the website:

Warning: ftp_nlist() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 402
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226
Warning: ftp_nlist() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 402
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 681
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226
Warning: ftp_pwd() expects parameter 1 to be resource, null given in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 226

Warning: Cannot modify header information - headers already sent by (output started at /wp-admin/includes/class-wp-filesystem-ftpext.php:402) in /wp-includes/functions.php on line 5946
Warning: Cannot modify header information - headers already sent by (output started at /wp-admin/includes/class-wp-filesystem-ftpext.php:402) in /wp-admin/includes/misc.php on line 1252
Warning: Cannot modify header information - headers already sent by (output started at /wp-admin/includes/class-wp-filesystem-ftpext.php:402) in /wp-admin/admin-header.php on line 9

I suppressed the ouput by "muting" the function calls with '@' in the file 'class-wp-filesystem-ftpext.php', i.e. changed ftp_nlist() to @ftp_nlist() etc..

I will be glad if any reason can be found and fixed until the next WP upgrade.

Attachments (1)

48689.diff (1.5 KB) - added by adamsilverstein 18 months ago.

Download all attachments as: .zip

Change History (25)

#1 follow-up: @SergeyBiryukov
4 years ago

Hi there, thanks for the report!

Related: [45611/trunk/src/wp-admin/includes/class-wp-filesystem-ftpext.php], which removed error suppression for ftp_nlist() and ftp_pwd() here. It's likely that the issue comes from somewhere else, but is now more visible.

I could not reproduce it on a clean install though, some more details about the environment and the steps to reproduce would be helpful.

#2 @Hinjiriyo
4 years ago

Due to the many websites on many servers I maintain (~15), it is difficult to see anything special about the respective WP installation. The debugging is further complicated by the fact that nowhere in this website the class WP_Filesystem_FTPext seems to be called and used.

#3 @Hinjiriyo
4 years ago

Additional comment: if a plugin is upgraded this text appears on the "Update Plugins" page between "Enabling Maintenance mode..." and "Updating Plugin {x} ({m}/{n})":

Warning: ftp_rmdir(): /.maintenance: File or directory not found in /wp-admin/includes/class-wp-filesystem-ftpext.php on line 381

#4 @Hinjiriyo
3 years ago

  • Severity changed from critical to minor

I could detect the plugin which was the reason of this error. So I would close this ticket. But I think "muting" more PHP file function calls with '@' in the file 'class-wp-filesystem-ftpext.php' makes still sense, since many other function calls in this file are set as muted.

#5 follow-up: @SirLouen
3 years ago

I've found who makes this happen: WPML Multilingual CMS in combination with RankMath.

Last edited 3 years ago by SirLouen (previous) (diff)

#6 in reply to: ↑ 5 @Hinjiriyo
3 years ago

Replying to SirLouen:

I've found who makes this happen: WPML Multilingual CMS in combination with RankMath.

In the case of my website it was the plugin "Amazon Associates Link Builder". Since I deactivated it, there are no error messages. Was it a combination with another plugin? I don't know and didn't research that.

#7 @SirLouen
3 years ago

I have inyected a little bit of extra verbosity through some php error logging function to discover that Rankmath was trying to access to certain files (.htaccess and robots.txt) without success. I'm not sure why WPML (also happened by activating AMP for WP plugin in combination with Rankmath) was hindering Rankmath from this access...

The function that was being hindered is exists from WP Filesystem.
So probably any plugin in combination with WPML or AMP for WP may have this issue?
https://developer.wordpress.org/reference/classes/wp_filesystem_base/exists/
Not 100% sure.

This is the rankmath snippet which was making this misbehaviour.

<?php
public static function get_htaccess_data() {
                $wp_filesystem = WordPress::get_filesystem();
                $htaccess_file = get_home_path() . '.htaccess';

                return ! $wp_filesystem->exists( $htaccess_file ) ? false : [
                        'content'  => $wp_filesystem->get_contents( $htaccess_file ),
                        'writable' => $wp_filesystem->is_writable( $htaccess_file ),
                ];
        }

I'm not a code maintainer of Rankmath. In fact I have uninstalled from a production site, and left it in a staging server. But just wanted to inform about this because it could be useful for WP maintainers (I'm not sure).

Last edited 3 years ago by SirLouen (previous) (diff)

#8 @erikgeurtsplatformiqcom
3 years ago

This changeset https://core.trac.wordpress.org/changeset/45611/trunk/src/wp-admin/includes/class-wp-filesystem-ftpext.php (for example in line 226) unearths these PHP warnings by removing the @ symbol. It means the warning is no longer suppressed, but it's been an issue before this change.

In my case, it was also the addition of a plugin, but not the same as the previous comments, that brought out the warnings. The cause is not in these plugins directly, but in the fact that certain plugins in certain environments make an attempt to use the WP filesystem in a way that apparently is not expected.

The actual issue in https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-filesystem-ftpext.php?rev=45611 since that change in July 2019 is that $this->link doesn't get a value assigned first, so any attempt to use the variable in this code throws the warning.

It seems that the if statement in https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-filesystem-ftpext.php?rev=45611#L86 is where things go wrong.

In my case, the problem may come from the fact that I'm used to defining the FTP hostname, user and password in the wp-config.php, yet doing this incorrectly or with something missing. So in fact this may be a documentation issue (i.e. not having followed the documentation to the letter).

Where can the documentation for defining the FTP credentials in the wp-config.php be found?

#9 @aiko2011
3 years ago

Проблемный плагин у меня Pirateforms, последнее обновление WP его не поддерживает ;)

Last edited 3 years ago by aiko2011 (previous) (diff)

#10 in reply to: ↑ 1 @aiko2011
3 years ago

Replying to SergeyBiryukov:

Hi there, thanks for the report!

Related: [45611/trunk/src/wp-admin/includes/class-wp-filesystem-ftpext.php], which removed error suppression for ftp_nlist() and ftp_pwd() here. It's likely that the issue comes from somewhere else, but is now more visible.

I could not reproduce it on a clean install though, some more details about the environment and the steps to reproduce would be helpful.

Привет!
Были такие же проблемы.
Проблемный плагин у меня Pirateforms, последнее обновление WP его не поддерживает ;)

#11 follow-up: @orenwolf
3 years ago

It appears that this problem occurs whenever a plugin tries to access a file, but doesn't have write access, and tries instead to use FTP to access it.

#12 in reply to: ↑ 11 @aiko2011
3 years ago

Replying to orenwolf:

It appears that this problem occurs whenever a plugin tries to access a file, but doesn't have write access, and tries instead to use FTP to access it.

https://ru.wordpress.org/support/topic/pirate-forms/

#13 @BearlyDoug
3 years ago

I also was receiving this... I thought it was tied to the WP Astra theme (https://wpastra.com/), since I'm not getting it on sites not running the WP Astra Theme and WP Astra Pro plugin.

The first result I found for this is located HERE

I added

if (!defined('FS_METHOD')) define('FS_METHOD', 'direct');

to my wp-config.php file, and now the error messages actually have stopped.

This issue didn't show up until WordPress 5.3 was released, and is KILLING my Apache log files.

#14 @SergeyBiryukov
3 years ago

#51283 was marked as a duplicate.

#15 @absotively
2 years ago

I see similar warnings, though I've tracked it to a different plugin.

At least on my site, the warnings appear on the site even if WP_DEBUG is set to false.

Edit: Looking more closely at the plugin in question, I realize this may be due to it calling the relevant functions too early, so it may not be relevant to this bug after all.

Last edited 2 years ago by absotively (previous) (diff)

#16 @SirLouen
2 years ago

  • Keywords close added
  • Resolution set to invalid
  • Status changed from new to closed

@BearlyDoug this problem happens when it (a plugin most of the time) tries to use an FTP-based FS_METHOD like ftpext wrongly. Considering that you switched to "direct" then, this 2 functions ftp_nlist and ftp_pwd will not be required more.

Some plugins although, disregard the FS_METHOD and they force using an FTP therefore this messages could appear.

I'm still not 100% sure, but according to my research, operating with FTPext in WP changed some time ago, not sure after which patch, maybe 5.2, 5.3, 5.4 but some plugins have still failed to adapt to the new mechanism.

For example, MainWP did not patch the FTPext method, so now they force using direct to all their clients. If you set ftpext for FS_METHOD you will see these errors.

I don't think this is a bug from WordPress, but a bug from plugin authors so we might close this.

#17 @desrosj
2 years ago

  • Keywords close removed
  • Milestone Awaiting Review deleted

#18 @adamsilverstein
18 months ago

  • Keywords php8 added
  • Resolution invalid deleted
  • Status changed from closed to reopened

I don't think this is purely plugin related. I can reproduce this on my local when visiting the Site Health screen, with no plugins active.

In my case, the web server user is not the owner of the site files and thus WordPress will attempt to use ftp if I try a plugin update for example and apparently on the Site Health screen as well. Fixing the file ownership issue made the problem go away for me.

Worth noting this is a fatal in PHP 8+.

This description seems to identify the underlying issue.

#19 @adamsilverstein
18 months ago

  • Owner set to adamsilverstein
  • Status changed from reopened to assigned

#20 @adamsilverstein
18 months ago

The issue arrises because we pass the file link to several ftp functions without first validating it. In certain conditions (for example, when WordPress lacks the correct file permissions), the file link is null.

Passing null to ftp_pwd, ftp_chdir and ftp_nlist throws a warning in PHP 7.4 and a fatal in PHP 8+. to avoid this I've added some checks to the ftpext functions that were throwing the errors for me. After this patch I can load the Site Health page with PHP 8.

The patch is a POC:

  • not sure this is the best way to fix the issue, didn't look for potential issues with other related classes (eg. WP_Filesystem_ftpsockets)
  • needs tests that validate whatever fix we choose.

#21 @desrosj
11 months ago

  • Keywords needs-unit-tests has-patch needs-testing 2nd-opinion added; needs-patch removed
  • Milestone set to Future Release

Re-adding a milestone for this one.

#22 @ThemeZee
8 months ago

This issue also seems to happen by using the WPTT/webfont-loader packing in some server environments:
https://make.wordpress.org/themes/2020/09/25/new-package-to-allow-locally-hosting-webfonts/

#23 @sc0ttkclark
6 months ago

The patch makes a lot of sense here but it doesn't cover ssh2 which also makes use of $this->link and would exhibit the same type of issue as it uses $this->link in functions that expect a resource.

The WP_Filesystem_ftpsockets class is safe because uses ftp_sockets or ftp_pure classes (depending on sockets use) which already checks to see if the ftp resource is connected before calling the functions that would cause errors like this.

To move forward:

  • Add checks in WP_Filesystem_SSH2 that check $this->link before passing it to a function expecting a resource
  • Add tests for ftpext and ssh2 in cases where a connection could not be made, confirming the methods return as expected (with no errors)

#24 @hellofromTonya
2 months ago

  • Keywords php80 added; php8 removed
Note: See TracTickets for help on using tickets.