From 942b17a980888b09980217735b12c9dd583a3b8c Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 Feb 2018 08:57:07 -0800 Subject: [PATCH] Improve correctness of email address escaping in Mailgun/Postmark Summary: Ref T13053. Uses the changes in D19026 to escape mail addresses. Those rules may not be right yet, but they're at least all in one place, have test coverage, and aren't obviously incorrect. Test Plan: Will vet this more extensively when re-testing all mailers. Maniphest Tasks: T13053 Differential Revision: https://secure.phabricator.com/D19027 --- .../adapter/PhabricatorMailImplementationAdapter.php | 5 +++-- .../PhabricatorMailImplementationMailgunAdapter.php | 9 +++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php index ce56345194..dfbe891651 100644 --- a/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php @@ -96,8 +96,9 @@ abstract class PhabricatorMailImplementationAdapter extends Phobject { protected function renderAddress($email, $name = null) { if (strlen($name)) { - // TODO: This needs to be escaped correctly. - return "{$name} <{$email}>"; + return (string)id(new PhutilEmailAddress()) + ->setDisplayName($name) + ->setAddress($email); } else { return $email; } diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php index bed0dada63..12c54e0d6a 100644 --- a/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php @@ -21,7 +21,7 @@ final class PhabricatorMailImplementationMailgunAdapter if (empty($this->params['reply-to'])) { $this->params['reply-to'] = array(); } - $this->params['reply-to'][] = "{$name} <{$email}>"; + $this->params['reply-to'][] = $this->renderAddress($name, $email); return $this; } @@ -110,11 +110,8 @@ final class PhabricatorMailImplementationMailgunAdapter } $from = idx($this->params, 'from'); - if (idx($this->params, 'from-name')) { - $params['from'] = "\"{$this->params['from-name']}\" <{$from}>"; - } else { - $params['from'] = $from; - } + $from_name = idx($this->params, 'from-name'); + $params['from'] = $this->renderAddress($from, $from_name); if (idx($this->params, 'reply-to')) { $replyto = $this->params['reply-to'];