From fea2389066edf3ad0a7547ae12d8e988428a4f5c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 2 Apr 2016 10:34:13 -0700
Subject: [PATCH] (stable) Improve error and header behaviors for Mailgun
 received mail webhook

Summary:
Ref T10709. Two issues:

  - If a user sends an invalid `!command`, we can throw, which means we don't return HTTP 200. This makes Mailgun re-send the mail later.
  - We don't parse headers of the modern API correctly, so the "Message-ID" failsafe doesn't work. Parse them correctly. I //believe// Mailgun's API changed at some point.

Test Plan:
This is difficult to test exhaustively in isolation. I used Mailgun's web tools to verify the format of the hook request, and faked some requests locally.

I'll keep an eye on this as it goes to production and make sure the fix is correct there.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10709

Differential Revision: https://secure.phabricator.com/D15575
---
 ...ricatorMetaMTAMailgunReceiveController.php | 34 ++++++++++++++-----
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php
index af5ca34712..467995a186 100644
--- a/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php
+++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php
@@ -28,14 +28,14 @@ final class PhabricatorMetaMTAMailgunReceiveController
         pht('Mail signature is not valid. Check your Mailgun API key.'));
     }
 
-    $user = $request->getUser();
-
-    $raw_headers = $request->getStr('headers');
-    $raw_headers = explode("\n", rtrim($raw_headers));
+    $raw_headers = $request->getStr('message-headers');
     $raw_dict = array();
-    foreach (array_filter($raw_headers) as $header) {
-      list($name, $value) = explode(':', $header, 2);
-      $raw_dict[$name] = ltrim($value);
+    if (strlen($raw_headers)) {
+      $raw_headers = phutil_json_decode($raw_headers);
+      foreach ($raw_headers as $raw_header) {
+        list($name, $value) = $raw_header;
+        $raw_dict[$name] = $value;
+      }
     }
 
     $headers = array(
@@ -65,9 +65,25 @@ final class PhabricatorMetaMTAMailgunReceiveController
       }
     }
     $received->setAttachments($file_phids);
-    $received->save();
 
-    $received->processReceivedMail();
+    try {
+      $received->save();
+      $received->processReceivedMail();
+    } catch (Exception $ex) {
+      // We can get exceptions here in two cases.
+
+      // First, saving the message may throw if we have already received a
+      // message with the same Message ID. In this case, we're declining to
+      // process a duplicate message, so failing silently is correct.
+
+      // Second, processing the message may throw (for example, if it contains
+      // an invalid !command). This will generate an email as a side effect,
+      // so we don't need to explicitly handle the exception here.
+
+      // In these cases, we want to return HTTP 200. If we do not, MailGun will
+      // re-transmit the message later.
+      phlog($ex);
+    }
 
     $response = new AphrontWebpageResponse();
     $response->setContent(pht("Got it! Thanks, Mailgun!\n"));