diff --git a/src/applications/audit/PhabricatorAuditReplyHandler.php b/src/applications/audit/PhabricatorAuditReplyHandler.php index 203b6c5b7c..32393c7e80 100644 --- a/src/applications/audit/PhabricatorAuditReplyHandler.php +++ b/src/applications/audit/PhabricatorAuditReplyHandler.php @@ -49,7 +49,7 @@ final class PhabricatorAuditReplyHandler extends PhabricatorMailReplyHandler { } } - public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { $commit = $this->getMailReceiver(); $actor = $this->getActor(); diff --git a/src/applications/differential/DifferentialReplyHandler.php b/src/applications/differential/DifferentialReplyHandler.php index 3104c04b26..fc935b7691 100644 --- a/src/applications/differential/DifferentialReplyHandler.php +++ b/src/applications/differential/DifferentialReplyHandler.php @@ -105,7 +105,7 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler { return $actions; } - public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { $this->receivedMail = $mail; $this->handleAction($mail->getCleanTextBody()); } diff --git a/src/applications/maniphest/ManiphestReplyHandler.php b/src/applications/maniphest/ManiphestReplyHandler.php index 52935b44f7..d1959dd23e 100644 --- a/src/applications/maniphest/ManiphestReplyHandler.php +++ b/src/applications/maniphest/ManiphestReplyHandler.php @@ -50,7 +50,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler { } } - public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { // NOTE: We'll drop in here on both the "reply to a task" and "create a // new task" workflows! Make sure you test both if you make changes! diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index c15057e929..3c74108c3e 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -45,7 +45,82 @@ abstract class PhabricatorMailReplyHandler { PhabricatorObjectHandle $handle); abstract public function getReplyHandlerDomain(); abstract public function getReplyHandlerInstructions(); - abstract public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail); + abstract protected function receiveEmail( + PhabricatorMetaMTAReceivedMail $mail); + + public function processEmail(PhabricatorMetaMTAReceivedMail $mail) { + $error = $this->sanityCheckEmail($mail); + + if ($error) { + if ($this->shouldSendErrorEmail($mail)) { + $this->sendErrorEmail($error, $mail); + } + return null; + } + + return $this->receiveEmail($mail); + } + + private function sanityCheckEmail(PhabricatorMetaMTAReceivedMail $mail) { + $body = $mail->getCleanTextBody(); + if (empty($body)) { + return 'Empty email body. Email should begin with an !action and / or '. + 'text to comment. Inline replies and signatures are ignored.'; + } + + return null; + } + + /** + * Only send an error email if the user is talking to just Phabricator. We + * can assume if there is only one To address it is a Phabricator address + * since this code is running and everything. + */ + private function shouldSendErrorEmail(PhabricatorMetaMTAReceivedMail $mail) { + return count($mail->getToAddresses() == 1) && + count($mail->getCCAddresses() == 0); + } + + private function sendErrorEmail($error, + PhabricatorMetaMTAReceivedMail $mail) { + $template = new PhabricatorMetaMTAMail(); + $template->setSubject('Exception: unable to process your mail request'); + $template->setBody($this->buildErrorMailBody($error, $mail)); + $template->setRelatedPHID($mail->getRelatedPHID()); + $phid = $this->getActor()->getPHID(); + $tos = array( + $phid => PhabricatorObjectHandleData::loadOneHandle($phid) + ); + $mails = $this->multiplexMail($template, $tos, array()); + + foreach ($mails as $email) { + $email->saveAndSend(); + } + + return true; + } + + private function buildErrorMailBody($error, + PhabricatorMetaMTAReceivedMail $mail) { + $original_body = $mail->getRawTextBody(); + + $main_body = <<addRawSection($main_body); + $body->addReplySection($this->getReplyHandlerInstructions()); + + return $body->render(); + } public function supportsPrivateReplies() { return (bool)$this->getReplyHandlerDomain() && diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php index 4a05119952..59736d0538 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php @@ -54,6 +54,71 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return idx($this->headers, 'subject'); } + public function getCCAddresses() { + return $this->getRawEmailAddresses(idx($this->headers, 'cc')); + } + + public function getToAddresses() { + return $this->getRawEmailAddresses(idx($this->headers, 'to')); + } + + /** + * Parses "to" addresses, looking for a public create email address + * first and if not found parsing the "to" address for reply handler + * information: receiver name, user id, and hash. + */ + private function getPhabricatorToInformation() { + // Only one "public" create address so far + $create_task = PhabricatorEnv::getEnvConfig( + 'metamta.maniphest.public-create-email'); + + // For replies, look for an object address with a format like: + // D291+291+b0a41ca848d66dcc@example.com + $single_handle_prefix = PhabricatorEnv::getEnvConfig( + 'metamta.single-reply-handler-prefix'); + + $prefixPattern = ($single_handle_prefix) + ? preg_quote($single_handle_prefix, '/') . '\+' + : ''; + $pattern = "/^{$prefixPattern}((?:D|T|C)\d+)\+([\w]+)\+([a-f0-9]{16})@/U"; + + $phabricator_address = null; + $receiver_name = null; + $user_id = null; + $hash = null; + foreach ($this->getToAddresses() as $address) { + if ($address == $create_task) { + $phabricator_address = $address; + // it's okay to stop here because we just need to map a create + // address to an application and don't need / won't have more + // information in these cases. + break; + } + + $matches = null; + $ok = preg_match( + $pattern, + $address, + $matches); + + if ($ok) { + $phabricator_address = $address; + $receiver_name = $matches[1]; + $user_id = $matches[2]; + $hash = $matches[3]; + break; + } + } + + return array( + $phabricator_address, + $receiver_name, + $user_id, + $hash + ); + } + + public function processReceivedMail() { // If Phabricator sent the mail, always drop it immediately. This prevents @@ -68,11 +133,19 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return $this->setMessage($message)->save(); } - $to = idx($this->headers, 'to'); - $to = $this->getRawEmailAddress($to); + list($to, + $receiver_name, + $user_id, + $hash) = $this->getPhabricatorToInformation(); + if (!$to) { + $raw_to = idx($this->headers, 'to'); + return $this->setMessage("Unrecognized 'to' format: {$raw_to}")->save(); + } $from = idx($this->headers, 'from'); + // TODO -- make this a switch statement / better if / when we add more + // public create email addresses! $create_task = PhabricatorEnv::getEnvConfig( 'metamta.maniphest.public-create-email'); @@ -112,7 +185,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { $handler = $editor->buildReplyHandler($receiver); $handler->setActor($user); - $handler->receiveEmail($this); + $handler->processEmail($this); $this->setRelatedPHID($receiver->getPHID()); $this->setMessage('OK'); @@ -120,30 +193,6 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return $this->save(); } - // We've already stripped this, so look for an object address which has - // a format like: D291+291+b0a41ca848d66dcc@example.com - $matches = null; - $single_handle_prefix = PhabricatorEnv::getEnvConfig( - 'metamta.single-reply-handler-prefix'); - - $prefixPattern = ($single_handle_prefix) - ? preg_quote($single_handle_prefix, '/') . '\+' - : ''; - $pattern = "/^{$prefixPattern}((?:D|T|C)\d+)\+([\w]+)\+([a-f0-9]{16})@/U"; - - $ok = preg_match( - $pattern, - $to, - $matches); - - if (!$ok) { - return $this->setMessage("Unrecognized 'to' format: {$to}")->save(); - } - - $receiver_name = $matches[1]; - $user_id = $matches[2]; - $hash = $matches[3]; - if ($user_id == 'public') { if (!PhabricatorEnv::getEnvConfig('metamta.public-replies')) { return $this->setMessage("Public replies not enabled.")->save(); @@ -208,7 +257,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { } $handler->setActor($user); - $handler->receiveEmail($this); + $handler->processEmail($this); $this->setMessage('OK'); @@ -287,6 +336,14 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { return $address; } + private function getRawEmailAddresses($addresses) { + $raw_addresses = array(); + foreach (explode(',', $addresses) as $address) { + $raw_addresses[] = $this->getRawEmailAddress($address); + } + return $raw_addresses; + } + private function lookupPublicUser() { $from = idx($this->headers, 'from'); $from = $this->getRawEmailAddress($from); diff --git a/src/applications/owners/OwnersPackageReplyHandler.php b/src/applications/owners/OwnersPackageReplyHandler.php index 4c21181cc0..6878f5ab18 100644 --- a/src/applications/owners/OwnersPackageReplyHandler.php +++ b/src/applications/owners/OwnersPackageReplyHandler.php @@ -40,7 +40,7 @@ final class OwnersPackageReplyHandler extends PhabricatorMailReplyHandler { return null; } - public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { return; } }