Page MenuHomePhabricator

Stop trying to assess the image dimensions of large files and file chunks
ClosedPublic

Authored by epriestley on Dec 13 2017, 2:48 PM.
Tags
None
Referenced Files
F15476914: D18830.diff
Mon, Apr 7, 10:05 AM
F15474333: D18830.id45183.diff
Sun, Apr 6, 7:28 AM
F15474332: D18830.id.diff
Sun, Apr 6, 7:28 AM
F15474184: D18830.diff
Sun, Apr 6, 5:54 AM
F15460618: D18830.id45174.diff
Tue, Apr 1, 12:30 AM
F15457237: D18830.id.diff
Sun, Mar 30, 3:07 PM
F15454996: D18830.diff
Sat, Mar 29, 9:47 PM
F15447339: D18830.id45174.diff
Thu, Mar 27, 11:14 PM
Subscribers
None

Details

Summary

Depends on D18828. Ref T7789. See https://discourse.phabricator-community.org/t/git-lfs-fails-with-large-images/584.

Currently, when you upload a large (>4MB) image, we may try to assess the dimensions for the image and for each individual chunk.

At best, this is slow and not useful. At worst, it fatals or consumes a ton of memory and I/O we don't need to be using.

Instead:

  • Don't try to assess dimensions for chunked files.
  • Don't try to assess dimensions for the chunks themselves.
  • Squelch errors for bad data, etc., that gd can't actually read, since we recover sensibly.
Test Plan
  • Created a 2048x2048 PNG in Photoshop using the "Random Noise" filter which weighs 8.5MB.
  • Uploaded it.
  • Before patch: got complaints in log about imagecreatefromstring() failing, although the actual upload went OK in my environment.
  • After patch: clean log, no attempt to detect the size of a big image.
  • Also uploaded a small image, got dimensions detected properly still.

Diff Detail

Repository
rP Phabricator
Branch
lfs4
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 18947
Build 25554: Run Core Tests
Build 25553: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/applications/files/storage/PhabricatorFile.php
43

chunk.jpg (417×467 px, 46 KB)

This revision is now accepted and ready to land.Dec 14 2017, 5:54 PM
This revision was automatically updated to reflect the committed changes.