diff --git a/src/applications/differential/mail/DifferentialMail.php b/src/applications/differential/mail/DifferentialMail.php index 7cebc79069..b54b799737 100644 --- a/src/applications/differential/mail/DifferentialMail.php +++ b/src/applications/differential/mail/DifferentialMail.php @@ -110,15 +110,26 @@ abstract class DifferentialMail { 'X-Differential-Author', '<'.$revision->getAuthorPHID().'>'); } - if ($revision->getReviewers()) { + + $reviewer_phids = $revision->getReviewers(); + if ($reviewer_phids) { + // Add several headers to support e-mail clients which are not able to + // create rules using regular expressions or wildcards (namely Outlook). + $template->addPHIDHeaders('X-Differential-Reviewer', $reviewer_phids); + + // Add it also as a list to allow matching of the first reviewer and + // also for backwards compatibility. $template->addHeader( 'X-Differential-Reviewers', - '<'.implode('>, <', $revision->getReviewers()).'>'); + '<'.implode('>, <', $reviewer_phids).'>'); } - if ($revision->getCCPHIDs()) { + + $cc_phids = $revision->getCCPHIDs(); + if ($cc_phids) { + $template->addPHIDHeaders('X-Differential-CC', $cc_phids); $template->addHeader( 'X-Differential-CCs', - '<'.implode('>, <', $revision->getCCPHIDs()).'>'); + '<'.implode('>, <', $cc_phids).'>'); // Determine explicit CCs (those added by humans) and put them in a // header so users can differentiate between Herald CCs and human CCs. @@ -142,6 +153,7 @@ abstract class DifferentialMail { } if ($explicit_cc) { + $template->addPHIDHeaders('X-Differential-Explicit-CC', $explicit_cc); $template->addHeader( 'X-Differential-Explicit-CCs', '<'.implode('>, <', $explicit_cc).'>'); diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index 00bfea095e..8c78a513f9 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -118,24 +118,24 @@ abstract class PhabricatorMailReplyHandler { } } + $tos = mpull($to_handles, null, 'getPHID'); + $ccs = mpull($cc_handles, null, 'getPHID'); + // Merge all the recipients together. TODO: We could keep the CCs as real // CCs and send to a "noreply@domain.com" type address, but keep it simple // for now. - $recipients = mpull($to_handles, null, 'getPHID') + - mpull($cc_handles, null, 'getPHID'); + $recipients = $tos + $ccs; // When multiplexing mail, explicitly include To/Cc information in the // message body and headers. - $add_headers = array(); + + $mail_template = clone $mail_template; + + $mail_template->addPHIDHeaders('X-Phabricator-To', array_keys($tos)); + $mail_template->addPHIDHeaders('X-Phabricator-Cc', array_keys($ccs)); $body = $mail_template->getBody(); $body .= "\n"; - if ($to_handles) { - $add_headers['X-Phabricator-To'] = $this->formatPHIDList($to_handles); - } - if ($cc_handles) { - $add_headers['X-Phabricator-Cc'] = $this->formatPHIDList($cc_handles); - } $body .= $this->getRecipientsSummary($to_handles, $cc_handles); foreach ($recipients as $recipient) { @@ -143,9 +143,6 @@ abstract class PhabricatorMailReplyHandler { $mail->addTos(array($recipient->getPHID())); $mail->setBody($body); - foreach ($add_headers as $header => $value) { - $mail->addHeader($header, $value); - } $reply_to = null; if (!$reply_to && $this->supportsPrivateReplies()) { @@ -166,15 +163,6 @@ abstract class PhabricatorMailReplyHandler { return $result; } - protected function formatPHIDList(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $list = array(); - foreach ($handles as $handle) { - $list[] = '<'.$handle->getPHID().'>'; - } - return implode(', ', $list); - } - protected function getDefaultPublicReplyHandlerEmailAddress($prefix) { $receiver = $this->getMailReceiver(); diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index e189296885..d169febab2 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -141,8 +141,15 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return $return; } + public function addPHIDHeaders($name, array $phids) { + foreach ($phids as $phid) { + $this->addHeader($name, '<'.$phid.'>'); + } + return $this; + } + public function addHeader($name, $value) { - $this->parameters['headers'][$name] = $value; + $this->parameters['headers'][] = array($name, $value); return $this; } @@ -408,7 +415,9 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { } break; case 'headers': - foreach ($value as $header_key => $header_value) { + foreach ($value as $pair) { + list($header_key, $header_value) = $pair; + // NOTE: If we have \n in a header, SES rejects the email. $header_value = str_replace("\n", " ", $header_value); diff --git a/src/docs/userguide/mail_rules.diviner b/src/docs/userguide/mail_rules.diviner index 27079b93d7..b5c8253bb0 100644 --- a/src/docs/userguide/mail_rules.diviner +++ b/src/docs/userguide/mail_rules.diviner @@ -34,7 +34,7 @@ to write more sophisticated mail rules. Phabricator sends a variety of mail headers that can be useful in crafting rules to route and manage mail. -Many of these headers contain lists. A list containing two items, ##1## and +Headers in plural contain lists. A list containing two items, ##1## and ##15## will generally be formatted like this: X-Header: <1>, <15> @@ -43,13 +43,17 @@ The intent is to allow you to write a rule which matches against "<1>". If you just match against "1", you'll incorrectly match "15", but matching "<1>" will correctly match only "<1>". +Some other headers use a single value but can be presented multiple times. +It is to support e-mail clients which are not able to create rules using regular +expressions or wildcards (namely Outlook). + The headers Phabricator adds to mail are: - ##X-Phabricator-Sent-This-Message##: this is attached to all mail Phabricator sends. You can use it to differentiate between email from Phabricator and replies/forwards of Phabricator mail from human beings. - ##X-Phabricator-To##: this is attached to all mail Phabricator sends. - It this shows the PHIDs of the original "To" line, before any mutation + It shows the PHIDs of the original "To" line, before any mutation by the mailer configuration. - ##X-Phabricator-Cc##: this is attached to all mail Phabricator sends. It shows the PHIDs of the original "Cc" line, before any mutation by the @@ -57,15 +61,18 @@ The headers Phabricator adds to mail are: - ##X-Differential-Author##: this is attached to Differential mail and shows the revision's author. You can use it to filter mail about your revisions (or other users' revisions). - - ##X-Differential-Reviewers##: this is attached to Differential mail and + - ##X-Differential-Reviewer##: this is attached to Differential mail and shows the reviewers. You can use it to filter mail about revisions you are reviewing, versus revisions you are explicitly CC'd on or CC'd as a result of Herald rules. - - ##X-Differential-CCs##: this is attached to Differential mail and shows + - ##X-Differential-Reviewers##: list version of the previous. + - ##X-Differential-CC##: this is attached to Differential mail and shows the CCs on the revision. - - ##X-Differential-Explicit-CCs##: this is attached to Differential mail and + - ##X-Differential-CCs##: list version of the previous. + - ##X-Differential-Explicit-CC##: this is attached to Differential mail and shows the explicit CCs on the revision (those that were added by humans, not by Herald). + - ##X-Differential-Explicit-CCs##: list version of the previous. - ##X-Phabricator-Mail-Tags##: this is attached to some mail and has a list of descriptors about the mail. (This is fairly new and subject to some change.)