Here's a patch for this MR which adds support for the imagick toolkit, if you find it useful.
@catch absolutely, I wasn't aware of this core issue. I can confirm the patch fixes the issue (at least my case). Thank you.
I saw some infinite recursions in one of my projects and couldn't figure out what is going on until today. I am finally able to reproduce this problem and have an explanation for why it happens. And yes, it happens on a fresh installation, but it a (very small) customization for it to happen.
My particular case:
- Automated bots were hitting my website with drupalgeddon hacking attempts. This particular bot was POSTing on the homepage using these query params: name[post_render] and name[markup]. After sanitizing these inputs, drupal will delete everything and put an empty name[] array in the query parameter bag. All fine so far.
- As it happens, I also print a form on all pages (including 4xx) which does a very benign \Drupal::request()->query->get('name')
- Now adding these two non-issues together leads to an infinite loop, as follows: Drupal will redirect to a 404 page, thus making a subrequest to the 4xxController. My code will call query->get('name'), which will trigger an exception, because the empty array that exists in the query is a non-scalar (Symfony\Component\HttpFoundation\InputBag.php:38). Reading this value leads to a new exception which will render an error page, which will again try to read the value from the query, and on and on until it either runs out of memory or triggers a script timeout.
A general case:
- Inside a hook that runs on error pages too, such as a hook_preprocess_html, add the following: \Drupal::request()->query->get('name')
- Perform a request on any page, adding the query parameter without any value: ?name[]
- Watch it crash
Checking isAllowed() with threshold = 1 causes the logs to fill up with Flood control blocked login attempt for uid %uid from %ip messages for all users who try to log in from the second (or more, below threshold) attempt. This is because UserFloodSubscriber::blockedUser() is called on every FALSE result of isAllowed() calls. Should this be addressed in another issue?
The previous patch won't do it, because in case of a missing field, it just goes on to try to load it in line 132.
An additional check before trying to load it fixes this problem, altough I didn't have much time to look into the original cause. If you want a quick solution to get you past this error, try the attached patch.
Re-rolling for the latest version.
Re-rolled patch for Drupal 10.3.6
breidert → credited alex.st → .
Here's a re-roll of patch #2 on the latest 8.x-1.x as the previous one no longer applies.
Patch attached
alex.stanciu → created an issue.
Tests are trying to use 'classy' which no longer exists in D10. Let's try with 'starterkit_theme'.
alex.stanciu → created an issue.
Just wanted to say that we experienced this on a (quite high-traffic) production site today. We saw hundreds of occurences in the logs. With no alternative, we disabled JS aggregation until we have an official solution.
alex.stanciu → created an issue.
Added a deprecation notice and created a (draft) change record https://www.drupal.org/node/3378735 →
Third time's the charm?
Applied this to 11.x and fixed a failing test case.
I reworked this a bit on the 10.1.x branch addressing the previous comments:
- #88: I removed the JS changes, they did not seem necessary and was creating a very long query string. Click sorting persists between AJAX requests just fine. Not sure why we needed it in the first place, but in my testing, it works fine without it.
- #31 (1) I kept the key_value implementation but I added hooks for view_update, view_delete, block_update and block_delete. Whenever we update/delete a view or block, we remove the key_value entry. Thus eliminating the risk of bloat adding up over time
- #31 (2) I changed the key_value structure to be a nested array, keyed by the view id and reworked a bit the function used to generate the hash. Thus eliminating the risk of colisions.
- The last patch was not working at all on 10.1.x because it was using
->getRequest()->request->get()I changed it to->getRequest()->query->get()
alex.stanciu → created an issue.
alex.stanciu → made their first commit to this issue’s fork.
alex.stanciu → made their first commit to this issue’s fork.
alex.stanciu → made their first commit to this issue’s fork.
Patch #7 works but it doesn't fix the initial problem. The problem appears when you are trying to lazy load SVGs. This can work if the SVGs are wrapped in an <img> tag, but with this patch, they will simply be ignored. It's better to use $element['#items'] to load the file ID, which is the same even when we have ['#markup'] instead of the expected render array for files.