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

PHP8 does not like our max_input_vars workaround. Proposed solution - set minimum requirement for max_input_vars

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Testing instructions on PHP7 (Moodle 3.11 and master):

      1. Login as admin and navigate to Site administration > Server > PHP info
      2. Note the value for max_input_vars there, normally it would be default (1000)
      3. Go to Site administration > Server > Environment
      4. Make sure you get a warning message "PHP setting max_input_vars is recommended to be at least 5000."
      5. Change the value for max_input_vars . You can either modify php.ini file or create .htaccess file with contents:

        php_value max_input_vars 5000
        

      6. Return to the Environment page and make sure the warning is no longer there

       

      Testing instructions on PHP8 (Moodle 3.11 and master):

      This is a more advanced test because you actually need to have PHP 8.

      1. Change the max_input_vars to something less than 5000 (note that you will not be able to install Moodle with this setting and also because php 8 is not yet supported)
      2. Go to the Environment page and make sure you see the error message (in red) that says "PHP setting max_input_vars must be at least 5000."
      3. Set max_input_vars to 5000 or more
      4. Make sure there is no error on the Environment page any more

       

      Testing instructions on Moodle 3.10, 3.9, 3.8 and 3.5:

      1. Change the max_input_vars to something less than 5000
      2. Log in as admin and go to Site administration>Server>Environment
      3. Change version to 3.11
      4. Make sure you see the warning message (in yellow)

        max_input_vars 	
        if this test fails, it indicates a potential problem
        PHP setting max_input_vars is recommended to be at least 5000.
        

       

      Show
      Testing instructions on PHP7 (Moodle 3.11 and master) : Login as admin and navigate to Site administration > Server > PHP info Note the value for max_input_vars there, normally it would be default (1000) Go to Site administration > Server > Environment Make sure you get a warning message "PHP setting max_input_vars is recommended to be at least 5000." Change the value for max_input_vars . You can either modify php.ini file or create .htaccess file with contents: php_value max_input_vars 5000 Return to the Environment page and make sure the warning is no longer there   Testing instructions on PHP8 (Moodle 3.11 and master) : This is a more advanced test because you actually need to have PHP 8. Change the max_input_vars to something less than 5000 (note that you will not be able to install Moodle with this setting and also because php 8 is not yet supported) Go to the Environment page and make sure you see the error message (in red) that says "PHP setting max_input_vars must be at least 5000." Set max_input_vars to 5000 or more Make sure there is no error on the Environment page any more   Testing instructions on Moodle 3.10, 3.9, 3.8 and 3.5 : Change the max_input_vars to something less than 5000 Log in as admin and go to Site administration>Server>Environment Change version to 3.11 Make sure you see the warning message (in yellow) max_input_vars if this test fails, it indicates a potential problem PHP setting max_input_vars is recommended to be at least 5000.  
    • Affected Branches:
      MOODLE_311_STABLE
    • Fixed Branches:
      MOODLE_310_STABLE, MOODLE_35_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE
    • Pull 3.9 Branch:
    • Pull 3.10 Branch:
      MDL-71390-310
    • Pull 3.11 Branch:
      MDL-71390-311
    • Pull Master Branch:
      MDL-71390-master

      Description

      Summary:
      Due to the following change in PHP8 - https://php.watch/versions/8.0/startup-errors-enabled
      Moodle site administration (and probably other pages like quizzes) will fail on PHP8 with the default PHP settings, which are:

      max_input_vars = 1000
      display_startup_errors=On
      

      The warning about the max_input_vars will be always displayed and breaks some pages. Our workaround does not help in this case.
      The proposal is to add a requirement for the max_input_vars and remove the workaround, there have been other reports that workaround does not really work well anyway.

      ----- conversation log ------

      Marina, [17.02.21 12:52]
      ^ this one is interesting. I "fixed" it in my local behat by changing the max_input_vars in php settings;
      but it's a fix in moodle-docker, not in the code

      Eloy Lafuente Plaza, [17.02.21 12:53]
      And is it because of a change in php8? I mean, why doesn’t it show with php 7?

      Marina, [17.02.21 12:53]
      ... but my fix broke the tests that actually test the max_input_vars workaround

      as far as I can see in the release docs, the error reporting changed to E_ALL

      https://php.watch/versions/8.0/error-display-E_ALL

      maybe https://php.watch/versions/8.0/startup-errors-enabled too

      Eloy Lafuente Plaza, [17.02.21 12:57]
      but that’s a warning, it’s not strict or deprecated… uhm..

      it’s like now we are passing more variables than with php7 or that the workaround we have has stopped to work?

      just guessing. Maybe it’s more related with the fact that now @ doesn’t silences all the things?

      Eloy Lafuente Plaza, [17.02.21 13:04]
      ah ok, the error is in the session handler that tries its own init_set.

      In any case, I’m not sure why php8 says the limit has been raised and php7 does not. I’ve here E_ALL | E_STRICT | E_DEPRECATED (that now is not needed) since ages ago and never have got that problem.

      looking the default values in php.ini ...

      heh, i’ve max_input_vars = 20000

      default was the same in both 7.x and 8.x (1000)

      Marina, [17.02.21 13:19]
      It's just PHP now displays warning and then it breaks code that tries to do something before output

      Because it's no longer before output

      But there are tests that actually expect small value for max input vars

      Eloy Lafuente Plaza, [17.02.21 13:21]
      yeah, but then visiting those pages having > 1000 fail

      because it’s detected too early

      and now reported

      Marina, [17.02.21 13:22]
      Correct

      Eloy Lafuente Plaza, [17.02.21 13:22]
      and… if somebody can edit php.ini to set display_startup_errors, then for sure can also edit max_input_vars

      problem is people not able to edit that

      Marina, [17.02.21 13:24]
      How can somebody hosting Moodle wouldn't be able to edit PHP settings?

      Eloy Lafuente Plaza, [17.02.21 13:24]
      well, don’t know… then… why do we have that workaround for max_input_vars ?

      Marina, [17.02.21 13:25]
      No clue

      Eloy Lafuente Plaza, [17.02.21 13:25]
      I always assumed it was because of ppl not having access to php.ini

      Marina, [17.02.21 13:26]
      Partners say they have to edit max input vars everywhere because otherwise quizzes fail

      Eloy Lafuente Plaza, [17.02.21 13:26]
      aha, the recommended php.ini-production comes with:

      display_startup_errors = Off

      so it’s the php.ini-development one

      Marina, [17.02.21 13:27]
      ... so our workaround code doesn't really work... I think I saw some issues in tracker too

      Eloy Lafuente Plaza, [17.02.21 13:27]
      i’ve also max input vars set in all my prod sites.

      maybe we could consider making it an environmental requirement.

      it’s not the end of the world

      Marina, [17.02.21 13:28]
      Making what?

      Eloy Lafuente Plaza, [17.02.21 13:28]
      requiring "max input vars” > X

      pretty clear for everybody

      and then kill the workaround if possible and done.

      Marina, [17.02.21 13:29]
      Yeah good idea

        Attachments

        1. behat_73.png
          behat_73.png
          31 kB
        2. behat_80.png
          behat_80.png
          30 kB
        3. MDL-71390-1.png
          MDL-71390-1.png
          7.11 MB
        4. MDL-71390-2.png
          MDL-71390-2.png
          6.15 MB
        5. MDL-71390-3.png
          MDL-71390-3.png
          7.33 MB
        6. phpinfo_73.png
          phpinfo_73.png
          57 kB
        7. phpinfo_80.png
          phpinfo_80.png
          51 kB
        8. We-can-change-what-we-are-viewing-on-the-grader-report_I-set-the-following-system-permissions-of-Teacher-role-_91.png
          We-can-change-what-we-are-viewing-on-the-grader-report_I-set-the-following-system-permissions-of-Teacher-role-_91.png
          80 kB

          Issue Links

            Activity

              People

              Assignee:
              marina Marina Glancy
              Reporter:
              marina Marina Glancy
              Peer reviewer:
              Paul Holden Paul Holden
              Integrator:
              Sara Arjona (@sarjona) Sara Arjona (@sarjona)
              Tester:
              Huong Nguyen Huong Nguyen
              Participants:
              Component watchers:
              Andrew Lyons, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze, Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                10/May/21

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 4 hours, 45 minutes
                  4h 45m