backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

[UX] set warning messages if max_input_vars is not sufficiently large

Open jenlampton opened this issue 5 years ago • 92 comments

Description of the bug

I just got bitten by the problem where PHP's max_input_vars is set to 1000 by default. This is a really frustrating bug because there are no errors or warnings when things go wrong, things just act funny when not all the values in a node form (for example) are passed through $_POST.

This problem most commonly comes to bite you if:

  • You have more than 3 user roles, and/or a lot of modules (on the permissions form)
  • You have a lot of user roles or modules (on the permissions form)
  • you have a lot of taxonomy terms in one vocab (on term reorder page)
  • you have a lot of menu links in one menu (on menu link reorder page)
  • you have a lot of fields on a node (on node form)
  • you have a lot of components on a webform (on webform)
  • you have a lot of conditionals on a webform (on webform conditions settings)
  • you have a lot of values in a multi-value or paragraphs field (on node form)

We should warn people when this value hasn't been adjusted appropriately.

Options for where/how to notify people of the problem with their Backdrop site:

  • In the status report since it is an installation requirement (and set automatically when possible).
  • On a form that has more form elements than max_input_vars (to warn people that form won't work properly)
  • After submission of that has more form elements than max_input_vars (letting people know the form submission has failed)

Here's an example of what the status report could look like:

warning (when server is misconfigured) Screen Shot 2020-09-12 at 12 53 08 PM

info (when server is properly configured) Screen Shot 2019-10-28 at 2 17 46 PM

Here's an example of an error message on a failed form submission: Screen Shot 2020-11-12 at 3 04 01 PM


##How to test

  1. Set your max_input_vars in php back to 1000 (you can do this by editing the .htacess file and removing the "7" from the end of the current value)
  2. Add 5 user roles to your site
  3. Visit the permissions matrix at /admin/config/people/permissions

PR: https://github.com/backdrop/backdrop/pull/2955

jenlampton avatar May 30 '19 02:05 jenlampton

Related Drupal core issues:

https://www.drupal.org/project/drupal/issues/1203766 (specific to the permissions page) https://www.drupal.org/project/drupal/issues/1565704

Additionally, the same problem seems to be happening in many contrib projects:

https://www.drupal.org/project/cat/issues/2403993 https://www.drupal.org/project/features/issues/2754005 https://www.drupal.org/project/menu_editor/issues/2950256 https://www.drupal.org/project/gathercontent/issues/2948437 ...

klonos avatar May 30 '19 03:05 klonos

This one got me again today... PR on the way.

jenlampton avatar Oct 28 '19 20:10 jenlampton

Interesting, it looks like we can fix this in .htaccess for all sites, and we already have for those running PHP 5. See the related drupal issue.

This PR includes a fix for PHP 7, as well as the warning in the status report for those not using Apache.

I could use some help with the language, but here's what I'm starting with...

Name of the setting: PHP form input max

Error message when not properly set: PHP limits the number of form values to 1,000 items.

Description when not properly set: When set too low, PHP's max_input_vars setting can cause silent failures on large forms. Use caution when using the permissions page, ordering large menus, arraging large vocabularies, or editing content with lots of fields.

Message when properly set: PHP limits the number of form values to 10,007 items. This number should be sufficient.

error: Screen Shot 2019-10-28 at 2 17 21 PM

info: Screen Shot 2019-10-28 at 2 17 46 PM

jenlampton avatar Oct 28 '19 21:10 jenlampton

Name of the setting: PHP form input max

Perhaps PHP form input limit?

Typo on arraging in the description.

I like the rest of the wording 👍

klonos avatar Oct 28 '19 22:10 klonos

Thanks for this @jenlampton 👍. Please check my comments in the PR.

...in the meantime, I have closed #3587 as a duplicate of this issue here 😉

klonos avatar Oct 28 '19 22:10 klonos

Is the minimum Apache version required specified somewhere?

bradbulger avatar Oct 29 '19 16:10 bradbulger

I don't think there is a min version? https://backdropcms.org/requirements

jenlampton avatar Oct 29 '19 16:10 jenlampton

I think I was looking at the wrong PR or an outdated version, with an IF condition in .htaccess.

bradbulger avatar Oct 30 '19 15:10 bradbulger

There are ifs for php modules in .htaccess, maybe that's what you were thinking of?

jenlampton avatar Oct 30 '19 19:10 jenlampton

Yes. But if I am reading PR: backdrop/backdrop#2955 right it looks like the simple solution, a mod_php5 block and a mod_php7 block, which should be fine.

bradbulger avatar Oct 30 '19 19:10 bradbulger

Looks good, two items of feedback:

  • Can we put <IfModule mod_php7.c> first? We'll eventually remove the PHP 5 block, it should be given secondary treatment.
  • The addition to hook_requirements() is great, but we should provide a link to documentation on how to increase the limit.

quicksketch avatar Nov 07 '19 01:11 quicksketch

When running updatedb from drush, the system module warning about max_input_vars is triggered, because command line PHP doesn't read .htaccess settings. It will show every time updatedb is run. I don't know how much that matters. Could this make a difference to tests run from the command line, for instance?

bradbulger avatar Jan 23 '20 16:01 bradbulger

This works as expected. Still NW in order to move the php7 block in .htaccess before the php5 block + some wording suggestions.

As for the documentation, should that go under the PHP section in https://backdropcms.org/requirements ? We've set the default value in .htaccess to 10007, but what is the minimum safe/recommended in case people cannot set it as high as that? 5000?

klonos avatar Jul 24 '20 23:07 klonos

but we should provide a link to documentation on how to increase the limit.

I'm working on a documentation page at https://api.backdropcms.org/max-input-vars

jenlampton avatar Aug 06 '20 20:08 jenlampton

Documentation looks good 👍 ...thanks @jenlampton

klonos avatar Aug 06 '20 21:08 klonos

I'm working on a documentation page at https://api.backdropcms.org/max-input-vars

Thanks @jenlampton! One thing we need to check: I don't think setting max_input_vars in settings.php works. Backdrop sets a number of INI values in the bootstrap process, but max_input_vars isn't one of them because it has to be set before PHP executes anything at all. My understanding is that this is because the global $_POST variable is set based on this value, increasing it after PHP has already started can't restore the truncated values to $_POST. So we should either delete that section of the documentation, or perhaps better yet explain why doing it in settings.php won't work.

quicksketch avatar Aug 13 '20 20:08 quicksketch

I pushed an update to the PR with the changes @klonos and I recommended: https://github.com/backdrop/backdrop/pull/2955

Docs still need to be tweaked per above comment.

quicksketch avatar Sep 03 '20 19:09 quicksketch

I updated the docs page with how to set the value in .htaccess, and in httpd.conf, and I removed the bit on settings.php (and added why) https://api.backdropcms.org/max-input-vars

Latest code looks fab!

Latest test works great! I didn't even know my settings for php version 7.3.9 were too low until testing this PR ❤️

Screenshot of what happens when new line in .htaccess is removed. Screen Shot 2020-09-03 at 6 52 46 PM

jenlampton avatar Sep 04 '20 01:09 jenlampton

The grammar isn't quite right here: When set too low, the <code>max_input_vars</code> PHP setting can cause silent failures on forms with a large number of input fields, such as the permissions page or structuring large menus.

To simplify: X can cause Y on forms such as structuring large menus. The problem is the addition of structuring large menus. I wonder if that adds much value here?

How about we shorten it to: When set too low, the <code>max_input_vars</code> PHP setting can cause silent failures on any form with a large number of input fields, such as the permissions page.

I changed on forms with to on any form with to indicate that there are many of them, but then removed the second example.

Another alternative might be something like When set too low, the <code>max_input_vars</code> PHP setting can cause silent failures on forms with a large number of input fields. Examples include the permissions page, the 'Manage links' page for large menus, and the 'List terms' page for large vocabularies.

This seemed to verbose to me, but feedback welcome :)

jenlampton avatar Sep 04 '20 01:09 jenlampton

I would mark this RTBC but I'd like someone else to review the final language for the status report :)

I pushed an updated PR that contains the following: When set too low, the <code>max_input_vars</code> PHP setting can cause silent failures on any form with a large number of input fields, such as the permissions page.

jenlampton avatar Sep 04 '20 01:09 jenlampton

In my opinion there is one issue with this PR but I'm not sure how to address it:

  • Some people may never have an issue with a value of 1000 but they'd see the big red error all the time. Does the message has to be an error, or would a warning be sufficient? (As far as I know, we don't have dismissible messages so far.)

More questions:

  • If we see a low max_input_vars value as a requirement error, should we also mention the requirement on https://backdropcms.org/requirements?
  • Regarding documentation on the API page: Will one of the approaches actually increase max_input_vars, even on various shared hosting services? If not sure, maybe add something like "If that doesn't help, ask your hosting provider".

olafgrabienski avatar Sep 04 '20 07:09 olafgrabienski

Regarding documentation on the API page: Will one of the approaches actually increase max_input_vars, even on various shared hosting services? If not sure, maybe add something like "If that doesn't help, ask your hosting provider".

Almost all Apache-based hosting providers respect .htaccess settings, so for most sites Backdrop will work with the out of box .htaccess file. I've also seen a lot of shared hosting providers allow you to create a custom php.ini file in your home directory that could also be used.

quicksketch avatar Sep 10 '20 20:09 quicksketch

Almost all Apache-based hosting providers respect .htaccess settings

That's right.

But one remark of @olafgrabienski is also true:

Some people may never have an issue with a value of 1000

I only had problems once with D7 and that had really huge ajax based forms. So there could be rare (?) cases when the big fat red error message appears, but the admin couldn't do anything about it. And they wouldn't have any trouble with the default value at all.

Maybe one possible solution: allow to override this in settings.php? Like, for example, is possible in D7 with the (often wrong) "HTTP request status"? Or is this too much effort?

indigoxela avatar Sep 11 '20 06:09 indigoxela

What if it was a warning instead of an error?

On Thu, Sep 10, 2020, 11:36 PM indigoxela [email protected] wrote:

Almost all Apache-based hosting providers respect .htaccess settings

That's right.

But one remark of @olafgrabienski https://github.com/olafgrabienski is also true:

Some people may never have an issue with a value of 1000

I only had problems once with D7 and that had really huge ajax based forms. So there could be rare (?) cases when the big fat red error message appears, but the admin couldn't do anything about it. And they wouldn't have any trouble with the default value at all.

Maybe one possible solution: allow to override this in settings.php? Like, for example, is possible in D7 with the (often wrong) "HTTP request status"? Or is this too much effort?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop/backdrop-issues/issues/3824#issuecomment-690906176, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBERZ5LUEIJHUKNLQTSQ3SFHAOHANCNFSM4HQ2LIQQ .

jenlampton avatar Sep 11 '20 06:09 jenlampton

Even warning seems like call to action which is unnecessary for most users. Information icon with link to the correct documentation (or some kind of FAQ) can be useful instead.

findlabnet avatar Sep 11 '20 07:09 findlabnet

So there could be rare (?)

So far, out of 400+ GovCMS sites, we've had only 7 sites report issues caused by too low max_input_ _vars value. All of these sites had either one or both of the following:

  • HUGE menus. When trying to reorder menu items, the checkboxes in the "Enabled" column in the menu links edit form cause the page to have more inputs than the limit. Using https://www.drupal.org/project/bigmenu solves this issue, as it loads and submits only portions of a menu.
  • Many user roles, with many modules enabled. This causes the permissions page to have so many checkboxes, that it hits the max input limit. No contrib module solves this issue - increasing the max_input_vars limit is the only solution/workaround (or suggesting that the customer removes some roles).

klonos avatar Sep 11 '20 13:09 klonos

Some more real life feedback: I've encountered the issue only once so far, caused by a webform with many (but really many) conditionals.

olafgrabienski avatar Sep 11 '20 14:09 olafgrabienski

The problem is extra frustrartng since the failure is completely silent. Most people won't even know their site isn't submitting all the values it needs until things are horribly corrupted, or lots of data has been lost. It's a really terrible problem.

Since the number of people who aren't using apache is very small, and the number of people who won't be able to change the setting on their own is a fraction of that fraction, showing these people a warning shouldn't be a cause for concern.

On Fri, Sep 11, 2020, 7:15 AM Olaf Grabienski [email protected] wrote:

Some more real life feedback: I've encountered the issue only once so far, caused by a webform with many (but really many) conditionals.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/backdrop/backdrop-issues/issues/3824#issuecomment-691120246, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBER6G7VSYSUZOSJECL5DSFIWG7ANCNFSM4HQ2LIQQ .

jenlampton avatar Sep 11 '20 16:09 jenlampton

So, I must change the setting which is not relevant for me just to not see this annoying warning sign?

findlabnet avatar Sep 11 '20 20:09 findlabnet

You might find it annoying, but it is an important warning for everyone who is affected.

the setting which is not relevant for me

Just because it's not relevant for a particular site right now does not mean that it will always not be relevant. One day your site might grow, or you may need to include a long and complicated survey.

If you are comfortable with taking the risk of loosing data and/or having things like your permissions, menu, taxonomy terms and webforms becoming corrupted, then you do not need to make the change. But, when your data is lost, at least we warned you that it would happen by providing a useful message on the status report telling you exactly what is happening :)

jenlampton avatar Sep 11 '20 20:09 jenlampton