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.
Tags
None
Referenced Files
F14027751: D20046.diff
Fri, Nov 8, 9:04 AM
F14025265: D20046.diff
Thu, Nov 7, 3:46 PM
F13986004: D20046.diff
Mon, Oct 21, 12:02 AM
F13973746: D20046.id.diff
Fri, Oct 18, 2:24 AM
Unknown Object (File)
Oct 7 2024, 2:13 AM
Unknown Object (File)
Oct 1 2024, 5:52 PM
Unknown Object (File)
Sep 26 2024, 1:44 PM
Unknown Object (File)
Sep 26 2024, 1:38 PM
Subscribers

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
Branch
xaction1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21725
Build 29641: Run Core Tests
Build 29640: arc lint + arc unit

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:

itsmagic

src/aphront/configuration/AphrontApplicationConfiguration.php
784

angry eyes.gif (247×440 px, 632 KB)

src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
4

im a sign.jpg (338×450 px, 30 KB)

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.