diff --git a/backup/util/ui/base_moodleform.class.php b/backup/util/ui/base_moodleform.class.php index fb0e975acfd..c5ba97d1d85 100644 --- a/backup/util/ui/base_moodleform.class.php +++ b/backup/util/ui/base_moodleform.class.php @@ -73,6 +73,11 @@ abstract class base_moodleform extends moodleform { */ function __construct(base_ui_stage $uistage, $action=null, $customdata=null, $method='post', $target='', $attributes=null, $editable=true) { $this->uistage = $uistage; + $attributes = (array)$attributes; + if (!isset($attributes['enctype'])) { + $attributes['enctype'] = 'application/x-www-form-urlencoded'; // Enforce compatibility with our max_input_vars hack. + } + parent::__construct($action, $customdata, $method, $target, $attributes, $editable); } /** diff --git a/backup/util/ui/restore_ui_stage.class.php b/backup/util/ui/restore_ui_stage.class.php index a884fc721ca..e32608d07e0 100644 --- a/backup/util/ui/restore_ui_stage.class.php +++ b/backup/util/ui/restore_ui_stage.class.php @@ -697,6 +697,7 @@ class restore_ui_stage_process extends restore_ui_stage { $html .= html_writer::start_tag('form', array( 'action' => $url->out_omit_querystring(), 'class' => 'backup-restore', + 'enctype' => 'application/x-www-form-urlencoded', // Enforce compatibility with our max_input_vars hack. 'method' => 'post')); foreach ($url->params() as $name => $value) { $html .= html_writer::empty_tag('input', array( diff --git a/grade/report/grader/index.php b/grade/report/grader/index.php index a0b082ad4e2..fe05b0ef680 100644 --- a/grade/report/grader/index.php +++ b/grade/report/grader/index.php @@ -156,7 +156,7 @@ $reporthtml = $report->get_grade_table(); // print submit button if ($USER->gradeediting[$course->id] && ($report->get_pref('showquickfeedback') || $report->get_pref('quickgrading'))) { - echo '
'; + echo ''; // Enforce compatibility with our max_input_vars hack. echo '
'; echo ''; echo ''; diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index 70147a2c9b6..c5e9fb54ce7 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -1734,51 +1734,10 @@ class grade_report_grader extends grade_report { /** * Returns the maximum number of students to be displayed on each page * - * Takes into account the 'studentsperpage' user preference and the 'max_input_vars' - * PHP setting. Too many fields is only a problem when submitting grades but - * we respect 'max_input_vars' even when viewing grades to prevent students disappearing - * when toggling editing on and off. - * * @return int The maximum number of students to display per page */ public function get_students_per_page() { - global $USER; - static $studentsperpage = null; - - if ($studentsperpage === null) { - $originalstudentsperpage = $studentsperpage = $this->get_pref('studentsperpage'); - - // Will this number of students result in more fields that we are allowed? - $maxinputvars = ini_get('max_input_vars'); - if ($maxinputvars !== false) { - // We can't do anything about there being more grade items than max_input_vars, - // but we can decrease number of students per page if there are >= max_input_vars - $fieldsperstudent = 0; // The number of fields output per student - - if ($this->get_pref('quickgrading') || $this->get_pref('showquickfeedback')) { - // Each array (grade, feedback) will gain one element - $fieldsperstudent ++; - } - - $fieldsrequired = $studentsperpage * $fieldsperstudent; - if ($fieldsrequired >= $maxinputvars) { - $studentsperpage = $maxinputvars - 1; // Subtract one to be on the safe side - if ($studentsperpage<1) { - // Make sure students per page doesn't fall below 1, though if your - // max_input_vars is only 1 you've got bigger problems! - $studentsperpage = 1; - } - - $a = new stdClass(); - $a->originalstudentsperpage = $originalstudentsperpage; - $a->studentsperpage = $studentsperpage; - $a->maxinputvars = $maxinputvars; - debugging(get_string('studentsperpagereduced', 'grades', $a)); - } - } - } - - return $studentsperpage; + return $this->get_pref('studentsperpage'); } } diff --git a/lib/formslib.php b/lib/formslib.php index b184ad13a9f..fe514818940 100644 --- a/lib/formslib.php +++ b/lib/formslib.php @@ -261,10 +261,11 @@ abstract class moodleform { $submission = array(); if ($method == 'post') { if (!empty($_POST)) { - $submission = $this->_get_post_params(); + $submission = $_POST; } } else { - $submission = array_merge_recursive($_GET, $this->_get_post_params()); // Emulate handling of parameters in xxxx_param(). + $submission = $_GET; + merge_query_params($submission, $_POST); // Emulate handling of parameters in xxxx_param(). } // following trick is needed to enable proper sesskey checks when using GET forms @@ -283,34 +284,12 @@ abstract class moodleform { } /** - * Internal method. Gets all POST variables, bypassing max_input_vars limit if needed. - * - * @return array All POST variables as an array, in the same format as $_POST. + * Internal method - should not be used anywhere. + * @deprecated since 2.6 + * @return array $_POST. */ protected function _get_post_params() { - $enctype = $this->_form->getAttribute('enctype'); - $max = (int)ini_get('max_input_vars'); - - if (empty($max) || count($_POST, COUNT_RECURSIVE) < $max || (!empty($enctype) && $enctype == 'multipart/form-data')) { - return $_POST; - } - - // Large POST request with enctype supported by php://input. - // Parse php://input in chunks to bypass max_input_vars limit, which also applies to parse_str(). - $allvalues = array(); - $values = array(); - $str = file_get_contents("php://input"); - $delim = '&'; - - $fun = create_function('$p', 'return implode("'.$delim.'", $p);'); - $chunks = array_map($fun, array_chunk(explode($delim, $str), $max)); - - foreach ($chunks as $chunk) { - parse_str($chunk, $values); - $allvalues = array_merge_recursive($allvalues, $values); - } - - return $allvalues; + return $_POST; } /** diff --git a/lib/setup.php b/lib/setup.php index 3037136f282..be43942c69b 100644 --- a/lib/setup.php +++ b/lib/setup.php @@ -712,6 +712,10 @@ if (!empty($CFG->profilingenabled)) { } } +// Hack to get around max_input_vars restrictions, +// we need to do this after session init to have some basic DDoS protection. +workaround_max_input_vars(); + // Process theme change in the URL. if (!empty($CFG->allowthemechangeonurl) and !empty($_GET['theme'])) { // we have to use _GET directly because we do not want this to interfere with _POST diff --git a/lib/setuplib.php b/lib/setuplib.php index b48aba25c0e..2748881ae20 100644 --- a/lib/setuplib.php +++ b/lib/setuplib.php @@ -944,6 +944,108 @@ function setup_get_remote_url() { return $rurl; } +/** + * Try to work around the 'max_input_vars' restriction if necessary. + */ +function workaround_max_input_vars() { + // Make sure this gets executed only once from lib/setup.php! + static $executed = false; + if ($executed) { + debugging('workaround_max_input_vars() must be called only once!'); + return; + } + $executed = true; + + if (!isset($_SERVER["CONTENT_TYPE"]) or strpos($_SERVER["CONTENT_TYPE"], 'multipart/form-data') !== false) { + // Not a post or 'multipart/form-data' which is not compatible with "php://input" reading. + return; + } + + if (!isloggedin() or isguestuser()) { + // Only real users post huge forms. + return; + } + + $max = (int)ini_get('max_input_vars'); + + if ($max <= 0) { + // Most probably PHP < 5.3.9 that does not implement this limit. + return; + } + + if ($max >= 200000) { + // This value should be ok for all our forms, by setting it in php.ini + // admins may prevent any unexpected regressions caused by this hack. + + // Note there is no need to worry about DDoS caused by making this limit very high + // because there are very many easier ways to DDoS any Moodle server. + return; + } + + if (count($_POST, COUNT_RECURSIVE) < $max) { + return; + } + + // Large POST request with enctype supported by php://input. + // Parse php://input in chunks to bypass max_input_vars limit, which also applies to parse_str(). + $str = file_get_contents("php://input"); + if ($str === false or $str === '') { + // Some weird error. + return; + } + + $delim = '&'; + $fun = create_function('$p', 'return implode("'.$delim.'", $p);'); + $chunks = array_map($fun, array_chunk(explode($delim, $str), $max)); + + foreach ($chunks as $chunk) { + $values = array(); + parse_str($chunk, $values); + + if (ini_get_bool('magic_quotes_gpc')) { + // Use the same logic as lib/setup.php to work around deprecated magic quotes. + $values = array_map('stripslashes_deep', $values); + } + + merge_query_params($_POST, $values); + merge_query_params($_REQUEST, $values); + } +} + +/** + * Merge parsed POST chunks. + * + * NOTE: this is not perfect, but it should work in most cases hopefully. + * + * @param array $target + * @param array $values + */ +function merge_query_params(array &$target, array $values) { + if (isset($values[0]) and isset($target[0])) { + // This looks like a split [] array, lets verify the keys are continuous starting with 0. + $keys1 = array_keys($values); + $keys2 = array_keys($target); + if ($keys1 === array_keys($keys1) and $keys2 === array_keys($keys2)) { + foreach ($values as $v) { + $target[] = $v; + } + return; + } + } + foreach ($values as $k => $v) { + if (!isset($target[$k])) { + $target[$k] = $v; + continue; + } + if (is_array($target[$k]) and is_array($v)) { + merge_query_params($target[$k], $v); + continue; + } + // We should not get here unless there are duplicates in params. + $target[$k] = $v; + } +} + /** * Initializes our performance info early. * diff --git a/lib/tests/setuplib_test.php b/lib/tests/setuplib_test.php index 63431ee0efc..a2060c3a45f 100644 --- a/lib/tests/setuplib_test.php +++ b/lib/tests/setuplib_test.php @@ -142,4 +142,82 @@ class core_setuplib_testcase extends basic_testcase { $this->assertContains($expected, $exceptioninfo->message, 'Exception message does not contain system paths'); $this->assertContains($expected, $exceptioninfo->debuginfo, 'Exception debug info does not contain system paths'); } + + public function test_merge_query_params() { + $original = array( + 'id' => '1', + 'course' => '2', + 'action' => 'delete', + 'grade' => array( + 0 => 'a', + 1 => 'b', + 2 => 'c', + ), + 'items' => array( + 'a' => 'aa', + 'b' => 'bb', + ), + 'mix' => array( + 0 => '2', + ), + 'numerical' => array( + '2' => array('a' => 'b'), + '1' => '2', + ), + ); + + $chunk = array( + 'numerical' => array( + '0' => 'z', + '2' => array('d' => 'e'), + ), + 'action' => 'create', + 'next' => '2', + 'grade' => array( + 0 => 'e', + 1 => 'f', + 2 => 'g', + ), + 'mix' => 'mix', + ); + + $expected = array( + 'id' => '1', + 'course' => '2', + 'action' => 'create', + 'grade' => array( + 0 => 'a', + 1 => 'b', + 2 => 'c', + 3 => 'e', + 4 => 'f', + 5 => 'g', + ), + 'items' => array( + 'a' => 'aa', + 'b' => 'bb', + ), + 'mix' => 'mix', + 'numerical' => array( + '2' => array('a' => 'b', 'd' => 'e'), + '1' => '2', + '0' => 'z', + ), + 'next' => '2', + ); + + $array = $original; + merge_query_params($array, $chunk); + + $this->assertSame($expected, $array); + $this->assertNotSame($original, $array); + + $query = "id=1&course=2&action=create&grade%5B%5D=a&grade%5B%5D=b&grade%5B%5D=c&grade%5B%5D=e&grade%5B%5D=f&grade%5B%5D=g&items%5Ba%5D=aa&items%5Bb%5D=bb&mix=mix&numerical%5B2%5D%5Ba%5D=b&numerical%5B2%5D%5Bd%5D=e&numerical%5B1%5D=2&numerical%5B0%5D=z&next=2"; + $decoded = array(); + parse_str($query, $decoded); + $this->assertSame($expected, $decoded); + + // Prove that we cannot use array_merge_recursive() instead. + $this->assertNotSame($expected, array_merge_recursive($original, $chunk)); + } }