Page MenuHomePhabricator

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

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



Ref T4369. Ref T12297. Ref T13242. Ref PHI1010. I want to take a quick look at 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

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

Event Timeline

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:




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

Looks like this patch broke serving Phabricator via php built-in server as __path__ variable is not available on $_POST after readHTTPPOSTData method is executed.

Please report issues you encounter with Phabricator via Discourse, not by commenting here.