WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 12 months ago

#28616 new defect (bug)

ftp_fput should have a retry threshold

Reported by: runderwo Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.9
Component: Filesystem API Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

In class-wp-filesystem-ftpext.php in put_contents(), ftp_fput() is called to transfer a file to the FTP server.

The first problem is that warnings are suppressed, so the user never knows that this is the location where something went wrong.

The broader problem is that a single transient network failure between the web server and FTP server causes this call to fail. Well, that would be not a big problem if the user's operation were a single call. However, this function is used by Wordpress auto-upgrade when direct file access fails. For each file in the Wordpress distribution, this function is called to transfer it to the FTP server.

As the default Wordpress distribution gets more and more bloated, the number of calls increases proportionally, and the likelihood of at least one FTP transaction failing increases. A single failure aborts the entire Wordpress upgrade. It seems reasonable to at least retry up to (say) three times the ftp_fput() call before returning an error to the higher level and aborting a large process such as Wordpress upgrade which is using FTP as backing for the filesystem abstraction layer.

Attachments (2)

28616.patch (937 bytes) - added by stevenlinx 12 months ago.
28616.2.patch (937 bytes) - added by stevenlinx 12 months ago.

Download all attachments as: .zip

Change History (7)

#1 @runderwo
3 years ago

I patched around it with this hack for now.

while (($ret = ftp_fput($this->link, $file, $temp, $type)) != 1)
  {
    error_log("Reconnecting"); $this->connect();
  }

This naive approach retries infinitely, reconnecting to FTP server on failure. There should probably be a new option in PHP to ftp_set_option() to control the number of connection retries. But WP does not have any control over that.

It doesn't really seem feasible to fix at the higher level because many things call put_contents() directly (like unzip layer). I don't see an option besides making the lower level more resilient.

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

#2 @tellyworth
3 years ago

  • Version changed from trunk to 3.9

#3 @chriscct7
22 months ago

  • Keywords needs-patch added

@stevenlinx
12 months ago

#4 @stevenlinx
12 months ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

1.) "...a new option in PHP to ftp_set_option() to control the number of connection retries"

I don't think PHP will ever provide such thing because the response of each iteration could be different and should be handled by the users.

2.) I think the best way forward may be to simply limit the number of retries? I don't foresee how simply giving the upload code a second or third try can impact other calling code.

#5 @stevenlinx
12 months ago

The red and green color coded block in the previous patch should be reversed.

A revised version was uploaded.

Note: See TracTickets for help on using tickets.