From 18f049a282f4085de5f68db42a9fe4dc3ef393ce Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 11 Jan 2021 10:29:58 -0800
Subject: [PATCH] Fix reading of the request path when running the PHP builtin
 webserver

Summary:
Ref T13575. Since PHP builtin webserver support was added, the pathway for parsing request parameters became more complex. We now rebuild "$_REQUEST" later, and this rebuild will destroy any mutations made to it here, so the assignment to "__path__" is lost.

Instead of "validating" the request path, make this method "read" the request path and store it explicitly, so it will survive any later request mutations.

Test Plan:
  - Submitted any POST form while running Phabricator under the builtin PHP webserver. Old behavior was an error when accessing "__path__"; new behavior is a working application.
  - Loaded normal pages, etc.

Maniphest Tasks: T13575

Differential Revision: https://secure.phabricator.com/D21506
---
 .../AphrontApplicationConfiguration.php       |  4 +-
 support/startup/PhabricatorStartup.php        | 51 ++++++++++++++++---
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
index 5bed835d8f..31b3db9a7f 100644
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -176,7 +176,7 @@ final class AphrontApplicationConfiguration
     }
 
     $host = AphrontRequest::getHTTPHeader('Host');
-    $path = $_REQUEST['__path__'];
+    $path = PhabricatorStartup::getRequestPath();
 
     $application = new self();
 
@@ -759,7 +759,7 @@ final class AphrontApplicationConfiguration
   }
 
   private static function newSelfCheckResponse() {
-    $path = idx($_REQUEST, '__path__', '');
+    $path = PhabricatorStartup::getRequestPath();
     $query = idx($_SERVER, 'QUERY_STRING', '');
 
     $pairs = id(new PhutilQueryStringParser())
diff --git a/support/startup/PhabricatorStartup.php b/support/startup/PhabricatorStartup.php
index 5dac7505d9..870d342464 100644
--- a/support/startup/PhabricatorStartup.php
+++ b/support/startup/PhabricatorStartup.php
@@ -35,6 +35,7 @@
  * @task validation   Validation
  * @task ratelimit    Rate Limiting
  * @task phases       Startup Phase Timers
+ * @task request-path Request Path
  */
 final class PhabricatorStartup {
 
@@ -47,6 +48,7 @@ final class PhabricatorStartup {
   private static $phases;
 
   private static $limits = array();
+  private static $requestPath;
 
 
 /* -(  Accessing Request Information  )-------------------------------------- */
@@ -119,6 +121,7 @@ final class PhabricatorStartup {
     self::$phases = array();
 
     self::$accessLog = null;
+    self::$requestPath = null;
 
     static $registered;
     if (!$registered) {
@@ -140,7 +143,7 @@ final class PhabricatorStartup {
 
     self::normalizeInput();
 
-    self::verifyRewriteRules();
+    self::readRequestPath();
 
     self::beginOutputCapture();
   }
@@ -552,17 +555,29 @@ final class PhabricatorStartup {
 
 
   /**
-   * @task validation
+   * @task request-path
    */
-  private static function verifyRewriteRules() {
+  private static function readRequestPath() {
+
+    // See T13575. The request path may be provided in:
+    //
+    //  - the "$_GET" parameter "__path__" (normal for Apache and nginx); or
+    //  - the "$_SERVER" parameter "REQUEST_URI" (normal for the PHP builtin
+    //    webserver).
+    //
+    // Locate it wherever it is, and store it for later use. Note that writing
+    // to "$_REQUEST" here won't always work, because later code may rebuild
+    // "$_REQUEST" from other sources.
+
     if (isset($_REQUEST['__path__']) && strlen($_REQUEST['__path__'])) {
+      self::setRequestPath($_REQUEST['__path__']);
       return;
     }
 
+    // Compatibility with PHP 5.4+ built-in web server.
     if (php_sapi_name() == 'cli-server') {
-      // Compatibility with PHP 5.4+ built-in web server.
-      $url = parse_url($_SERVER['REQUEST_URI']);
-      $_REQUEST['__path__'] = $url['path'];
+      $path = parse_url($_SERVER['REQUEST_URI']);
+      self::setRequestPath($path['path']);
       return;
     }
 
@@ -580,6 +595,30 @@ final class PhabricatorStartup {
     }
   }
 
+  /**
+   * @task request-path
+   */
+  public static function getRequestPath() {
+    $path = self::$requestPath;
+
+    if ($path === null) {
+      self::didFatal(
+        'Request attempted to access request path, but no request path is '.
+        'available for this request. You may be calling web request code '.
+        'from a non-request context, or your webserver may not be passing '.
+        'a request path to Phabricator in a format that it understands.');
+    }
+
+    return $path;
+  }
+
+  /**
+   * @task request-path
+   */
+  public static function setRequestPath($path) {
+    self::$requestPath = $path;
+  }
+
 
 /* -(  Rate Limiting  )------------------------------------------------------ */