Improve rules for embedding files received via email

Summary:
Ref T7199. Ref T7712. This improves the file rules for email:

  - Embed visible images as thumbnails.
  - Put all other file types in a nice list.

This "fixes" an issue caused by the opposite of the problem described in T7712 -- files being dropped if the default ruleset is too restrictive. T7712 is the real solution here, but use a half-measure for now.

Test Plan:
  - Sent mail with two non-images and two images.
  - Got a nice list of non-images and embeds of images.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7712, T7199

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

src/applications/conpherence/mail/ConpherenceReplyHandler.php

@@ -58,11 +58,7 @@ final class ConpherenceReplyHandler extends PhabricatorMailReplyHandler {
       ->setParentMessageID($mail->getMessageID());
 
     $body = $mail->getCleanTextBody();
-    $file_phids = $mail->getAttachments();
-    $body = $this->enhanceBodyWithAttachments(
-      $body,
-      $file_phids,
-      '{F%d}');
+    $body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
 
     $xactions = array();
     if ($this->getMailAddedParticipantPHIDs()) {

src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php

@@ -319,30 +319,56 @@ abstract class PhabricatorMailReplyHandler {
     return $this->getSingleReplyHandlerPrefix($address);
   }
 
- final protected function enhanceBodyWithAttachments(
+  final protected function enhanceBodyWithAttachments(
     $body,
-    array $attachments,
-    $format = '- {F%d, layout=link}') {
+    array $attachments) {
+
     if (!$attachments) {
       return $body;
     }
 
+    // NOTE: This is safe, but not entirely correct. Clean it up after
+    // T7712. These files have the install-default policy right now, which
+    // may or may not be permissive.
     $files = id(new PhabricatorFileQuery())
-      ->setViewer($this->getActor())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withPHIDs($attachments)
       ->execute();
 
-    // if we have some text then double return before adding our file list
-    if ($body) {
-      $body .= "\n\n";
-    }
+    $output = array();
+    $output[] = $body;
 
+    // We're going to put all the non-images first in a list, then embed
+    // the images.
+    $head = array();
+    $tail = array();
     foreach ($files as $file) {
-      $file_str = sprintf($format, $file->getID());
-      $body .= $file_str."\n";
+      if ($file->isViewableImage()) {
+        $tail[] = $file;
+      } else {
+        $head[] = $file;
+      }
     }
 
-    return rtrim($body);
+    if ($head) {
+      $list = array();
+      foreach ($head as $file) {
+        $list[] = '  - {'.$file->getMonogram().', layout=link}';
+      }
+      $output[] = implode("\n", $list);
+    }
+
+    if ($tail) {
+      $list = array();
+      foreach ($tail as $file) {
+        $list[] = '{'.$file->getMonogram().'}';
+      }
+      $output[] = implode("\n\n", $list);
+    }
+
+    $output = implode("\n\n", $output);
+
+    return rtrim($output);
   }
 
   private function expandRecipientHandles(array $handles) {