diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php @@ -123,4 +123,12 @@ return $this; } + public function getBody() { + return idx($this->guts, 'body'); + } + + public function getHTMLBody() { + return idx($this->guts, 'html-body'); + } + } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -892,15 +892,16 @@ $body = $raw_body; } - $max = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); - if (strlen($body) > $max) { + $body_limit = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); + 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 @@ $send_html = $this->shouldSendHTML($preferences); } - if ($send_html && isset($params['html-body'])) { - $mailer->setHTMLBody($params['html-body']); + if ($send_html) { + $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 diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php --- a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php +++ b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php @@ -331,4 +331,84 @@ $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)); + } + }