(bug 34421) duplicate Subject / wrong To: headers in mail
authorAntoine Musso <hashar@users.mediawiki.org>
Sat, 18 Feb 2012 15:34:12 +0000 (15:34 +0000)
committerAntoine Musso <hashar@users.mediawiki.org>
Sat, 18 Feb 2012 15:34:12 +0000 (15:34 +0000)
This fixup our mail sending system which duplicated the Subject and To: header.
In some conditions it used only the email address for the From: field skipping
the username ($dest in old code only contains the email address).

Mails sent to a single recipients will look alike with mail() or PEAR Mail.
For multiple recipients:
 - php mail() will show the recipient email and 'undisclosed-recipients:'
 - PEAR Mail will only show 'undisclosed-recipients:'

Reverts r111820
Follow  r111819
Fixup    r93397

includes/UserMailer.php

index 88a7d61..a2a0626 100644 (file)
@@ -163,29 +163,52 @@ class UserMailer {
 
                wfDebug( __METHOD__ . ': sending mail to ' . implode( ', ', $to ) . "\n" );
 
-               $dest = array();
+               # Make sure we have at least one address
+               $has_address = false;
                foreach ( $to as $u ) {
                        if ( $u->address ) {
-                               $dest[] = $u->address;
+                               $has_address = true;
+                               break;
                        }
                }
-               if ( count( $dest ) == 0 ) {
+               if ( !$has_address ) {
                        return Status::newFatal( 'user-mail-no-addy' );
                }
 
+               # Forge email headers
+               # -------------------
+               #
+               # WARNING
+               #
+               # DO NOT add To: or Subject: headers at this step. They need to be
+               # handled differently depending upon the mailer we are going to use.
+               #
+               # To:
+               #  PHP mail() first argument is the mail receiver. The argument is
+               #  used as a recipient destination and as a To header.
+               #
+               #  PEAR mailer has a recipient argument which is only used to
+               #  send the mail. If no To header is given, PEAR will set it to
+               #  to 'undisclosed-recipients:'.
+               #
+               #  NOTE: To: is for presentation, the actual recipient is specified
+               #  by the mailer using the Rcpt-To: header.
+               #
+               # Subject: 
+               #  PHP mail() second argument to pass the subject, passing a Subject
+               #  as an additional header will result in a duplicate header.
+               #
+               #  PEAR mailer should be passed a Subject header.
+               #
+               # -- hashar 20120218
+
                $headers['From'] = $from->toString();
                $headers['Return-Path'] = $from->address;
-               if ( count( $to ) == 1 ) {
-                       $headers['To'] = $to[0]->toString();
-               } else {
-                       $headers['To'] = 'undisclosed-recipients:;';
-               }
 
                if ( $replyto ) {
                        $headers['Reply-To'] = $replyto->toString();
                }
 
-               $headers['Subject'] = self::quotedPrintable( $subject );
                $headers['Date'] = date( 'r' );
                $headers['MIME-Version'] = '1.0';
                $headers['Content-type'] = ( is_null( $contentType ) ?
@@ -203,6 +226,10 @@ class UserMailer {
                }
 
                if ( is_array( $wgSMTP ) ) {
+                       #
+                       # PEAR MAILER
+                       # 
+
                        if ( function_exists( 'stream_resolve_include_path' ) ) {
                                $found = stream_resolve_include_path( 'Mail.php' );
                        } else {
@@ -224,9 +251,20 @@ class UserMailer {
                        }
 
                        wfDebug( "Sending mail via PEAR::Mail\n" );
-                       $chunks = array_chunk( $dest, $wgEnotifMaxRecips );
+
+                       $headers['Subject'] = self::quotedPrintable( $subject );
+
+                       # When sending only to one recipient, shows it its email using To:
+                       if ( count( $to ) == 1 ) {
+                               $headers['To'] = $to[0]->toString();
+                       }
+
+                       # Split jobs since SMTP servers tends to limit the maximum
+                       # number of possible recipients.        
+                       $chunks = array_chunk( $to, $wgEnotifMaxRecips );
                        foreach ( $chunks as $chunk ) {
                                $status = self::sendWithPear( $mail_object, $chunk, $headers, $body );
+                               # FIXME : some chunks might be sent while others are not!
                                if ( !$status->isOK() ) {
                                        wfRestoreWarnings();
                                        return $status;
@@ -235,6 +273,10 @@ class UserMailer {
                        wfRestoreWarnings();
                        return Status::newGood();
                } else  {
+                       # 
+                       # PHP mail()
+                       #
+
                        # Line endings need to be different on Unix and Windows due to
                        # the bug described at http://trac.wordpress.org/ticket/2603
                        if ( wfIsWindows() ) {
@@ -244,8 +286,9 @@ class UserMailer {
                                $endl = "\n";
                        }
 
-                       # Subject header is unneeded since it an argument of mail()
-                       unset( $headers['Subject'] );
+                       if( count($to) > 1 ) {
+                               $headers['To'] = 'undisclosed-recipients:;';
+                       }
                        $headers = self::arrayToHeaderString( $headers, $endl );
 
                        wfDebug( "Sending mail via internal mail() function\n" );
@@ -256,7 +299,7 @@ class UserMailer {
                        set_error_handler( 'UserMailer::errorHandler' );
 
                        $safeMode = wfIniGetBool( 'safe_mode' );
-                       foreach ( $dest as $recip ) {
+                       foreach ( $to as $recip ) {
                                if ( $safeMode ) {
                                        $sent = mail( $recip, self::quotedPrintable( $subject ), $body, $headers );
                                } else {