Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-52226

max_input_vars still causes problems with advanced checkboxes

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      1. Place attached test.php in the root of a Moodle installation.
      2. View the page.
      3. Click the first tickbox and the last tickbox.
      4. Hit save; the form reloads.
      EXPECTED: The first and last tickbox should still be ticked, as entered by the user.
      BEFORE FIX: The first tickbox is ticked, but the last one is not.

      Show
      1. Place attached test.php in the root of a Moodle installation. 2. View the page. 3. Click the first tickbox and the last tickbox. 4. Hit save; the form reloads. EXPECTED: The first and last tickbox should still be ticked, as entered by the user. BEFORE FIX: The first tickbox is ticked, but the last one is not.
    • Affected Branches:
      MOODLE_29_STABLE
    • Fixed Branches:
      MOODLE_29_STABLE, MOODLE_30_STABLE
    • Pull Master Branch:
      MDL-52226-master

      Description

      In MDL-41819, a fix was implemented for the issue with max_input_vars PHP setting.

      Short summary to save people reading through that and related issues: Some Moodle forms have a very large number of fields, which can exceed the max_input_vars variable and therefore the fields towards the end are not received by PHP. This has very bad consequences - the form appears to save correctly with no error, but everything toward the end is treated as unset even if it the user just set it, so it's an undetectable dataloss bug.

      There are good reasons why some Moodle admins might not find it easy/want to change max_input_vars. So there is a workaround in setuplib where the input is manually processed if it exceeds max_input_vars (MDL-41819):

          $max = (int)ini_get('max_input_vars');
          //...
          if (count($_POST, COUNT_RECURSIVE) < $max) {
              // behave normally, otherwise do special processing
      

      However, this solution does not work with the 'advanced checkbox' field (advcheckbox). To review, advanced checkboxes work by having two fields with the same name; a hidden input field, which is always set, plus the checkbox field itself, which is set only if the user ticks the checkbox.

      Supposing the checkbox is not ticked then it has only one field. However if it is ticked then it has two fields. These two fields both count against max_input_vars, but because they have the same name, they will only be stored once in $_POST.

      In other words if max_input_vars is 1000 (default), this will be exceeded if you have, for example, 900 checkboxes of which 101 are ticked. But the code above will not notice because there are 1001 fields for max_input_vars but only (at most) 900 in $_POST.

      Advcheckboxes are used in bulk in the role edit screen (in simple mode). When you have a lot of custom plugins, this error will occur. For example, if there are 600 capabilities and a manager type role has 500 of them ticked, this error would occur. This now occurs on OU systems.

      The best solution I can think of is very simple:

          if (count($_POST, COUNT_RECURSIVE) < $max / 2) {
      

      I will provide a test script and patch etc.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Jan/16