Page MenuHomePhabricator

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

Authored by epriestley on Jan 28 2019, 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.Jan 28 2019, 7:09 PM
epriestley requested review of this revision.Jan 28 2019, 7:10 PM
amckinley accepted this revision.Jan 30 2019, 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.Jan 30 2019, 6:13 AM
This revision was automatically updated to reflect the committed changes.
Pawka added a subscriber: Pawka.Mar 21 2019, 1:50 PM

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.