Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 20 months ago

#53318 closed defect (bug) (duplicate)

FTPext method exists() returns always false for files!

Reported by: arl1nd's profile arl1nd Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.7.2
Component: Filesystem API Keywords: needs-patch dev-feedback
Focuses: docs Cc:

Description

Hi there,

Are you aware that WP_Filesystem_FTPext method exists( $file ) only checks for existence of directory, giving file path as argument will always return false!

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

The original method code:

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

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

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

If you check the PHP manual for ftp_nlist it only accepts directory as input value.

Any thoughts?

Attachments (1)

53318.diff (1.1 KB) - added by mkox 2 years ago.
Added docs for ftp_nlist handling

Download all attachments as: .zip

Change History (7)

#1 @SergeyBiryukov
3 years ago

  • Component changed from General to Filesystem API

#2 @mkox
2 years ago

  • Keywords dev-feedback added

This seems not to be working properly from PHP manuals, but from RFC959(https://www.ietf.org/rfc/rfc959.txt page 32) it looks well.

As ftp_nlist is using ftp list and ftp list supports both directories and files with a result, it works out.

It seems to work and there is a good explanation at stackoverflow: https://stackoverflow.com/questions/22040633/php-check-if-file-exists-on-ftp-server-with-no-size-support

Anyway there is no other clean way to determine if a file exists without downloading it. Other solutions like ftp_size are not supported by all servers.

Maybe this could be documented within the method as well, I will leave a patch here.

The discussion why someone wants to check for existing files/dirs at all is another story.

@mkox
2 years ago

Added docs for ftp_nlist handling

#3 @SergeyBiryukov
2 years ago

  • Focuses docs added
  • Milestone changed from Awaiting Review to 6.0

#4 @afragen
2 years ago

Related #51170

#5 @afragen
2 years ago

  • Milestone 6.0 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #51170.

#6 @audrasjb
20 months ago

In 53860:

Filesystem: Rewrite FTP/FTP Sockets exists() methods to implement a more stable check.

WordPress FTP file checking was previously based upon ftp_nlist(). This function can be problematic at scale with a directory containing a large number of files. The same issue occurred using it with ftpsockets.

This changeset rewrites the FTP exists() functions to utilize a more efficient and stable check.

Props giox069, desrosj, mkox, afragen, costdev, pbiron, peterwilsoncc.
Fixes #51170.
See #53318, #39781.

Note: See TracTickets for help on using tickets.