Page MenuHomePhabricator

bogus email error message when creating an account ($wgSMTP only)
Closed, ResolvedPublic

Description

Author: stephen.f.lawrence

Description:
Email is sent by wiki when registering a new user, but registering user gets message on wiki page after registering stating that the wiki failed to send confirmation email for reason "1". In fact the confirmation email was sent.

wiki configured using wgSMTP, which is documented as:

$wgSMTP = array(
'host' => "localhost",
'IDHost' => "domain for MessageID",
'port' => 25,
'auth' => false,
'username' => "my_user_name",
'password' => "my_password"
);

The error essage is caused by this code in SpecialUserlogin.php

			$error = $u->sendConfirmationMail();
			if( WikiError::isError( $error ) ) {
				$wgOut->addWikiText( wfMsg( 'confirmemail_sendfailed', $error->getMessage() ) );
			} else {
				$wgOut->addWikiText( wfMsg( 'confirmemail_oncreate' ) );
			}

WikiError::isError( $error ) is evaluating as true and
$error->getMessage() is returning "1".

In the $wgSMTP case, the called functions are:

sendConfirmationMail()

return (sendMail())

sendMail()

        $error = userMailer( $to, $sender, $subject, $body );

		if( $error == '' ) {
			return true;
		} else {
			return new WikiError( $error );
		}

userMailer()

                $mail_object =& Mail::factory('smtp', $wgSMTP); 
		if (is_array($dest)) {
			$chunks = array_chunk($dest, $wgEnotifMaxRecips);
			foreach ($chunks as $chunk) {
				$e = send_mail($mail_object, $chunk, $headers, $body);
				if ($e != '')
					return $e;
			}
		} else
			return $mail_object->send($dest, $headers, $body);

single destination (recipient), so $dest is not an array.

Mail::factory->send() returns true on success.

So on successful email, I think this is happening:

send() returns true
userMailer() returns true
sendMail returns new WikiError(true);

I don't know much about php. My guess is that true does not equal ''.


Version: 1.11.x
Severity: normal

Details

Reference
bz11567

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:00 PM
bzimport added a project: MediaWiki-Email.
bzimport set Reference to bz11567.
bzimport added a subscriber: Unknown Object (MLST).

ot-wikimedia wrote:

Proposed patch to fix mailer problem

attachment mailpatch.diff ignored as obsolete

ot-wikimedia wrote:

svn diff

I haven't found a coding styles guideline, so I hope I didn't commit any horrible errors.

Attached:

ot-wikimedia wrote:

There's definitely a little quirkiness on how the error result is handled. The
send_mail function does better by checking if it's an object, but according to
the pear mail docs, the Pear::isError function is recommended.

I don't know whether the original choice was deliberate, based on superior knowledge of the system, or just a bug.

I've normalized the calling checks in r30896...

  • When sending to a single recipient, the PEAR return value now goes through the proper checks

^^ this would have been the direct cause of this bug; returning bogus success messages on failure and bogus error messages on success.

  • Using PEAR::isError() explicitly
  • UserMailer::send() / userMailer() now returns true-or-WikiError, matching the calling convention of User::sendMail(). This convention is apparently expected by many of its callers anyway. :P
  • Bug 13094 has been marked as a duplicate of this bug. ***
  • Bug 13276 has been marked as a duplicate of this bug. ***
  • Bug 13411 has been marked as a duplicate of this bug. ***