From 6fb3f857fb57bd902b826567d38c07eb131b008b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 12 Sep 2017 15:32:15 -0700 Subject: [PATCH] Stop the bleeding caused by attaching enormous patches to revision mail Summary: Ref T12033. This is a very narrow fix for this issue, but it should fix the major error: don't attach patches if they're bigger than the mail body limit (by default, 512KB). Specifically, the logs from an install in T12033 show a 112MB patch being attached, and that's the biggest practical problem here. I'll follow up on the tasks with more nuanced future work. Test Plan: Enabled `differential.attach-patches`, saw a patch attached to email. Set the byte limit very low, saw patches get thrown away. Reviewers: chad, amckinley Reviewed By: amckinley Maniphest Tasks: T12033 Differential Revision: https://secure.phabricator.com/D18598 --- .../editor/DifferentialTransactionEditor.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index e916728c86..c6270013aa 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -766,10 +766,14 @@ final class DifferentialTransactionEditor } if ($config_attach) { - $name = pht('D%s.%s.patch', $object->getID(), $diff->getID()); - $mime_type = 'text/x-patch; charset=utf-8'; - $body->addAttachment( - new PhabricatorMetaMTAAttachment($patch, $name, $mime_type)); + // See T12033, T11767, and PHI55. This is a crude fix to stop the + // major concrete problems that lackluster email size limits cause. + if (strlen($patch) < $body_limit) { + $name = pht('D%s.%s.patch', $object->getID(), $diff->getID()); + $mime_type = 'text/x-patch; charset=utf-8'; + $body->addAttachment( + new PhabricatorMetaMTAAttachment($patch, $name, $mime_type)); + } } } }