revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00
Code Issues Releases Wiki Activity

Parse "multipart/form-data" bodies even if "enable_post_data_reading" is on

Browse source 
Summary:
Ref T4369. During T13507, I set my "max_post_size" to a very small value, like 7 (i.e., 7 bytes). This essentially disables "enable_post_data_reading" even if the setting is technically on.

This breaks forms which use "multipart/form-data", which are rare but not nonexistent. Notably, forms in Config use this setting (because of `ui.header` stuff?) although perhaps they should not or no longer need to.

This can be fixed by parsing the raw input.

Since the only reason we don't parse the raw input is concern that we may not be able to read it (per documentation, but never actually observed), and we do a `strlen()` test anyway, just read it unconditionally.

This should fix cases where POST data wasn't read because of "max_post_size" without impacting anything else.

Test Plan: With very small "max_post_size", updated "ui.footer-items" in Config. Before: form acted as a no-op. After: form submitted.

Maniphest Tasks: T4369

Differential Revision: https://secure.phabricator.com/D21165
This commit is contained in:
epriestley 2020-04-24 10:21:59 -07:00
parent 5a460e4ea5
commit 6f7147376f
1 changed files with 6 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
src/aphront/configuration/AphrontApplicationConfiguration.php
View file

@ -812,27 +812,22 @@ final class AphrontApplicationConfiguration
// 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 "enable_post_data_reading" is on, the documentation suggests we
// can not read the body. In practice, we seem to be able to. This may
// need to be resolved at some point, likely by instructing installs
// to disable this option.
// 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();
$parser = new PhutilQueryStringParser();
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')) {
if ($is_multipart) {
$multipart_parser = id(new AphrontMultipartParser())
->setContentType($content_type);