Page MenuHomePhabricator

Read "$_POST" before hooking the profiler, and remove "aphront.default-application-configuration-class"
ClosedPublic

Authored by epriestley on Mon, Jan 28, 7:09 PM.

Details

Summary

Ref T4369. Ref T12297. Ref T13242. Ref PHI1010. I want to take a quick look at transaction.search and see if there's anything quick and obvious we can do to improve performance.

On secure, the __profile__ flag does not survive POST like it's supposed to: when you profile a page and then submit a form on the page, the result is supposed to be profiled. The intent is to make it easier to profile Conduit calls.

I believe this is because we're hooking the profiler, then rebuilding POST data a little later -- so $_POST['__profile__'] isn't set yet when the profiler checks.

Move the POST rebuild a little earlier to fix this.

Also, remove the very ancient "aphront.default-application-configuration-class". I believe this was only used by Facebook to do CIDR checks against corpnet or something like that. It is poorly named and long-obsolete now, and AphrontSite does everything we might reasonably have wanted it to do.

Test Plan

Poked around locally without any issues. Will check if this fixes the issue on secure.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Mon, Jan 28, 7:09 PM
epriestley requested review of this revision.Mon, Jan 28, 7:10 PM
amckinley accepted this revision.Wed, Jan 30, 6:13 AM

I would kind of like to see these changes split into two diffs, but there's nothing wrong with reverting the whole thing if this causes problems.

I've read enough of the linked context to claim that I mostly understand this, but still:

itsmagic

src/aphront/configuration/AphrontApplicationConfiguration.php
784

src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
4

This revision is now accepted and ready to land.Wed, Jan 30, 6:13 AM
This revision was automatically updated to reflect the committed changes.