Respect "metamta.email-body-limit" when building mail HTML bodies

Summary:
Ref T13151. See T11767. See PHI686. Although we limit outbound mail text bodies, the limit doesn't currently apply to attachments, HTML bodies, or headers. T11767 discusses improving this in the general case.

In the wild, an install hit an issue (see PHI686) where edits to Phriction pages generate very large HTML bodies. Check and respect the limit when building HTML bodies.

If we don't have enough room for the HTML body, we just drop it. We have the text body to fall back to, and HTML is difficult to truncate safely.

Test Plan: Added unit tests and made them pass.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13151

Differential Revision: https://secure.phabricator.com/D19489

src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php

@@ -123,4 +123,12 @@ final class PhabricatorMailImplementationTestAdapter
     return $this;
   }
 
+  public function getBody() {
+    return idx($this->guts, 'body');
+  }
+
+  public function getHTMLBody() {
+    return idx($this->guts, 'html-body');
+  }
+
 }

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -892,15 +892,16 @@ final class PhabricatorMetaMTAMail
       $body = $raw_body;
     }
 
-    $max = PhabricatorEnv::getEnvConfig('metamta.email-body-limit');
+    $body_limit = PhabricatorEnv::getEnvConfig('metamta.email-body-limit');
-    if (strlen($body) > $max) {
+    if (strlen($body) > $body_limit) {
       $body = id(new PhutilUTF8StringTruncator())
-        ->setMaximumBytes($max)
+        ->setMaximumBytes($body_limit)
         ->truncateString($body);
       $body .= "\n";
-      $body .= pht('(This email was truncated at %d bytes.)', $max);
+      $body .= pht('(This email was truncated at %d bytes.)', $body_limit);
     }
     $mailer->setBody($body);
+    $body_limit -= strlen($body);
 
     // If we sent a different message body than we were asked to, record
     // what we actually sent to make debugging and diagnostics easier.
@@ -914,8 +915,16 @@ final class PhabricatorMetaMTAMail
       $send_html = $this->shouldSendHTML($preferences);
     }
 
-    if ($send_html && isset($params['html-body'])) {
+    if ($send_html) {
-      $mailer->setHTMLBody($params['html-body']);
+      $html_body = idx($params, 'html-body');
+
+      // NOTE: We just drop the entire HTML body if it won't fit. Safely
+      // truncating HTML is hard, and we already have the text body to fall
+      // back to.
+      if (strlen($html_body) <= $body_limit) {
+        $mailer->setHTMLBody($html_body);
+        $body_limit -= strlen($html_body);
+      }
     }
 
     // Pass the headers to the mailer, then save the state so we can show

src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php

@@ -331,4 +331,84 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
     $this->assertEqual(null, $mail->getMailerKey());
   }
 
+  public function testMailSizeLimits() {
+    $env = PhabricatorEnv::beginScopedEnv();
+    $env->overrideEnvConfig('metamta.email-body-limit', 1024 * 512);
+
+    $user = $this->generateNewTestUser();
+    $phid = $user->getPHID();
+
+    $string_1kb = str_repeat('x', 1024);
+    $html_1kb = str_repeat('y', 1024);
+    $string_1mb = str_repeat('x', 1024 * 1024);
+    $html_1mb = str_repeat('y', 1024 * 1024);
+
+    // First, send a mail with a small text body and a small HTML body to make
+    // sure the basics work properly.
+    $mail = id(new PhabricatorMetaMTAMail())
+      ->addTos(array($phid))
+      ->setBody($string_1kb)
+      ->setHTMLBody($html_1kb);
+
+    $mailer = new PhabricatorMailImplementationTestAdapter();
+    $mail->sendWithMailers(array($mailer));
+    $this->assertEqual(
+      PhabricatorMailOutboundStatus::STATUS_SENT,
+      $mail->getStatus());
+
+    $text_body = $mailer->getBody();
+    $html_body = $mailer->getHTMLBody();
+
+    $this->assertEqual($string_1kb, $text_body);
+    $this->assertEqual($html_1kb, $html_body);
+
+
+    // Now, send a mail with a large text body and a large HTML body. We expect
+    // the text body to be truncated and the HTML body to be dropped.
+    $mail = id(new PhabricatorMetaMTAMail())
+      ->addTos(array($phid))
+      ->setBody($string_1mb)
+      ->setHTMLBody($html_1mb);
+
+    $mailer = new PhabricatorMailImplementationTestAdapter();
+    $mail->sendWithMailers(array($mailer));
+    $this->assertEqual(
+      PhabricatorMailOutboundStatus::STATUS_SENT,
+      $mail->getStatus());
+
+    $text_body = $mailer->getBody();
+    $html_body = $mailer->getHTMLBody();
+
+    // We expect the body was truncated, because it exceeded the body limit.
+    $this->assertTrue(
+      (strlen($text_body) < strlen($string_1mb)),
+      pht('Text Body Truncated'));
+
+    // We expect the HTML body was dropped completely after the text body was
+    // truncated.
+    $this->assertTrue(
+      !strlen($html_body),
+      pht('HTML Body Removed'));
+
+
+    // Next send a mail with a small text body and a large HTML body. We expect
+    // the text body to be intact and the HTML body to be dropped.
+    $mail = id(new PhabricatorMetaMTAMail())
+      ->addTos(array($phid))
+      ->setBody($string_1kb)
+      ->setHTMLBody($html_1mb);
+
+    $mailer = new PhabricatorMailImplementationTestAdapter();
+    $mail->sendWithMailers(array($mailer));
+    $this->assertEqual(
+      PhabricatorMailOutboundStatus::STATUS_SENT,
+      $mail->getStatus());
+
+    $text_body = $mailer->getBody();
+    $html_body = $mailer->getHTMLBody();
+
+    $this->assertEqual($string_1kb, $text_body);
+    $this->assertTrue(!strlen($html_body));
+  }
+
 }