HomePhabricator

Implement new profile transform with amazing "error handling" feature

Description

Implement new profile transform with amazing "error handling" feature

Summary:
Ref T7707. Ref T4406. Ref T2479. This implements the profile-style (fixed width and height) transforms in a modern way.

  • Added a "regnerate" feature to the support UI to make testing easier and surface errors.
  • Laboriously check errors from everything.
  • Fix the profile thumbnailing so it crops properly instead of leaving margins.
  • Also defuses the "gigantic white PNG" attack.

This doesn't handle the imagemagick case (for animated GIFs) yet.

Test Plan:

  • Uploaded a variety of wide/narrow/small/large files and converted them into sensible profile pictures.
  • Tried to thumbnail some text files.
  • Set the pixel-size and file-size limits artificially small and hit them.
  • Used "regenerate" a bunch while testing the rest of this stuff.
  • Verified that non-regenerate flows still produce a default/placeholder image.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4406, T2479, T7707

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

Event Timeline

talshiri added inline comments.
/src/applications/files/transform/PhabricatorFileImageTransform.php
199

We were just bitten by this limit.
I understand why it's there, but seems a little low and arbitrary.

Are you interested in having this be pulled into a config param?

I don't want to make it an option; see T8227.

I'll accept some other arbitrary limit (e.g., 32MB?), although 4MB seems fairly large.

Is the case you were bitten on an actual image, or a gigantic GIF you were trying to use as a macro?

It was a screenshot taken on a retina Mac. It had some mapping image on it. The whole thing was around 5mb.