Make WordPress Core

Opened 15 years ago

Closed 8 years ago

#10831 closed defect (bug) (wontfix)

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

Reported by: darkman82's profile darkman82 Owned by: dd32's profile dd32
Milestone: Priority: normal
Severity: normal Version: 2.8.4
Component: Filesystem API Keywords: needs-refresh
Focuses: Cc:

Description

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

Attachments (1)

10831.diff (2.2 KB) - added by dd32 14 years ago.
patch from darkman82

Download all attachments as: .zip

Change History (12)

#1 follow-up: @dd32
15 years ago

  • Component changed from General to Filesystem
  • Keywords reporer-feedback added; FTP auto upgrade plugin dirlist removed
  • Milestone changed from Unassigned to 2.9
  • Owner set 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.

#2 in reply to: ↑ 1 @darkman82
15 years ago

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

#3 @markjaquith
14 years ago

  • Milestone changed from 2.9 to 3.0

@dd32
14 years ago

patch from darkman82

#4 @dd32
14 years ago

  • Keywords has-patch needs-testing added; reporer-feedback removed

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

#5 @dd32
14 years ago

  • Milestone changed from 3.0 to 3.1
  • Status changed from new to reviewing

#6 @bike
14 years ago

  • Version changed from 2.8.4 to 3.0.1

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.

#7 @nacin
13 years ago

  • Milestone changed from Awaiting Triage to Future Release
  • Version changed from 3.0.1 to 2.8.4

#8 @dd32
13 years ago

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

#9 @chriscct7
9 years ago

  • Keywords needs-refresh added; has-patch needs-testing removed

#10 @chriscct7
8 years ago

@dd32 Did you want to revisit this?

#11 @dd32
8 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reviewing to closed

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.

Note: See TracTickets for help on using tickets.