Page MenuHomePhabricator

Don't load attached files for profile images
ClosedPublic

Authored by epriestley on Jun 8 2015, 9:56 PM.
Tags
None
Referenced Files
F14051682: D13216.diff
Fri, Nov 15, 2:56 AM
F14049150: D13216.diff
Thu, Nov 14, 11:24 AM
F14038067: D13216.diff
Sun, Nov 10, 9:21 PM
F14035766: D13216.id.diff
Sun, Nov 10, 7:30 AM
F14035270: D13216.id.diff
Sun, Nov 10, 5:14 AM
F14035244: D13216.id31952.diff
Sun, Nov 10, 5:09 AM
F14035112: D13216.id31950.diff
Sun, Nov 10, 4:15 AM
F14034491: D13216.id31950.diff
Sun, Nov 10, 12:36 AM
Subscribers

Details

Summary

Ref T8478. I think the cycle is:

  • Conpherence Thread > Loads handle for participant > loads file for profile image > loads attached files to check visibility > loads conpherence thread > ...

So, specifically, someone attached their profile image to a thread or message somewhere.

This breaks the cycle by stopping the attached-files visibility check from happening, since we don't need it. This seemed like the easiest link in the chain to break.

Ideally, I think the longer-term and more complete fix here is to stop Conpherence from requiring handles in order to load thread handles (and, generally, having a "handles must not load other handles" rule), but that's not trivial and might not be especially practical.

Test Plan

Will test in production.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Don't load attached files for profile images.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Jun 8 2015, 10:39 PM
This revision was automatically updated to reflect the committed changes.