Incorrect POP3 error messages in wp-mail.php

[solarissmoke]

The current code in wp-mail.php is:

$pop3 = new POP3();
$count = 0;

if ( ! $pop3->connect(get_option('mailserver_url'), get_option('mailserver_port') ) ||
	! $pop3->user(get_option('mailserver_login')) ||
	( ! $count = $pop3->pass(get_option('mailserver_pass')) ) ) {
		$pop3->quit();
		wp_die( ( 0 === $count ) ? __('There doesn’t seem to be any new mail.') : esc_html($pop3->ERROR) );
}

Because of the multiple OR statements, if one of the first two fails (i.e., connect() or user() ) then the pass() function is never called and $count is never changed from the initial 0. Thus (0 === $count) always evaluates to true in these cases, giving an incorrect error message. e.g., if the connection failed, you still get "there doesn't seem to be any new mail" instead of the POP3 error.

[solarissmoke]

Also, calling pop3->quit() changes the pop3->ERROR message anyway, so the original true error is masked.

[solarissmoke]

Make sure we have a working pop3 connection before attempting to send password and validate response. Do not call pop3->quit() in case of failure as this is done internally by the class. Trying to call it after a failure will cause another failure (connection does not exist) to overwrite the original one.

[nacin]

Make sure we have a working pop3 connection before attempting to send password and validate response.

I don't understand that part. Once we hit false (connection, user, pass, in that order), we would break into the if statement and spit out the error. So we would never evaluate the user if we don't have a connection.

The pop3->quit part makes sense, though I'd want to test.

This has been around for quite a while I'd imagine, so I'm going to move this to future for now.

[solarissmoke]

Replying to nacin:

I don't understand that part. Once we hit false (connection, user, pass, in that order), we would break into the if statement and spit out the error. So we would never evaluate the user if we don't have a connection.

The error-spitting bit checks for $count === 0 before deciding what to output. Problem is, we manually set $count to 0 beforehand. And it won't change unless the last of the three of (connection, user, pass) is reached. Which it won't be if connection/user fails. So if connection/user fails, you still get the message "there doesn't appear to be any new mail".

So we need to do connection/user first, make sure there was no error, and then do the password.

[nacin]

Makes sense now.

[nacin]

(In [14518]) Adjust POP3 error checks in wp-mail.php. props solarissmoke, fixes #13163.