Page MenuHomePhabricator

Support imagemagick on new image transform pathway
ClosedPublic

Authored by epriestley on May 12 2015, 6:52 PM.
Tags
None
Referenced Files
F13095951: D12813.id30832.diff
Thu, Apr 25, 1:30 PM
F13095950: D12813.id30792.diff
Thu, Apr 25, 1:30 PM
F13095948: D12813.id.diff
Thu, Apr 25, 1:30 PM
Unknown Object (File)
Thu, Apr 25, 12:48 AM
Unknown Object (File)
Sat, Apr 20, 8:05 PM
Unknown Object (File)
Sat, Apr 13, 9:32 PM
Unknown Object (File)
Tue, Apr 9, 9:46 AM
Unknown Object (File)
Sat, Mar 30, 11:37 AM
Subscribers

Details

Summary

Ref T7707. For animated GIFs, use imagemagick if it is available.

Test Plan

Generated small versions of a bunch of different GIFs.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Support imagemagick on new image transform pathway.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
btrahan added inline comments.
src/applications/files/transform/PhabricatorFileImageTransform.php
96

Would this get tweaked post T5166 or is there no reasonable 10+ second conversion? I guess gifs could be arbitrarily many frames of animation....

This revision is now accepted and ready to land.May 12 2015, 7:02 PM
src/applications/files/transform/PhabricatorFileImageTransform.php
96

I'm OK with bumping this up a bit if there's a use case for it, but generally think uploading 80MB GIFs and expecting us to process them isn't a real use case, no matter how hilarious they are, since the applications are all fun/silly (memes, funny animated avatars, etc). If there was a real use case that we were missing with weightier consequences, I'd be more open to trying harder here (but I'd probably want to look at converting to webm/mp4 first/instead).

The specific problematic case is attackers uploading 512x512px solid white GIFs with 65535 frames. The on-disk size of these is a few KB; the in-memory size is like 80GB; imagemagick takes days/forever to process them while maxing CPU and eating memory.

(We won't thumbnail anything bigger than 4MB now anyway so that's probably the limit most movies-in-GIF-form will hit first.)

This revision was automatically updated to reflect the committed changes.