Page MenuHomePhabricator
Files F15503437

D18702.diff
No OneTemporary
Actions

D18702.diff
View Options

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
@@ -15,26 +15,79 @@
$parser = new PhutilQueryStringParser();
$data = array();
- // If the request has "multipart/form-data" content, we can't use
- // PhutilQueryStringParser to parse it, and the raw data supposedly is not
- // available anyway (according to the PHP documentation, "php://input" is
- // not available for "multipart/form-data" requests). However, it is
- // available at least some of the time (see T3673), so double check that
- // 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);
-
$request_method = idx($_SERVER, 'REQUEST_METHOD');
if ($request_method === 'PUT') {
// For PUT requests, do nothing: in particular, do NOT read input. This
// allows us to stream input later and process very large PUT requests,
// like those coming from Git LFS.
} else {
+ // For POST requests, we're going to read the raw input ourselves here
+ // if we can. Among other things, this corrects variable names with
+ // the "." character in them, which PHP normally converts into "_".
+
+ // There are two major considerations here: whether the
+ // `enable_post_data_reading` option is set, and whether the content
+ // type is "multipart/form-data" or not.
+
+ // If `enable_post_data_reading` is off, we're free to read the entire
+ // raw request body and parse it -- and we must, because $_POST and
+ // $_FILES are not built for us. If `enable_post_data_reading` is on,
+ // which is the default, we may not be able to read the body (the
+ // documentation says we can't, but empirically we can at least some
+ // of the time).
+
+ // If the content type is "multipart/form-data", we need to build both
+ // $_POST and $_FILES, which is involved. The body itself is also more
+ // difficult to parse than other requests.
$raw_input = PhabricatorStartup::getRawInput();
- if (strlen($raw_input) && !$is_form_data) {
- $data += $parser->parseQueryString($raw_input);
+ if (strlen($raw_input)) {
+ $content_type = idx($_SERVER, 'CONTENT_TYPE');
+ $is_multipart = preg_match('@^multipart/form-data@i', $content_type);
+ 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;

File Metadata

Mime Type
text/plain
Expires
Tue, Apr 15, 9:57 AM (1 d, 14 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7681390
Default Alt Text
D18702.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility