WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#13163 closed defect (bug) (fixed)

Incorrect POP3 error messages in wp-mail.php

Reported by: solarissmoke Owned by: westi
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Blog by Email Keywords: has-patch
Focuses: Cc:

Description

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.

Attachments (1)

13163.patch (1014 bytes) - added by solarissmoke 4 years ago.
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.

Download all attachments as: .zip

Change History (8)

comment:1 solarissmoke4 years ago

  • Keywords has-patch added

comment:2 solarissmoke4 years ago

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

solarissmoke4 years ago

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.

comment:3 follow-up: nacin4 years ago

  • Milestone changed from 3.0 to Future Release

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.

comment:4 in reply to: ↑ 3 solarissmoke4 years ago

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.

comment:5 nacin4 years ago

  • Keywords early added
  • Milestone changed from Future Release to 3.1

Makes sense now.

comment:6 nacin4 years ago

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

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

comment:7 nacin4 years ago

  • Keywords early removed
  • Milestone changed from 3.1 to 3.0
Note: See TracTickets for help on using tickets.