WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 18 months ago

#28013 reopened defect (bug)

WP_Filesystem_FTPext::exists($filename) returns false when file exists on FTP Service ( IIS/6.0 Microsoft & Linux/vsftp )

Reported by: joostdekeijzer Owned by: dd32
Milestone: Future Release Priority: normal
Severity: normal Version: 3.9.2
Component: Filesystem API Keywords:
Focuses: administration Cc:

Description

I'm having trouble using the update/upgrade functions of WordPress on a Windows IIS/6.0 server.

The .maintenance file is never deleted.

I have to use the FTPext method of updating and I've found that WP_Filesystem_FTPext::exists('.maintenance') returns false even though the file exists.

Since the fix for #10060 that function uses ftp_nlist() which returns invalid results on the IIS/6.0 Microsoft FTP Service.

I did some more testing and it turns out that the current function has issues with "dotfiles"

Work correctly
WP_Filesystem_FTPext::exists('index.php')
WP_Filesystem_FTPext::exists('wp-content/debug.log')

Fail
WP_Filesystem_FTPext::exists('.maintenance')
WP_Filesystem_FTPext::exists('wp-content/.maintenance')

(yes, I made sure those files actually existed ;-)

For this particular purpose, couldend WP_Upgrader::maintenance_mode() be modified to test for the existence of .maintenance using is_file( ABSPATH . $file ) and if so execute $wp_filesystem->delete($file, false, 'f'); (setting the f bypases all the tests)?

Attachments (2)

28013.diff (916 bytes) - added by joostdekeijzer 5 years ago.
test for .maintenance using php is_file()
wp.maintenance.patch (760 bytes) - added by hnhn2 18 months ago.
treat .maintenance file deletion as file DELETE instead of RMDIR

Download all attachments as: .zip

Change History (22)

@joostdekeijzer
5 years ago

test for .maintenance using php is_file()

#1 @joostdekeijzer
5 years ago

  • Keywords has-patch added

Added patch to use is_file() to check if there is a .maintenance file.

Tested on IIS/6.0 with FTP and works fine.
Tested on Apache (MacPorts) with "Direct access" (not using ftp) and works fine.

#2 @tellyworth
5 years ago

  • Version changed from trunk to 3.9.2

This applies to 3.9 at least, probably older versions too.

#3 @joostdekeijzer
5 years ago

This issue does not exist on IIS/8.5

Last edited 5 years ago by joostdekeijzer (previous) (diff)

#4 @jcroucher
4 years ago

  • Summary changed from WP_Filesystem_FTPext::exists($filename) returns false when file exists on IIS/6.0 Microsoft FTP Service to WP_Filesystem_FTPext::exists($filename) returns false when file exists on FTP Service ( IIS/6.0 Microsoft & Linux/vsftp )

This issue still exists but I am finding it on a Linux box using vsftp. I am getting this across multiple boxes.
I am running Wordpress 4.4.2

The issue is in

wp-admin/includes/class-wp-filesystem-ftpext.php

public function exists($file) {
                $list = @ftp_nlist($this->link, $path);

                if ( empty( $list ) && $this->is_dir( $file ) ) {
                        return true; // File is an empty directory.
                }

                return !empty($list); //empty list = no file, so invert.
        }

The problem seems to be that ftp_nlist does not return . files, such as .maintenance
This was causing the site to be stuck in maintenance mode after each update.

I have fixed it by changing the ftp_nlist line to

$list = @ftp_rawlist($this->link, "-al" .$file);

There are numerous posts on other forums mentioning the issue of ftp_nlist not returning hidden files.

Last edited 4 years ago by jcroucher (previous) (diff)

#5 @dd32
4 years ago

  • Keywords early added
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to dd32
  • Status changed from new to accepted

I see no issue with switching to ftp_rawlist here, I guess it varies between servers as to which ones include -a by default. It makes sense that some would have hidden files disabled by default.
I think there's also an argument to be made that we should use wordpress.maintenance instead or something similar.

The actual change here would probably be just to $list = @ftp_rawlist($this->link, "-a" . $file ); (lacking the -l).

Marking as early for 4.4, as I don't want to change upgrade-related code during beta.

#6 @samuelsidler
4 years ago

  • Keywords 4.4-early added

#7 @dd32
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 33648:

Upgrades: When upgrading via FTP, use LIST -a to detect if a file exists.
LIST & NLST by default on some servers require the -a flag to view hidden files (ie. .maintenance)
Although we could simply pass the -a flag to NLST, opting to use LIST which we use elsewhere should mean less chance of server incompatibilities.

Props jcroucher.
Fixes #28013.

#8 @dd32
4 years ago

  • Keywords early 4.4-early removed
  • Milestone changed from Future Release to 4.4

This issue still exists but I am finding it on a Linux box using vsftp.

@jcroucher Can you let me know what your configuration of vsftp is? I've been unable to make it work by using either force_dot_files or hide_files directives.. I'd like to actually test this change properly :)

If someone reports strange upgrade behaviour in the 4.4 cycle of it not detecting files properly, please re-open :)

#9 @mmucklo
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This fix did not work for me.

I may be doing something wrong, and I apologize in advance for wasting your time if so.

My update still fails

I patched my 4.2 code actually (been coming across this same issue trying to update a 4.2 installation).

I've been stepping through via XDEBUG and was coming to the same issue.

In includes/update-core.php this line of code is failing.

if ( $wp_filesystem->exists( $from . $root . 'readme.html' ) && $wp_filesystem->exists( $from . $root . 'wp-includes/version.php' ) ) {

This is calling into:
wp-admin/includes/class-wp-filesystem-ftpext.php

Both functions fail:
It fails with ftpn_list and it fails with ftp_rawlist

Maybe because input to command is a file:
The reason it fails is that you're passing in a $file into a command that seems to require a directory as it's argument. I may be wrong here, however when I pass in dirname($file) into the former command, things work fine.

The docs seem to indicate the required parameter is a directory, not a file.
http://php.net/manual/en/function.ftp-nlist.php
http://php.net/manual/en/function.ftp-rawlist.php

Suggestion:
Maybe first try the raw list, if that doesn't work, then do a rawlist of dirname($file) (given dirname($file) is not empty), and then check to see if the list returned contains the file asked for.

Maybe something similar to this:

if (empty($list) && !$this->is_dir($file) && !empty($dirname = dirname($file))) {
    $testList = @ftp_rawlist( $this->link, $dirname );
    if (!empty($testList)) {
        // Find if $file exists in $testList, set some flag, or $list itself, etc.
    }
}

Let me know and I can give you a working example...

#10 @dd32
4 years ago

Thanks for the feedback @mmucklo!

The reason it fails is that you're passing in a $file into a command that seems to require a directory as it's argument. I may be wrong here, however when I pass in dirname($file) into the former command, things work fine.

When a file is presented, a FTP server that respects the protocol is supposed to return information about that file (Ignore whatever the PHP docs suggest here), unfortunately there are a number of FTP servers out there which obviously do not conform to spec.

The alternative solution, as you suggest, is to list the contents of the directory above it to determine if the file is present.
Can you try switching out the exists method for this one?

	public function exists( $file ) {
		$path = dirname( $file );
		$filename = basename( $file );

		$file_list = @ftp_nlist( $this->link, '-a ' . $path );

		return $list && in_array( $filename, $file_list );
	}

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


4 years ago

#12 @SergeyBiryukov
4 years ago

  • Keywords needs-patch added; has-patch removed

#13 @dd32
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 34733:

Updates: Try a more compatible method to detect if a file exists when using the FTP Extension.
This change causes it to list the parent directories files, and assets that the node exists within the returned listing, this is a little more compatible than relying upon the FTP server to correctly filter the returned resultset to the specific file/node being requested.
Fixes #28013

#14 @dd32
4 years ago

  • Keywords needs-patch removed
  • Milestone changed from 4.4 to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening this ahead of a revert for 4.4.1 & trunk.

See #34976 and #35026 where this has caused an issue. Long story short: Many servers are not happy with this new method.

See my patches on #34976 and comments on #35026 which could be helpful in fixing this in the future.

#15 @dd32
4 years ago

In 35944:

Filesystem: Revert [33648] and [34733] unfortunately these have caused issues for some servers, while fixing it for others.

See #28013

Fixes #34976 for trunk
Fixes #34976 for trunk

#16 @dd32
4 years ago

In 35945:

Filesystem: Revert [33648] and [34733] unfortunately these have caused issues for some servers, while fixing it for others.

See #28013
Fixes #34976 for the 4.4 branch
Fixes #34976 for the 4.4 branch

#17 @dd32
4 years ago

#35967 was marked as a duplicate.

#18 @jcroucher
3 years ago

Is there any update to this? The fix in https://core.trac.wordpress.org/changeset/34733 does work for me.
I am still manually updating that file on all my installs so it will work.

I should add, that initial bug breaks maintenance mode also. As in, update a plugin. it creates the .maintenance file, but it never deletes it. Adding in the patch from 34733 fixes this issue.

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

#19 @dd32
3 years ago

There's unlikely to be any updates to this in the next few versions.

If it works on your server, great, if it doesn't, unfortunately your only options are to switch FTP servers, FTP configurations, or try to avoid needing to use FTP at all.

#20 @hnhn2
18 months ago

This is rather painful. Seeing this on several 4.9.4 installations (Linux vsftpd) and any plugin updates just bring the sites down!

I don't understand two things
here:

  • Why at :839 bother with ->exists($file) when at :837 there is no such extra care? If the call on a non-existent file has undesired consequences, why it's being handled in one case only? Yes, the call is returning an empty list in this case, but it also seems superfluous.
  • At :841 the call to $wp_filesystem->delete( $file ); invokes RMDIR in vsftp, which fails. So regardless of the failing exists() call, this seems to be the main problem. Shouldn't there instead be $wp_filesystem->delete($file, false, 'f');?

@hnhn2
18 months ago

treat .maintenance file deletion as file DELETE instead of RMDIR

Note: See TracTickets for help on using tickets.