Make WordPress Core

Opened 10 years ago

Last modified 5 years ago

#28616 new defect (bug)

ftp_fput should have a retry threshold

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


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 8 years ago.
28616.2.patch (937 bytes) - added by stevenlinx 8 years ago.

Download all attachments as: .zip

Change History (7)

#1 @runderwo
10 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 to ftp_set_option() to control the number of connection retries.

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.

Version 1, edited 10 years ago by runderwo (previous) (next) (diff)

#2 @tellyworth
10 years ago

  • Version changed from trunk to 3.9

#3 @chriscct7
9 years ago

  • Keywords needs-patch added

8 years ago

#4 @stevenlinx
8 years 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.

8 years ago

#5 @stevenlinx
8 years 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.