Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#8580 closed defect (bug) (fixed)

Automatic plugin ftp value stored improperly if FTP failed. causing function to break for good

Reported by: dwenaus Owned by:
Milestone: 2.7.1 Priority: normal
Severity: normal Version: 2.7
Component: Upgrade/Install Keywords: hsa-patch needs-testing
Focuses: Cc:


This is a minor bug as it does not affect that many, but an issue nonetheless.

On my server, a stock CentOS. using Wordpress 2.7 when i go to automatically install or upgrade a plugin if i initially enter localhost as the server, then everything works fine. however if I enter my actual hostname either http://mydomain.com, or ftp://mydomain.com simply mydomain.com then the FTP will not work. I realize that this issue is probably server specific.

The bug arises when I attempt to connect a second time. Then the already stored info in wp_option -> ftp_credentials is invalid. somehow the end part of the hostname got stored as the port and the hostname is only stored as 'http'. and then strange things occur as the stored domain is appended to the entered domain. So there is a bug in how the ftp_credentials data is stored when a failure occurs.

here is the value of my ftp_credentials when it has stored the wrong value:

once the data is stored improperly the only solution is to delete the ftp_credentials option field, as entering it again and again does not work as the wrong port is always appended.

i would also recommend a small help text if there is an error (or even before the error) that on some hosts they may need to enter localhost instead of their domain name.

Is the solution to escape the char before serializing. It seems the problem might be the : after http:

Attachments (2)

8580.diff (1.9 KB) - added by DD32 7 years ago.
8580.2.diff (1.7 KB) - added by DD32 7 years ago.

Download all attachments as: .zip

Change History (13)

#1 @dwenaus
7 years ago

the problem seems to be on lines 669-670 on admin/file.php
where it is exploding out the : and just expecting it is a port number
before storing it as a port, should we not check to see if there are two colons?

#2 @dwenaus
7 years ago

or maybe simple strip http:// or ftp:// from the form when it is input using grep.

#3 @DD32
7 years ago

+1 for a $hostname = preg_replace('|\w+://|', '', $hostname); on the hostname

But you should be able to override the stored settings the 2nd time around.. Unless the Port is sticking (which it could be, i'll have to look closer at that)

Some extra help labels would also bring it into line with the rest of WP's forms (unless it already has some, i cant remember)

#4 @dwenaus
7 years ago

yes, the port is sticking, $credentialsport? is only set when there is a port, otherwise it is pulled from the db and never zerod.

#5 @Otto42
7 years ago

  • Version set to 2.7

Port is definitely sticking, which it should not do. Line 670:

if ( strpos($credentialshostname?, ':') )

list( $credentialshostname?, $credentialsport? ) = explode(':', $credentialshostname?, 2);


The credentials are loaded from the options table, then overridden like this. If the port was specified before and is not specified now (no colon in the hostname), then it doesn't get changed from its previous value.

Port should default to 21 if no port is specified. A simple else $credentialsport? = 21 after this bit of code would fix it.

#6 @dwenaus
7 years ago

that, or simply null it if it wasn't entered with an an extra else
add this on line 671:

    else $credentials['port'] = '';   

7 years ago

#7 @DD32
7 years ago

  • Keywords hsa-patch needs-testing added

Feel like testing the attached patch?

Er, Ignore the first patch, Theres no need for a FTP_PORT constant, the port can go in the hostname if needed.

7 years ago

#8 @dwenaus
7 years ago

I tested it, it works great.
I entered http:// and it removed it fine and stored it perfectly.
I used port number 21 and it worked and correctly remembered the port the next time.

but after a few or maybe one good attempt, it remembered my password and didn't ask it again. interesting. maybe this is normal behaviour.

thanks DD32!

#9 @dwenaus
7 years ago

ignore that comment about it remembering my password, I went to the wrong tab on my browser (oops)

#10 @ryan
7 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [10198]) Reset port when saving credntials. Props dwenaus and DD32. fixes #8580 for trunk

#11 @ryan
7 years ago

(In [10199]) Reset port when saving credntials. Props dwenaus and DD32. fixes #8580 for 2.7

Note: See TracTickets for help on using tickets.