From a15444aa7919053edd5002f3d355385d6ab97b36 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 4 Jun 2015 17:26:52 -0700
Subject: [PATCH] Remove PhabricatorStartup::getGlobal/setGlobal mechanism

Summary:
Ref T8424. Fixes T7114. This was envisioned as a per-request cache for reusing interpreters, but isn't a good fit for that in modern Phabricator.

In particular, it isn't loaded by the daemons, but they have equal need for per-request caching.

Since I finally need such a cache for Spaces, throw the old stuff away before I built a more modern cache.

Also resolves T7114 by dropping filtering on $_SERVER. I'm pretty sure this is the simplest fix, see D12977 for a bit more discussion.

Test Plan: Called `didFatal()` from somewhere in normal code and verified it was able to use the access log.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7114, T8424

Differential Revision: https://secure.phabricator.com/D13152
---
 src/__phutil_library_map__.php                |  1 -
 .../AphrontApplicationConfiguration.php       |  2 +-
 support/PhabricatorStartup.php                | 56 +++++--------------
 3 files changed, 15 insertions(+), 44 deletions(-)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index c9672655e7..1c6dc98816 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3903,7 +3903,6 @@ phutil_register_library_map(array(
     'DivinerLiveBook' => array(
       'DivinerDAO',
       'PhabricatorPolicyInterface',
-      'PhabricatorProjectInterface',
       'PhabricatorDestructibleInterface',
     ),
     'DivinerLivePublisher' => 'DivinerPublisher',
diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
index a3821f18b5..d1ced8a3e8 100644
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -80,7 +80,7 @@ abstract class AphrontApplicationConfiguration {
     // This is the earliest we can get away with this, we need env config first.
     PhabricatorAccessLog::init();
     $access_log = PhabricatorAccessLog::getLog();
-    PhabricatorStartup::setGlobal('log.access', $access_log);
+    PhabricatorStartup::setAccessLog($access_log);
     $access_log->setData(
       array(
         'R' => AphrontRequest::getHTTPHeader('Referer', '-'),
diff --git a/support/PhabricatorStartup.php b/support/PhabricatorStartup.php
index 30e8f3c937..fd612f27b0 100644
--- a/support/PhabricatorStartup.php
+++ b/support/PhabricatorStartup.php
@@ -39,7 +39,7 @@ final class PhabricatorStartup {
 
   private static $startTime;
   private static $debugTimeLimit;
-  private static $globals = array();
+  private static $accessLog;
   private static $capturingOutput;
   private static $rawInput;
   private static $oldMemoryLimit;
@@ -72,26 +72,11 @@ final class PhabricatorStartup {
   /**
    * @task info
    */
-  public static function setGlobal($key, $value) {
-    self::validateGlobal($key);
-
-    self::$globals[$key] = $value;
+  public static function setAccessLog($access_log) {
+    self::$accessLog = $access_log;
   }
 
 
-  /**
-   * @task info
-   */
-  public static function getGlobal($key, $default = null) {
-    self::validateGlobal($key);
-
-    if (!array_key_exists($key, self::$globals)) {
-      return $default;
-    }
-
-    return self::$globals[$key];
-  }
-
   /**
    * @task info
    */
@@ -108,7 +93,7 @@ final class PhabricatorStartup {
    */
   public static function didStartup() {
     self::$startTime = microtime(true);
-    self::$globals = array();
+    self::$accessLog = null;
 
     static $registered;
     if (!$registered) {
@@ -348,8 +333,7 @@ final class PhabricatorStartup {
     }
 
     self::endOutputCapture();
-    $access_log = self::getGlobal('log.access');
-
+    $access_log = self::$accessLog;
     if ($access_log) {
       // We may end up here before the access log is initialized, e.g. from
       // verifyPHP().
@@ -402,9 +386,15 @@ final class PhabricatorStartup {
    */
   private static function normalizeInput() {
     // Replace superglobals with unfiltered versions, disrespect php.ini (we
-    // filter ourselves)
-    $filter = array(INPUT_GET, INPUT_POST,
-      INPUT_SERVER, INPUT_ENV, INPUT_COOKIE,
+    // filter ourselves).
+
+    // NOTE: We don't filter INPUT_SERVER because we don't want to overwrite
+    // changes made in "preamble.php".
+    $filter = array(
+      INPUT_GET,
+      INPUT_POST,
+      INPUT_ENV,
+      INPUT_COOKIE,
     );
     foreach ($filter as $type) {
       $filtered = filter_input_array($type, FILTER_UNSAFE_RAW);
@@ -412,9 +402,6 @@ final class PhabricatorStartup {
         continue;
       }
       switch ($type) {
-        case INPUT_SERVER:
-          $_SERVER = array_merge($_SERVER, $filtered);
-          break;
         case INPUT_GET:
           $_GET = array_merge($_GET, $filtered);
           break;
@@ -551,21 +538,6 @@ final class PhabricatorStartup {
   }
 
 
-  /**
-   * @task validation
-   */
-  private static function validateGlobal($key) {
-    static $globals = array(
-      'log.access' => true,
-      'csrf.salt'  => true,
-    );
-
-    if (empty($globals[$key])) {
-      throw new Exception("Access to unknown startup global '{$key}'!");
-    }
-  }
-
-
   /**
    * Detect if this request has had its POST data stripped by exceeding the
    * 'post_max_size' PHP configuration limit.