Autoupgrade: Synchronous FTP client fails while plugin upgrade/installation (on some systems)

[darkman82]

Decription here
​http://wordpress.org/support/topic/288093

Apache/2.2.13 (Unix) DAV/2 mod_ssl/2.2.13 OpenSSL/0.9.8k configured

PHP 5.2.10 with Suhosin-Patch 0.9.7 (cli) (built: Jul 14 2009 11:56:54)

pure-ftpd-1.0.22-1

I solved by changing some lines of code in class-ftp-sockets.php
(_exec and _readmsg)

Below the patch:
​http://darkman.it/x/class-ftp-sockets.patch

[dd32]

Would it be possible for you to explain the changes you've made? ie "Instead of looking for bar, It looks for foo Followed by bar"

Also, attaching a diff of WordPress Trunk SVN would be appreciated, But if you cant do that, I can create a new patch from yours late for comparison.

[darkman82]

Replying to dd32:

Would it be possible for you to explain the changes you've made? ie "Instead of looking for bar, It looks for foo Followed by bar"

Also, attaching a diff of WordPress Trunk SVN would be appreciated, But if you cant do that, I can create a new patch from yours late for comparison.

I just added a kind of buffered text reader in class-ftp-sockets.php, I changed only _readmsg function and added a line in _exec function:

BEFORE:

        function _readmsg($fnction="_readmsg"){
                if(!$this->_connected) {
                        $this->PushError($fnction,'Connect first');
                        return FALSE;
                }
                $result=true;
                $this->_message="";
                $this->_code=0;
                $go=true;
                do {
                        $tmp=@socket_read($this->_ftp_control_sock, 4096, PHP_BINARY_READ);
                        if($tmp===false) {
                                $go=$result=false;
                                $this->PushError($fnction,'Read failed', socket_strerror(socket_last_error($this->_ftp_control_sock)));
                        } else {
                                $this->_message.=$tmp;
                                $go = !preg_match("/^([0-9]{3})(-.+\\1)? [^".CRLF."]+".CRLF."$/Us", $this->_message, $regs);
                        }
                } while($go);
                if($this->LocalEcho) echo "GET < ".rtrim($this->_message, CRLF).CRLF;
                $this->_code=(int)$regs[1];
                return $result;
        }

        function _exec($cmd, $fnction="_exec") {
                if(!$this->_ready) {
                        $this->PushError($fnction,'Connect first');
                        return FALSE;
                }
                if($this->LocalEcho) echo "PUT > ",$cmd,CRLF;
                $status=@socket_write($this->_ftp_control_sock, $cmd.CRLF);
                if($status===false) {
                        $this->PushError($fnction,'socket write failed', socket_strerror(socket_last_error($this->stream)));
                        return FALSE;
                }
                $this->_lastaction=time();
                if(!$this->_readmsg($fnction)) return FALSE;
                return TRUE;
        }

AFTER:

	function _readmsg($fnction="_readmsg"){
		if(!$this->_connected) {
			$this->PushError($fnction,'Connect first');
			return FALSE;
		}
		$result=true;
		$this->_message="";
		$this->_code=0;
		// Read until at least ONE line is available
		while( ($pos = strpos($this->_buffz, CRLF))===false ){
			$tmp=@socket_read($this->_ftp_control_sock, 4096, PHP_BINARY_READ);
			if($tmp===false){
                                $this->PushError($fnction,'Read failed', socket_strerror(socket_last_error($this->_ftp_control_sock)));
                                return FALSE;
                        }
			$this->_buffz.=$tmp;
		}
		$this->_message = substr($this->_buffz,0,$pos).CRLF;
		$this->_buffz = substr($this->_buffz,$pos+strlen(CRLF));
		if( !preg_match("/^([0-9]{3})(-.+\\1)? .+$/Us",$this->_message, $regs))
			return ($this->_readmsg($fnction));
		if($this->LocalEcho) echo "GET < ".rtrim($this->_message, CRLF).CRLF;
                $this->_code=(int)$regs[1];
                return $result;
	}

	function _exec($cmd, $fnction="_exec") {
		if(!$this->_ready) {
			$this->PushError($fnction,'Connect first');
			return FALSE;
		}
		if($this->LocalEcho) echo "PUT > ",$cmd,CRLF;
		$this->_buffz=""; // ADDED
		$status=@socket_write($this->_ftp_control_sock, $cmd.CRLF);
		if($status===false) {
			$this->PushError($fnction,'socket write failed', socket_strerror(socket_last_error($this->stream)));
			return FALSE;
		}
		$this->_lastaction=time();
		if(!$this->_readmsg($fnction)) return FALSE;
		return TRUE;
	}

That's all.

You can see the "patched" file here:
​http://darkman.it/x/class-ftp-sockets.php.txt

[dd32]

patch from darkman82

[dd32]

Just uploaded the patch from darkman82. Logic seems sane to me, But really needs some proper testing

[bike]

The automatic upgrade still has many problems as of 3.0.

The only way to solve it was to change from Pure-FTP to proFTPd on the WHM, after which suddenly all upgrades are lightning fast and more importantly, they finish successfully.

Not sure where the bug is, but apparently it still exists and most 'solutions' that float around the web are focused (incorrectly) on increasing PHP memory.

Cheers, Bike.

[dd32]

The automatic upgrade still has many problems as of 3.0.

The only way to solve it was to change from Pure-FTP to proFTPd on the WHM, after which suddenly all upgrades are lightning fast and more importantly, they finish successfully.

Just like to mention that I believe that's separate from this ticket, and has been partially taken care of with #10913

[chriscct7]

@dd32 Did you want to revisit this?

[dd32]

I'll be blunt, this has sat here for 5 years untouched because no-one really has a good way of testing low level changes like this, and it affects very few people, the original bug is probably incredibly hard (if not impossible) to reproduce.

Looking at the stats of both 4.4/4.3.1/4.3 upgrades, about 2% of sites use FTP to update and 2% of those 2% (So ~0.05% of total upgrades, a few thousand at most) use the FTPSockets class which this patch applies to. Even then, PemFTP has 2 modes, Sockets and pure PHP, this patch only applies to the sockets section..
There's about 3~5x more users using the SSH2 functionality than there are using the FTPSockets class.

So what I'm saying is, @darkman82 years later, Thanks for the patch, but unfortunately as we're unable to test this without fear of breaking a rarely used piece of code, i'm going to mark this as wontfix.