Convert inline profile image transforms to new transformations

Summary:
Ref T7707. Fixes T7879. Fixes T4406. When creating profile images:

  - Use the new transforms;
  - mark them as "profile" images so they're forced to the most-open policies.

Test Plan:
  - Set restrictive default file policies.
  - Changed profile picture, project pictures, etc. Verified they were visible to logged-out users.
  - Registered via OAuth.
  - Updated a Conpherence thread image.
  - Browsed around looking for profile images, fixed sizing on everything I could find.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7879, T7707, T4406

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

src/applications/auth/controller/PhabricatorAuthRegisterController.php

@@ -604,17 +604,9 @@ final class PhabricatorAuthRegisterController
       return null;
     }
 
-    try {
-      $xformer = new PhabricatorImageTransformer();
-      return $xformer->executeProfileTransform(
-        $file,
-        $width = 50,
-        $min_height = 50,
-        $max_height = 50);
-    } catch (Exception $ex) {
-      phlog($ex);
-      return null;
-    }
+    $xform = PhabricatorFileTransform::getTransformByKey(
+      PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE);
+    return $xform->executeTransform($file);
   }
 
   protected function renderError($message) {

src/applications/files/PhabricatorImageTransformer.php

@@ -20,22 +20,6 @@ final class PhabricatorImageTransformer {
       ));
   }
 
-  public function executeProfileTransform(
-    PhabricatorFile $file,
-    $x,
-    $min_y,
-    $max_y) {
-
-    $image = $this->crudelyCropTo($file, $x, $min_y, $max_y);
-
-    return PhabricatorFile::newFromFileData(
-      $image,
-      array(
-        'name' => 'profile-'.$file->getName(),
-        'canCDN' => true,
-      ));
-  }
-
   public function executeConpherenceTransform(
     PhabricatorFile $file,
     $top,
@@ -54,38 +38,11 @@ final class PhabricatorImageTransformer {
       $image,
       array(
         'name' => 'conpherence-'.$file->getName(),
+        'profile' => true,
         'canCDN' => true,
       ));
   }
 
-  private function crudelyCropTo(PhabricatorFile $file, $x, $min_y, $max_y) {
-    $data = $file->loadFileData();
-    $img = imagecreatefromstring($data);
-    $sx = imagesx($img);
-    $sy = imagesy($img);
-
-    $scaled_y = ($x / $sx) * $sy;
-    if ($scaled_y > $max_y) {
-      // This image is very tall and thin.
-      $scaled_y = $max_y;
-    } else if ($scaled_y < $min_y) {
-      // This image is very short and wide.
-      $scaled_y = $min_y;
-    }
-
-    $cropped = $this->applyScaleWithImagemagick($file, $x, $scaled_y);
-    if ($cropped != null) {
-      return $cropped;
-    }
-
-    $img = $this->applyScaleTo(
-      $file,
-      $x,
-      $scaled_y);
-
-    return self::saveImageDataInAnyFormat($img, $file->getMimeType());
-  }
-
   private function crasslyCropTo(PhabricatorFile $file, $top, $left, $w, $h) {
     $data = $file->loadFileData();
     $src = imagecreatefromstring($data);
@@ -116,34 +73,6 @@ final class PhabricatorImageTransformer {
     return $dst;
   }
 
-  private function applyScaleTo(PhabricatorFile $file, $dx, $dy) {
-    $data = $file->loadFileData();
-    $src = imagecreatefromstring($data);
-
-    $x = imagesx($src);
-    $y = imagesy($src);
-
-    $scale = min(($dx / $x), ($dy / $y), 1);
-
-    $sdx = $scale * $x;
-    $sdy = $scale * $y;
-
-    $dst = $this->getBlankDestinationFile($dx, $dy);
-    imagesavealpha($dst, true);
-    imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127));
-
-    imagecopyresampled(
-      $dst,
-      $src,
-      ($dx - $sdx) / 2,  ($dy - $sdy) / 2,
-      0, 0,
-      $sdx, $sdy,
-      $x, $y);
-
-    return $dst;
-
-  }
-
   public static function getScaleForCrop(
     PhabricatorFile $file,
     $des_width,
@@ -302,49 +231,6 @@ final class PhabricatorImageTransformer {
     );
   }
 
-  private function applyScaleWithImagemagick(PhabricatorFile $file, $dx, $dy) {
-    $img_type = $file->getMimeType();
-    $imagemagick = PhabricatorEnv::getEnvConfig('files.enable-imagemagick');
-
-    if ($img_type != 'image/gif' || $imagemagick == false) {
-      return null;
-    }
-
-    $data = $file->loadFileData();
-    $src = imagecreatefromstring($data);
-
-    $x = imagesx($src);
-    $y = imagesy($src);
-
-    if (self::isEnormousGIF($x, $y)) {
-      return null;
-    }
-
-    $scale = min(($dx / $x), ($dy / $y), 1);
-
-    $sdx = $scale * $x;
-    $sdy = $scale * $y;
-
-    $input = new TempFile();
-    Filesystem::writeFile($input, $data);
-
-    $resized = new TempFile();
-
-    $future = new ExecFuture(
-      'convert %s -coalesce -resize %sX%s%s %s',
-      $input,
-      $sdx,
-      $sdy,
-      '!',
-      $resized);
-
-    // Don't spend more than 10 seconds resizing; just fail if it takes longer
-    // than that.
-    $future->setTimeout(10)->resolvex();
-
-    return Filesystem::readFile($resized);
-  }
-
   private function applyMemeWithImagemagick(
     $input,
     $above,
@@ -382,57 +268,6 @@ final class PhabricatorImageTransformer {
     return Filesystem::readFile($output);
   }
 
-/* -(  Detecting Enormous Files  )------------------------------------------- */
-
-
-  /**
-   * Determine if an image is enormous (too large to transform).
-   *
-   * Attackers can perform a denial of service attack by uploading highly
-   * compressible images with enormous dimensions but a very small filesize.
-   * Transforming them (e.g., into thumbnails) may consume huge quantities of
-   * memory and CPU relative to the resources required to transmit the file.
-   *
-   * In general, we respond to these images by declining to transform them, and
-   * using a default thumbnail instead.
-   *
-   * @param int Width of the image, in pixels.
-   * @param int Height of the image, in pixels.
-   * @return bool True if this image is enormous (too large to transform).
-   * @task enormous
-   */
-  public static function isEnormousImage($x, $y) {
-    // This is just a sanity check, but if we don't have valid dimensions we
-    // shouldn't be trying to transform the file.
-    if (($x <= 0) || ($y <= 0)) {
-      return true;
-    }
-
-    return ($x * $y) > (4096 * 4096);
-  }
-
-
-  /**
-   * Determine if a GIF is enormous (too large to transform).
-   *
-   * For discussion, see @{method:isEnormousImage}. We need to be more
-   * careful about GIFs, because they can also have a large number of frames
-   * despite having a very small filesize. We're more conservative about
-   * calling GIFs enormous than about calling images in general enormous.
-   *
-   * @param int Width of the GIF, in pixels.
-   * @param int Height of the GIF, in pixels.
-   * @return bool True if this image is enormous (too large to transform).
-   * @task enormous
-   */
-  public static function isEnormousGIF($x, $y) {
-    if (self::isEnormousImage($x, $y)) {
-      return true;
-    }
-
-    return ($x * $y) > (800 * 800);
-  }
-
 
 /* -(  Saving Image Data  )-------------------------------------------------- */
 

src/applications/files/controller/PhabricatorFileComposeController.php

@@ -58,7 +58,7 @@ final class PhabricatorFileComposeController
       }
 
       $root = dirname(phutil_get_library_root('phabricator'));
-      $icon_file = $root.'/resources/sprite/projects_1x/'.$icon.'.png';
+      $icon_file = $root.'/resources/sprite/projects_2x/'.$icon.'.png';
       $icon_data = Filesystem::readFile($icon_file);
 
 
@@ -68,6 +68,7 @@ final class PhabricatorFileComposeController
         $data,
         array(
           'name' => 'project.png',
+          'profile' => true,
           'canCDN' => true,
         ));
 
@@ -325,10 +326,10 @@ final class PhabricatorFileComposeController
     $color_string = idx($map, $color, '#ff00ff');
     $color_const = hexdec(trim($color_string, '#'));
 
-    $canvas = imagecreatetruecolor(50, 50);
+    $canvas = imagecreatetruecolor(100, 100);
     imagefill($canvas, 0, 0, $color_const);
 
-    imagecopy($canvas, $icon_img, 0, 0, 0, 0, 50, 50);
+    imagecopy($canvas, $icon_img, 0, 0, 0, 0, 100, 100);
 
     return PhabricatorImageTransformer::saveImageDataInAnyFormat(
       $canvas,

src/applications/files/controller/PhabricatorFileInfoController.php

@@ -247,6 +247,12 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
 
     $finfo->addProperty(pht('Builtin'), $builtin_string);
 
+    $is_profile = $file->getIsProfileImage()
+      ? pht('Yes')
+      : pht('No');
+
+    $finfo->addProperty(pht('Profile'), $is_profile);
+
     $storage_properties = new PHUIPropertyListView();
     $box->addPropertyList($storage_properties, pht('Storage'));
 

src/applications/files/storage/PhabricatorFile.php

@@ -34,6 +34,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
   const METADATA_CAN_CDN = 'canCDN';
   const METADATA_BUILTIN = 'builtin';
   const METADATA_PARTIAL = 'partial';
+  const METADATA_PROFILE = 'profile';
 
   protected $name;
   protected $mimeType;
@@ -1112,6 +1113,15 @@ final class PhabricatorFile extends PhabricatorFileDAO
     return $this;
   }
 
+  public function getIsProfileImage() {
+    return idx($this->metadata, self::METADATA_PROFILE);
+  }
+
+  public function setIsProfileImage($value) {
+    $this->metadata[self::METADATA_PROFILE] = $value;
+    return $this;
+  }
+
   protected function generateOneTimeToken() {
     $key = Filesystem::readRandomCharacters(16);
 
@@ -1213,6 +1223,11 @@ final class PhabricatorFile extends PhabricatorFileDAO
       $this->setBuiltinName($builtin);
     }
 
+    $profile = idx($params, 'profile');
+    if ($profile) {
+      $this->setIsProfileImage(true);
+    }
+
     $mime_type = idx($params, 'mime-type');
     if ($mime_type) {
       $this->setMimeType($mime_type);
@@ -1280,6 +1295,9 @@ final class PhabricatorFile extends PhabricatorFileDAO
         if ($this->isBuiltin()) {
           return PhabricatorPolicies::getMostOpenPolicy();
         }
+        if ($this->getIsProfileImage()) {
+          return PhabricatorPolicies::getMostOpenPolicy();
+        }
         return $this->getViewPolicy();
       case PhabricatorPolicyCapability::CAN_EDIT:
         return PhabricatorPolicies::POLICY_NOONE;

src/applications/files/transform/PhabricatorFileImageTransform.php

@@ -38,6 +38,10 @@ abstract class PhabricatorFileImageTransform extends PhabricatorFileTransform {
     $this->imageY = null;
   }
 
+  protected function getFileProperties() {
+    return array();
+  }
+
   protected function applyCropAndScale(
     $dst_w, $dst_h,
     $src_x, $src_y,
@@ -144,7 +148,7 @@ abstract class PhabricatorFileImageTransform extends PhabricatorFileTransform {
       array(
         'name' => $name,
         'canCDN' => true,
-      ));
+      ) + $this->getFileProperties());
   }
 
 

src/applications/files/transform/PhabricatorFileThumbnailTransform.php

@@ -43,6 +43,16 @@ final class PhabricatorFileThumbnailTransform
     return $this->key;
   }
 
+  protected function getFileProperties() {
+    $properties = array();
+    switch ($this->key) {
+      case self::TRANSFORM_PROFILE:
+        $properties['profile'] = true;
+        break;
+    }
+    return $properties;
+  }
+
   public function generateTransforms() {
     return array(
       id(new PhabricatorFileThumbnailTransform())

src/applications/files/transform/PhabricatorFileTransform.php

@@ -15,6 +15,18 @@ abstract class PhabricatorFileTransform extends Phobject {
     return array($this);
   }
 
+  public function executeTransform(PhabricatorFile $file) {
+    if ($this->canApplyTransform($file)) {
+      try {
+        return $this->applyTransform($file);
+      } catch (Exception $ex) {
+        // Ignore.
+      }
+    }
+
+    return $this->getDefaultTransform($file);
+  }
+
   public static function getAllTransforms() {
     static $map;
 

src/applications/people/controller/PhabricatorPeopleProfilePictureController.php

@@ -70,12 +70,9 @@ final class PhabricatorPeopleProfilePictureController
             'This server only supports these image formats: %s.',
             implode(', ', $supported_formats));
         } else {
-          $xformer = new PhabricatorImageTransformer();
-          $xformed = $xformer->executeProfileTransform(
-            $file,
-            $width = 50,
-            $min_height = 50,
-            $max_height = 50);
+          $xform = PhabricatorFileTransform::getTransformByKey(
+            PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE);
+          $xformed = $xform->executeTransform($file);
         }
       }
 

src/applications/project/controller/PhabricatorProjectEditPictureController.php

@@ -68,12 +68,9 @@ final class PhabricatorProjectEditPictureController
             'This server only supports these image formats: %s.',
             implode(', ', $supported_formats));
         } else {
-          $xformer = new PhabricatorImageTransformer();
-          $xformed = $xformer->executeProfileTransform(
-            $file,
-            $width = 50,
-            $min_height = 50,
-            $max_height = 50);
+          $xform = PhabricatorFileTransform::getTransformByKey(
+            PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE);
+          $xformed = $xform->executeTransform($file);
         }
       }
 

webroot/rsrc/css/phui/phui-header-view.css

@@ -90,6 +90,7 @@ body.device-phone .phui-header-view {
 .phui-header-image {
   display: inline-block;
   background-repeat: no-repeat;
+  background-size: 50px;
   border: 2px solid white;
   width: 50px;
   height: 50px;

webroot/rsrc/css/phui/phui-timeline-view.css

@@ -92,6 +92,7 @@
 
 .phui-timeline-image {
   background-repeat: no-repeat;
+  background-size: 50px;
   position: absolute;
   border-radius: 3px;
 }