In this module's copy of the phpMailer library, it's coded to throw an exception and back out of sending the email if the SMTP server indicates that any of the intended recipients are invalid. This means that if you try to send an email to a list of 10 emails, and 1 is invalid, the email is not sent.
This seems like incorrect behavior. If I send an email from any other email client (gmail, Outlook, Mail app on phone, etc) and it contains one invalid email in list of valid emails, the email is still sent.
In fact, in the latest version of the phpMailer library, this behavior is corrected.
Here's the code snippet that this module is using now:
if (count($bad_rcpt) > 0 ) { //Create error message for any bad addresses
$badaddresses = implode(', ', $bad_rcpt);
throw new phpmailerException(t('SMTP Error: The following recipients failed: @bad', array('@bad' => $badaddresses)));
}
if (!$this->smtp->Data($header . $body)) {
throw new phpmailerException(t('SMTP Error: Data not accepted.'), self::STOP_CRITICAL);
}
And here's how it's handled in the latest version of phpMailer:
// Only send the DATA command if we have viable recipients
if ((count($this->all_recipients) > count($bad_rcpt)) and !$this->smtp->data($header . $body)) {
throw new Exception($this->lang('data_not_accepted'), self::STOP_CRITICAL);
}
So you can see that it only throws an exception and backs out of sending the email if it determines that ALL recipients were invalid, but this module does that if ANY recipients are bad.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2919816-dont-throw-exception-for-invalid-emails.patch | 1.44 KB | bkosborne |
Comments
Comment #2
bkosborneNeeded this fix for the 7.x version, so here's the fix.
Basically it only throws an exception if ALL the recipients were invalid. Otherwise it will still send. If SOME of them were invalid, it logs which ones.
Comment #4
yannick perret+1.
I had the same problem, and found this issue.
The patch #2 for 7.x corrected the problem for us (thanks for sharing it): valid email targets get the mail, and an entry log is created with the faulty email adress (in our case a user's mail which is invalid, which prevents receiving an expiration mail sent to both user and admins).
Why the status is 'needs work'? The patch is simple.
Comment #5
bkosbornetest bot did that when last patch failed to run for some reason. re-queued.
Comment #6
solideogloria commentedThe tests failed because they were targeting the D8 branch. I queued the tests for D7.
Comment #7
solideogloria commentedComment #8
tr commentedNote that in D8, SMTP no longer includes its own version of PHPMailer. Instead, it uses the latest version of PHPMailer installed using Composer.
The issue summary says "In fact, in the latest version of the phpMailer library, this behavior is corrected.", which tells me that this is an issue that now affects ONLY the D7 version of SMTP, and does not need to be addressed in D8 anymore.
Comment #9
solideogloria commentedUpdated the issue summary
Comment #10
solideogloria commentedThe patch still applies to dev. The code looks good to me, but I don't know how to test it.
Comment #11
tr commentedThe issue summary states that the current version of PHPMailer "only throws an exception and backs out of sending the email if it determines that ALL recipients were invalid" and shows the following code to support that conclusion:
I verified that this code snippet is accurate and appears in PHPMailer 6.1.7, which is the current version.
However, looking at the code, I don't agree with the interpretation. The code compares the count of all_recipients with the count of bad_rcpt. This code does NOT throw an exception only when ALL recipients are bad recipients, it is using a > to compare instead of ==. The patch uses ==, which is different that the current version of PHPMailer.
So I don't really understand the patch.
I think we should be doing the same thing as PHPMailer here. If #2295773: D7.x: Update PHPMailer to v6.0 is committed, we won't need to do anything here, but that issue may represent too big a change for this module as D7 support is really maintenance-only at this point and that would mean a pretty big change for a pretty big installed base.
Comment #12
solideogloria commentedAgreed. The IF(ALL > BAD) code you pointed out is the same as saying IF(ALL !== BAD), meaning that as long as there is at least one viable recipient, it will send the data.
Comment #13
damienmckennaComment #14
joseph.olstadComment #15
bluegeek9 commentedUnfortunately, Drupal 7 is End of Life and no longer supported. We strongly encourage you to upgrade to a supported version of Drupal.