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

[Hinjiriyo]

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.

[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.

[Hinjiriyo]

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.

[Hinjiriyo]

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

[Hinjiriyo]

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.

[SirLouen]

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

[Hinjiriyo]

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.

[SirLouen]

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).

[erikgeurtsplatformiqcom]

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?

[aiko2011]

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

[aiko2011]

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 его не поддерживает ;)

[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.

[aiko2011]

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/

[BearlyDoug]

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.

[SergeyBiryukov]

#51283 was marked as a duplicate.

[absotively]

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.

[SirLouen]

@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.

[adamsilverstein]

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.

[adamsilverstein]

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.

[desrosj]

Re-adding a milestone for this one.

[ThemeZee]

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/

[sc0ttkclark]

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)

[hellofromTonya]

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.

48689.diff​ adds property validation to silently bail out early. But the $link property is documented to always be a resource. This ticket identifies scenarios where the file link may fail.

I'm thinking about:

• Should the file link property be documented as resource|null? Is null a valid value?
• Where these "certain conditions" first happen, is there error logging to alert developers why it failed?
• Should a failure stop the progression of the work in the filesystem, meaning the code should not get to the methods where the warning / fatal are thrown?
• If null is a valid value for the file link property, then protecting the methods that act upon it makes sense if those methods are reachable in the call stack.

Silently bailing out without an early error can make debugging harder while potentially leaving users and developers wondering why their action failed.

I'm marking this ticket as a PHP 8.0 php-compatibility issue and pulling it into the 6.4 cycle to hopefully move it forward.

[SergeyBiryukov]

Replying to hellofromTonya:

Should the file link property be documented as resource|null? Is null a valid value?

I think not, as pretty much all methods of this class operate under an assumption that the connection was already initialized in WP_Filesystem() via $wp_filesystem->connect().

The only case where $this->link is null is when the connection was for some reason not initialized properly, and that could use some further investigation to determine the cause. It could be missing or incorrect credentials, like in #58940. So I think resource is the correct type here, and null is not a valid value.

The same applies to the WP_Filesystem_SSH2 class too.

[nicolefurlan]

During our bug scrub today we all discussed what @hellofromTonya mentioned about how silently bailing out early is not ideal. @costdev is going to take a closer look at this!

[nicolefurlan]

This ticket was discussed in today's bug scrub.

This ticket is getting pushed to 6.5. There was some concern about doing this work late in the cycle without tests or testing.

@hellofromTonya mentioned:

A full suite of tests are coming for the Filesystem. That'll help.

Props to @hellofromTonya and @costdev :)

[chaion07]

Thanks @Hinjiriyo for reporting this. We reviewed this ticket during a recent bug-scrub and based on the feedback we are updating the milestone to 6.6 since @costdev didn't have adequate time to review this again.

Cheers!

Prop to @mukesh27

[oglekler]

WP_Filesystem_SSH2 has

		if ( ! $this->link ) {
			return false;
		}

in one place. Possibly way forward is to add function that generate error and bail like this:

		if ( ! $this->link ) {
			$this->error_missing_link( 'SSH2' );
			return false;
		}

....
	private function error_missing_link( $connection_name ) {
		$this->errors->add(
			'connection_name',
			sprintf(
				/* translators: %s: Connection type. */
				__( 'Missing connection: %s' ),
				$connection_name
			)
		);
	}

This can be done for WP_Filesystem and for WP_Filesystem_SSH2 classes.

[hellofromTonya]

With the last scheduled beta happening in a couple of hours, moving this one to 6.7. However, if a patch is ready and there's consensus for commit, then please feel free to pull it back into 6.6 milestone.

[chaion07]

Thanks @Hinjiriyo for reporting this. As we move closer to the RC1 for 6.7 in a matter of days and with no progress in this cycle, we are updating the milestone based on the feedback received from a recent bug-scrub session. Thanks.

Props to @peterwilsoncc

Cheers!

[johnbillion]

Silently bailing out without an early error can make debugging harder while potentially leaving users and developers wondering why their action failed.

This is my main concern with the suggested patch on this ticket. If there is a default scenario where these methods can be called prior to a valid resource link being created, then that should be investigated and fixed rather than silently returning false from the affected methods.

This isn't a fun feature to debug, but papering over the issue will just make the problem harder to debug. Adam's comment here provides info about when this filesystem handler gets called.

[christianmeier]

I believe this bug can be fixed for PHP 8.1 and newer versions, while keeping compatibility with older PHP versions, by changing $link's PHPDoc type declaration to this:

<?php
        /**
         * @since 2.5.0
         * @var FTP\Connection|resource|bool
         */
        public $link;

That way the connect() method wouldn't break when either ftp_ssl_connect or ftp_connect initialize $link.

​https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-wp-filesystem-ftpext.php

[SirLouen]

@johnbillion reply:

This is my main concern with the suggested patch on this ticket. If there is a default scenario where these methods can be called prior to a valid resource link being created, then that should be investigated and fixed rather than silently returning false from the affected methods.

After 5 years, are there any current steps of reproduction for the current problematic?

It's interesting that I actually wrote back in the day, FTPext has been always my bane. This filesystem is problematic and not only needs a clear revamp, but also a full testing suite with mocking functions to prove everything right and hunt for more edge cases that are triggering FTP errors (specially because a lot of functions are using error control operator unnecessarily (and not recommended in WordPress CS).

Here are my 4 tickets regarding FTP #63173 #63172 #63197 #63201
I've tried to solve some of these issues but if I could have some reproduction steps for the current situation, I could try to check if my patches are solving this (specially #63173 and #63197 that solve two major issues).

Also the error_missing_link idea is the way to go here.

@christianmeier reply:

I believe this bug can be fixed for PHP 8.1 and newer versions, while keeping compatibility with older PHP versions, by changing $link's PHPDoc type declaration to this

Particularly, this one #63201 touches that. It will not solve anything error-wise, but it's very useful to remove all those warnings in the code.