Abstract case:
- Open any Maniphest task
- Click "Subscribe"
- Getting notification to email, open it
- There are duplicate entry of e-mail in the section "To:"
Concrete case:
{F112062}
Description
Description
Revisions and Commits
Revisions and Commits
| rP Phabricator | |||
| D8288 | rP67326bb47a0d Fix phpmailer-lite bug | ||
Event Timeline
Comment Actions
This actually reproduces on other objects like a Paste (and for every differential email I've gotten the past bit, etc.) and looking through my email, it has been happening since 8/22/2012....!
For example, I made and edited P1071, which sent an email to me after the edit. Running bin/mail, I see the following:
PROPERTIES ID: 3547 Status: sent Related PHID: PHID-PSTE-qob4sipwgyjatsiryj7e Message: PARAMETERS subject: P139: test paste test test from: PHID-USER-xee4ju2teq7mflitwfcs subject-prefix: [Paste] vary-subject-prefix: [Updated] thread-id: PHID-PSTE-qob4sipwgyjatsiryj7e is-first-message: mailtags: [] is-bulk: 1 to: ["PHID-USER-xee4ju2teq7mflitwfcs"] reply-to: P139+1+2fdd496bee3c855f@phabricator.dev worker-task: 18045 HEADERS Thread-Topic: P139 X-Phabricator-To: <PHID-USER-xee4ju2teq7mflitwfcs> RECIPIENTS btrahan (Bob Trahan) BODY btrahan updated the paste's title to "test paste test test". To: btrahan
This basically looks good. If I look at the raw headers, I see the following
Delivered-To: bob.trahan@gmail.com
Received: by 10.170.42.14 with SMTP id 14csp68383ykk;
Thu, 20 Feb 2014 16:06:30 -0800 (PST)
X-Received: by 10.60.16.168 with SMTP id h8mr5370749oed.32.1392941190470;
Thu, 20 Feb 2014 16:06:30 -0800 (PST)
Return-Path: <0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@amazonses.com>
Received: from a8-24.smtp-out.amazonses.com (a8-24.smtp-out.amazonses.com. [54.240.8.24])
by mx.google.com with ESMTP id jb8si1305198obb.53.2014.02.20.16.06.30
for <bob.trahan@gmail.com>;
Thu, 20 Feb 2014 16:06:30 -0800 (PST)
Received-SPF: pass (google.com: domain of 0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@amazonses.com designates 54.240.8.24 as permitted sender) client-ip=54.240.8.24;
Authentication-Results: mx.google.com;
spf=pass (google.com: domain of 0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@amazonses.com designates 54.240.8.24 as permitted sender) smtp.mail=0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@amazonses.com
To: bob.trahan@gmail.com
Date: Fri, 21 Feb 2014 00:06:29 +0000
Return-Path: 0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@amazonses.com
To: bob.trahan@gmail.com
From: "btrahan (Bob Trahan)" <noreply@phabricator.com>
Reply-to: P1071+53+5181b9e6de790cb3@phabricator.com
Subject: [Paste] [Updated] P1071: test paste test
Message-ID: <0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@email.amazonses.com>
X-Priority: 3
Thread-Topic: P1071
X-Phabricator-To: <PHID-USER-f1ca174bb6af9391a5f7>
In-Reply-To: <PHID-PSTE-bhbgug5sx7xgkefhmcan@phabricator.com>
References: <PHID-PSTE-bhbgug5sx7xgkefhmcan@phabricator.com>
Thread-Index: MTk5YmMxYTYzZmI4NjJiMWU3NzBhNjc0OTdiIFMGmIU=
X-Phabricator-Sent-This-Message: Yes
X-Mail-Transport-Agent: MetaMTA
X-Auto-Response-Suppress: All
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
X-SES-Outgoing: 2014.02.21-54.240.8.24This does not look good. In particular, these lines
To: bob.trahan@gmail.com Date: Fri, 21 Feb 2014 00:06:29 +0000 Return-Path: 0000014451c3c9c5-0e0b7efc-3fc8-4e1d-b963-6244bcab924f-000000@amazonses.com To: bob.trahan@gmail.com
...which show the "To:" header being repeated twice.
Using the time analysis, the only diff I think could have caused this is D3365 which happened on 8/22/2012.
Poking around in phpmailer, this should fix it, though its hacktastic
diff --git a/externals/phpmailer/class.phpmailer-lite.php b/externals/phpmailer/class.phpmailer-lite.php index 824878d..c064d6c 100644 --- a/externals/phpmailer/class.phpmailer-lite.php +++ b/externals/phpmailer/class.phpmailer-lite.php @@ -491,13 +491,7 @@ class PHPMailerLite { switch($this->Mailer) { case 'amazon-ses': - $toArr = array(); - foreach($this->to as $t) { - $toArr[] = $this->AddrFormat($t); - } - $to = implode(', ', $toArr); return $this->customMailer->executeSend( - "To: ".$to."\n". $header. $body);
Basically, $header is already getting "To" populated via CreateHeader, so I think this additional 'To" action was unnecessary and the code + headers give me confidence this is the spot this is breaking. The "hacktastic" part though is while the D3365 change means we don't ever use the SingleTo codepath, I am pretty sure it is now horribly broken by my change.
I propose putting my patch in production, seeing if it fixes it, and then remembering this is all a giant mess?
Also of note looks like phpmailer has gotten lots of changes over the years (https://github.com/PHPMailer/PHPMailer) but part of that seems to be removing the "lite"version. An alternative fix would be to upgrade this library, though I suspect that'll be lots of work to patch things how we need it a la D3365.
Comment Actions
That analysis seems reasonable to me. I'm onboard with the hack-patch. We definitely should never be using the "SingleTo" path.
Comment Actions
(We also ship with both the full-size mailer and the lite mailer right now, and updating them and getting rid of the lite flavor would be nice some day, but that seems like it could be a lot of work. We've also patched some security stuff in both of them.)
Comment Actions
This was vaguely annoying me for like a year, too, so I am slightly pleased to see it fixed.