Core interfaces can go over max_input_vars

Created on 7 May 2012, over 12 years ago
Updated 2 September 2024, 3 months ago

Problem/Motivation

Core interfaces can easily exceed common configurations for max_input_vars
e.g.

  • editing a menu with a lot of links
  • the permissions page

Steps to reproduce

Set max_input_vars to a low value
Try to submit changes to the permissions form

Proposed resolution

Add a hook_requirements that checks the number of permissions x the number of roles and then compares that to 80% of max_input_vars. If its higher, show a requirements error advising to increase the ini option
Show the same message on the permissions page when we know the form won't be able to submit
Show a similar message for the menu link edit page where it would be number of links x 3 (one for each of enabled, parent and weight) and again 80% to allow for form alters etc

Remaining tasks

All of the above

User interface changes

API changes

Data model changes

Release notes snippet

I've setup a content type having several field groups configured as horizontal tabs with field collections inside it (several ones including text, dropdowns, date, etc) one field collection for each field group (tab)

Im using:
Field collection 7.x-1.0-beta3+4-dev
Fieldgroup 7.x-1.1
File entity 7.x-2.0-unstable3+21-dev
Entity API 7.x-1.0-rc2+0-dev
Field Collection Table 7.x-1.0-beta1

Everything is working fine except for the issue of having 1 added empty row for each time I save the node. I know there's a bug report about that and Its in progress.

When I hit the remove button on each row to remove those empty lines (from the bottom-up), happens this:

1st row: deleted OK

2nd row: deleted OK but I got the following on the FF javascript console:

Error: $("#" + base, context).once is not a function
Source File: (obscured)/node/123/edit
Line: 16

3rd row: looks like goes to server, but when the "working" animated gif dissapears, nothing happens and I got this on javascript console:

Error: $("fieldset.path-form", context).drupalSetSummary is not a function
Source File: (obscured)/sites/all/modules/pathauto/pathauto.js?m3nlsr
Line: 5

Im setting this before that line on the other module's scripts

if (typeof jQuery.fn.drupalSetSummary == 'undefined') {
  return;
}

and works, but then another module (like metatag) that uses drupalSetSummary raises the error. etc.

Looks to me like jQuery is breaking and then everything that depends on it cease to work.

Plus, if I try to save the node on this instance (when remove button stopped working) im getting "An illegal choice has been detected. Please contact the site administrator."

I cannot find $("#" + base, context).once on the module's code to see whats going on. Any help?

Edit:
Additional data: http://drupal.org/node/445130#comment-2851276
/misc/form.js is loaded on my edit page when I see the source code, but maybe when this module makes ajax calls somehow the drupalSetSummary() function is removed or something.

Thanks for reading this.

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
FormΒ  β†’

Last updated about 10 hours ago

Created by

πŸ‡¨πŸ‡¦Canada seaarg

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡«πŸ‡·France nod_ Lille

    One idea that works only with JS enabled is when submitting the form, adding disabled to all unchecked checkboxes, that will exclude them from the POSTed values. Not sure how well it'd work when unchecking a permission…

  • πŸ‡¬πŸ‡§United Kingdom catch

    I think #53 is worth looking into, we've already had this but for years with or without js and making it work js-only would be a big improvement over not at all.

  • Merge request !9392Resolve #1565704 "Remove empty checkboxes in JS" β†’ (Closed) created by nod_
  • πŸ‡«πŸ‡·France nod_ Lille

    I'm not sure we can help more on the frontend side, browsers only submit checked checkboxes already. So on my 4000 checkbox pages, there are only the 143 checked checkboxes sent in the POST request.

    browsers may have gotten smarter about all this since then.

  • πŸ‡ΊπŸ‡ΈUnited States tedfordgif

    Aren't there "shadow" hidden inputs as well? In order to get rid of those you'd need to e.g. round-trip a hash of the current state to detect if another user saved permissions in the meantime. Then the front-end could submit only the checkbox transitions. That would require JS as well.

    While we're talking JS, if we send the max_post_vars as a setting, we can stop the submission if we would exceed it.

    Or turn it into a progressive enhancement exercise, where the no-JS form can only look at one role at a time, and only once you enable JS can you edit all roles at once.

  • πŸ‡ΊπŸ‡ΈUnited States tedfordgif

    Hmmm, another terrible idea: I wonder if you could implement the transition idea without JS using radio buttons. If previously checked, make the "checked" input have an empty value, and the "unchecked" input have a value attr that means "remove this permission from the role". You could even use CSS to get close to the current interface.

    I'm guessing the radio value always gets submitted, though, even with a blank value.

    And there are likely to be a11y concerns.

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Reviving this as I just ran into the issue once again (and think it happened to clients > 30 times in the last years).

    Maybe a good first shot would be to ensure a viable minimum for max_input_vars for Drupal installations when installing Drupal and in the status report as warning?

    Just having a mid-size Drupal project, for example the admin menu administration page (/admin/structure/menu/manage/admin) or the permissions administration typically grows > 1000 input elements very fast.
    So before going to be much into details here, adding a hook_requirements() check for max_input_vars being at least 10000, otherwise showing a warning (not an error) to raise attention would be super helpful I think.

    While discussing this for a long time, I think many people are hurt by having a too low default hosting provider value (typically '1000'). What do you think? Everything else as follow-up perhaps?

  • πŸ‡¬πŸ‡§United Kingdom catch

    @anybody while it only improves the permissions page, I think πŸ“Œ Improve performance of the user.permissions.js script running in /admin/people/permissions. Needs review may have improved things there - do you have a site you could confirm that on?

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Hi @catch and thanks for the fast reply! Maybe there's a misunderstanding. My comment is about forms with many inputs not being saved at all, getting people into trouble, not only on the permissions page, but also for example on the menu administration and other pages with many form elements.

    It's not about performance, but a risky (mis)configuration in php.ini for Drupal.

  • πŸ‡¬πŸ‡§United Kingdom catch

    @anybody that issue at least theoretically also reduced the number of checkboxes that the permissions page submits, so it would be good to see what if any effect it has on a real site affected by this issue.

    Agreed the permissions page isn't the only problem here though.

  • πŸ‡©πŸ‡ͺGermany geek-merlin Freiburg, Germany

    This can and should be solved generically in the DB driver.

    We can steal code from ye olde SQLite 999-limit issue: πŸ“Œ Make SQLite faster by combining multiple inserts and updates in a single query Needs work

    Note that there have been several approaches in that issue. The current approach that sends multple values via JSON is really elegant, but likely too SQLite specific. But a former approach had a parameter chunker, that likely every DB driver can use. See #2031261-86: Make SQLite faster by combining multiple inserts and updates in a single query β†’ ff.

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    @geek-merlin: Are you sure this landed in the right issue?

    It's database-related, yes, but what we see here can also happen without any database interaction, what ever is done with the form values. (Of course in most cases they will be stored in a database).

    This issue is about forms with many inputs, so that the sent request exceeds max_input_vars and values are dropped without a warning.

    I'm not sure if this belongs into database system. Guess you maybe worked in several related issues? (That clearly exist)

  • πŸ‡«πŸ‡·France nod_ Lille

    The main issue is at the http/php level

    @tedfordgif we have done all we can at the html level. no more cruft to remove. the shadow inputs are not submitted

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    @nod_ thanks! Could you maybe give a short comment on my suggestions in #60? That could be a quick win to at least introduce a warning if the limit is low?

    Additionally, we could show a warning if a form submitted exactly matches max_post_vars and allow opt-out via settings.php? With a high max_post_vars value this warning would then become improbable.

    Once we have a plan, I think it would make sense to split this into sub-issues for the different points to implement. But it first needs a decision which way to follow.

    Thanks!

  • πŸ‡«πŸ‡·France nod_ Lille

    hook_requirement make sense. i would prevent the form from being submitted with a warning if we know it will go over

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    @nod_ thanks! I don't think we can detect the form is going over, because we simply won't receive values for the values over the limit, but we should elaborate on that!

    Let's add a first subtask then to implement a hook_requirements check for a max_post_vars limit < 10 000 to show on installation and in the status report. I'll ask our team.

    Here we should proceed checking how to detect a form submit that exceeds the limit.

  • πŸ‡«πŸ‡·France nod_ Lille

    we can get the max input var value and check with JS at submit for example. It's a pretty dangerous situation so special casing makes sense

  • πŸ‡¬πŸ‡§United Kingdom catch

    When max_input_vars is exceeded an E_WARNING is triggered according to https://www.php.net/manual/en/info.configuration.php#ini.max-input-vars - could we special-case this in the error handler and then do something with it?

  • πŸ‡ͺπŸ‡ΈSpain tunic Madrid

    Idea from #71 would be great. Using an arbitrary big number doesn't ensure issue is not going to happen.

    Other option is adding the check in the Form API layer. During form rendering the number of variables needed can be known, and raise a warning to the watchdog. #71 idea is easier to implement, I guess.

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    Other option is adding the check in the Form API layer. During form rendering the number of variables needed can be known, and raise a warning to the watchdog. #71 idea is easier to implement, I guess.

    Indeed, I think that would be better than JS and server-side. On the other hand, that could also get complicated with compound elements, but definitely solvable.

    I hope I'll find the time in the next days to check #71 that would be best! Adding a warning to the status report for very low limit still makes sense to me anyway.

Production build 0.71.5 2024