diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -23,7 +23,7 @@ // we aren't trying to parse data we won't be able to parse correctly by // examining the Content-Type header. $content_type = idx($_SERVER, 'CONTENT_TYPE'); - $is_form_data = preg_match('@^multipart/form-data@i', $content_type); + $is_multipart = preg_match('@^multipart/form-data@i', $content_type); $request_method = idx($_SERVER, 'REQUEST_METHOD'); if ($request_method === 'PUT') { @@ -32,9 +32,52 @@ // like those coming from Git LFS. } else { $raw_input = PhabricatorStartup::getRawInput(); - if (strlen($raw_input) && !$is_form_data) { - $data += $parser->parseQueryString($raw_input); + if (strlen($raw_input)) { + if ($is_multipart && !ini_get('enable_post_data_reading')) { + $multipart_parser = id(new AphrontMultipartParser()) + ->setContentType($content_type); + + $multipart_parser->beginParse(); + $multipart_parser->continueParse($raw_input); + $parts = $multipart_parser->endParse(); + + $query_string = array(); + foreach ($parts as $part) { + if (!$part->isVariable()) { + continue; + } + + $name = $part->getName(); + $value = $part->getVariableValue(); + + $query_string[] = urlencode($name).'='.urlencode($value); + } + $query_string = implode('&', $query_string); + $post = $parser->parseQueryString($query_string); + + $files = array(); + foreach ($parts as $part) { + if ($part->isVariable()) { + continue; + } + + $files[$part->getName()] = $part->getPHPFileDictionary(); + } + $_FILES = $files; + } else { + $post += $parser->parseQueryString($raw_input); + } + + $_POST = $post; + PhabricatorStartup::rebuildRequest(); + + $data += $post; } else if ($_POST) { + $post = filter_input_array(INPUT_POST, FILTER_UNSAFE_RAW); + if (is_array($post)) { + $_POST = $post; + PhabricatorStartup::rebuildRequest(); + } $data += $_POST; } } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -178,9 +178,17 @@ } $tmp_name = idx($spec, 'tmp_name'); - $is_valid = @is_uploaded_file($tmp_name); - if (!$is_valid) { - throw new Exception(pht('File is not an uploaded file.')); + + // NOTE: If we parsed the request body ourselves, the files we wrote will + // not be registered in the `is_uploaded_file()` list. It's fine to skip + // this check: it just protects against sloppy code from the long ago era + // of "register_globals". + + if (ini_get('enable_post_data_reading')) { + $is_valid = @is_uploaded_file($tmp_name); + if (!$is_valid) { + throw new Exception(pht('File is not an uploaded file.')); + } } $file_data = Filesystem::readFile($tmp_name); diff --git a/support/PhabricatorStartup.php b/support/PhabricatorStartup.php --- a/support/PhabricatorStartup.php +++ b/support/PhabricatorStartup.php @@ -416,9 +416,12 @@ // NOTE: We don't filter INPUT_SERVER because we don't want to overwrite // changes made in "preamble.php". + + // NOTE: WE don't filter INPUT_POST because we may be constructing it + // lazily if "enable_post_data_reading" is disabled. + $filter = array( INPUT_GET, - INPUT_POST, INPUT_ENV, INPUT_COOKIE, ); @@ -434,9 +437,6 @@ case INPUT_COOKIE: $_COOKIE = array_merge($_COOKIE, $filtered); break; - case INPUT_POST: - $_POST = array_merge($_POST, $filtered); - break; case INPUT_ENV; $env = array_merge($_ENV, $filtered); $_ENV = self::filterEnvSuperglobal($env); @@ -444,18 +444,28 @@ } } - // rebuild $_REQUEST, respecting order declared in ini files + self::rebuildRequest(); + } + + /** + * @task validation + */ + public static function rebuildRequest() { + // Rebuild $_REQUEST, respecting order declared in ".ini" files. $order = ini_get('request_order'); + if (!$order) { $order = ini_get('variables_order'); } + if (!$order) { - // $_REQUEST will be empty, leave it alone + // $_REQUEST will be empty, so leave it alone. return; } + $_REQUEST = array(); - for ($i = 0; $i < strlen($order); $i++) { - switch ($order[$i]) { + for ($ii = 0; $ii < strlen($order); $ii++) { + switch ($order[$ii]) { case 'G': $_REQUEST = array_merge($_REQUEST, $_GET); break; @@ -466,7 +476,7 @@ $_REQUEST = array_merge($_REQUEST, $_COOKIE); break; default: - // $_ENV and $_SERVER never go into $_REQUEST + // $_ENV and $_SERVER never go into $_REQUEST. break; } } @@ -593,6 +603,12 @@ return; } + // If "enable_post_data_reading" is off, we won't have $_POST and this + // condition is effectively impossible. + if (!ini_get('enable_post_data_reading')) { + return; + } + // If there's POST data, clearly we're in good shape. if ($_POST) { return;