When being too greedy with selecting permissions you can break the module

Thanks for raising the issue @eelkeblok. I manage a site with a large number of permissions, and am able to easily replicate the issue — a WSOD on the core permissions form was the reason we started using Filter Perms. The error I get is:

Fatal error: Allowed memory size of ### bytes exhausted (tried to allocate ### bytes) in /var/www/docroot/core/includes/theme.inc on line 1047

The render array just gets too big when there are a lot of permissions. I assume your logs would reveal something similar. Anyway, your suggestion makes sense to me and MRs are welcome!

Side note: Looks like the previous filter values are stored for a hour, so at least the page should be usable again in a relatively short period of time.

One other thought. Filter Perms already disables the submit button when the form input count exceeds max_input_vars, but this is based on a count of the actual inputs after the form is already rendered. It seems like we could calculate the input count based on the number of permissions selected before the form is rendered, and bail out early if there are too many. I suspect that would also prevent most/all out-of-memory issues.

I just noticed the Filter Perms is currently Maintenance only status, so let me amend my "MRs are welcome" to "MRs will be considered"?

I think this would be a worthy update. Maybe the module owner will weigh in.

The max_input_vars is pretty easy to check against the permissions form to make sure that you aren't going to submit a form that contains too many permissions checkboxes (though the module makes some guesses about the number of non-checkbox inputs on the form). Dealing with out of memory errors would be a different beast as there wouldn't be a good way to know how many permissions would lead that error. Out of curiosity, what kind of numbers for roles/permissions and memory limits are causing this problem?

$this->keyValueExpirable->setWithExpire($this->currentUser()->id(), $values, 3600);

The selections are saved in that manner, so I don't think there is any cache clearing that helps there and as justcaldwell mentioned, it will take an hour to expire for that user.

I can see where a solution to this problem does line up with what this module is meant to help with, but I'm not sure of a good solution. Maybe something like only storing that key/value through AJAX after the form successfully renders, which doesn't prevent the error but would give a clean slate after it occurs.

I'm having the same issue. Installed this module to prevent the Permissions page from breaking but selecting All Roles and All Permissions will break the page and then it's stuck like that. I'd rather not have it "remember" the previous settings so revisiting /admin/people/permissions would take it back to the unselected state. In my case I don't think it's the max input vars, but rather a PHP timeout on the amount of time the page can take. And since we're on a particular platform that doesn't allow us changing this value we're stuck. Thoughts on making the "remember" state optional?

Hi @nsciacca - I think remembering the filter settings between submissions is essential to the way the module functions.

@cyu - regarding your question in #5:

Out of curiosity, what kind of numbers for roles/permissions and memory limits are causing this problem?

We have 865 permissions and 54 roles (yikes!).

I just tried 'All modules / All permissions' again on my local with a memory limit = 1024M and it looks like the form actually begins to render for me (after quite a long time). It's the browser (Chrome in this case) that chokes on the results and ends up giving the 'Oh snap, something when wrong' message after partially rendering the page.

On our actual web servers, the memory limit on admin routes is 512M, and the page exhausts the memory and fails quite a bit sooner with no attempt to render.

@cyu: Some initial testing of this approach worked for me. If we $form_state->setRebuild() in the submit handler, the submitted values will be still be available on subsequent loads, so $filter can just draw from those values and drop the KeyValueStore entirely. The submit handler would become:

public function submitFormFilter(array &$form, FormStateInterface $form_state) {
    $form_state->setRebuild();
}

Then $filter becomes:

$filter = [
    'roles' => $form_state->getValue('roles', []),
    'modules' => $form_state->getValue('modules', []),
];

See: FormState::$rebuild and FormState::setRebuild

The form 1) would no longer "remember" the previous filter if you navigate away from the permissions page AND 2) the render will still fail if too many permissions/modules are selected, BUT the page will be immediately usable if you navigate away and come back to it.

The solution for #2 is user training, unless we add some generic warning text about selecting too many permissions/modules.

@justcadwell thanks for your suggestion. I forked the issue, implemented your suggestion, it's working nicely for me. The only annoying part is that when you submit the page it goes back to unselected state. Meaning if you were incrementally doing changes on a role or comparing two roles with specific modules selected you have to redo those selections after save. Any thoughts?

https://git.drupalcode.org/issue/filter_perms-3457584

Hi @nsciacca. Thanks for starting the MR! Yeah, I ran into the same issue this morning. I added the fix to the MR, as well as some additional cleanup of KeyValueStore-related code.

Also, PermissionsRoleSpecificForm.php depends on saveFilterSettings(), so that needed to be addressed. Since a permission filter has been added to Drupal's core form, I don't think it's necessary for filter_perms to override the role-specific form any longer — so I removed it and the route subscriber.

We'll have to see how the module owner feels about all that 🙂. Setting to NR.

justcaldwell → closed merge request !6

MR 8 is an alternative solution that makes fewer changes to the module, retains the KeyValueStore approach and the role specific form/route handling. It just discards any stored settings after each use — settings are re-loaded/updated as usual on submitting the filter. I kept the store as expirable in case something unpredictable occurs after the form is submitted, but reduced the time to 5 minutes.

Either approach ensures the permissions page is immediately usable if the user returns to it after attempting to load too many values (or within 5 minutes if something else went wrong).

Thanks for the work on this. The original proposed solution would be one that I'd like to merge in, but I'm glad there is a working patch for folks that are running into this now.

If I'm understanding MR 8 correctly, for folks that aren't running into issues right now, this patch seems like it would have a slight negative effect as there would be new situations where saved selections would be wiped out. I didn't test this out, so maybe that impression is incorrect.

The original proposal, as I'm picturing it, would have no effect on users that use the module without issue right now while giving users that are able to generate errors an easy way out (after a failed load of the permissions page, it would return to defaulting to showing nothing with a helpful message about why it ended up in this state).

Until someone provides an MR for that feature, I recommend the approach in MR 8 for folks who need a resolution to the issue in the meantime. To that end, I'm attaching a static patch based on the MR.

Updated IS

justcaldwell → changed the visibility of the branch 3457584-remove-keyvaluestore to hidden.

Going to update the title to add some context.