Make WordPress Core

Opened 7 years ago

Closed 3 months ago

#39781 closed defect (bug) (duplicate)

Automatic updater does not work with VSFTPd server

Reported by: a1cypher's profile a1cypher Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7.2
Component: Upgrade/Install Keywords: reporter-feedback close
Focuses: Cc:

Description (last modified by SergeyBiryukov)

The automatic updater fails during update on verifying files.

The reason for the failure is that the file wp-admin/includes/class-wp-filesystem-ftpext.php and wp-admin/includes/class-wp-filesystem-ftpsockets.php use the ftp command NLIST to verify that the files wordpress/readme.html and wordpress/wp-includes/version.php exist before continuing with the installation.

When issuing the NLIST command to the VSFTPd server with a full filename, it returns with an empty list because the NLIST command is only meant to list directories. See https://www.ietf.org/rfc/rfc959.txt

         NAME LIST (NLST)

            This command causes a directory listing to be sent from
            server to user site.  The pathname should specify a
            directory or other system-specific file group descriptor; a
            null argument implies the current directory.  The server
            will return a stream of names of files and no other
            information.  The data will be transferred in ASCII or
            EBCDIC type over the data connection as valid pathname
            strings separated by <CRLF> or <NL>.  (Again the user must
            ensure that the TYPE is correct.)  This command is intended
            to return information that can be used by a program to
            further process the files automatically.  For example, in
            the implementation of a "multiple get" function.

When wordpress gets back an empty list it makes the false assumption that the file does not exist.

Instead of using NLIST wordpress should use the LIST command which according to the same RFC should return information on the file if a file is specified.

         LIST (LIST)

            This command causes a list to be sent from the server to the
            passive DTP.  If the pathname specifies a directory or other
            group of files, the server should transfer a list of files
            in the specified directory.  If the pathname specifies a
            file then the server should send current information on the
            file.  A null argument implies the user's current working or
            default directory.  The data transfer is over the data
            connection in type ASCII or type EBCDIC.  

Replacing the VSFTPd server which correctly implements the NLST command with ProFTPd which implements the NLST command incorrectly by returning information about the file in question fixes the problem.

Change History (9)

#1 @SergeyBiryukov
7 years ago

  • Description modified (diff)

#3 @hnhn2
6 years ago

Experiencing this with the 4.9.5 version. All kinds of automatic updates are broken, as the exists() function doesn't work properly.

#4 @hnhn2
6 years ago

This looks like another duplicate of #28013.

#6 @audrasjb
21 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.

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


20 months ago

#8 @costdev
18 months ago

  • Keywords reporter-feedback close dev-feedback added

Commits in #51170 change:

  • WP_Filesystem_ftpsockets::exists() to use file size to determine existence.
  • WP_Filesystem_FTPext::exists() to use ftp_rawlist(), which uses the LIST command rather than NLIST.

I believe this should resolve the issue in this ticket. I'm adding the close, reporter-feedback and dev-feedback keywords for a confidence check.

#9 @hellofromTonya
3 months ago

  • Keywords dev-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Update:

The commits in #51170 were reverted and the ticket was closed as a duplicate of #28013 (see the comment here for more information).

I'll close this ticket as well as a duplicate. Let's move the discussion and resolution to #28013, i.e. to keep all the details in one place. Thanks.

Note: See TracTickets for help on using tickets.