Closed (fixed)
Project:
Ban
Version:
1.1.x-dev
Component:
Code
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
30 Mar 2026 at 07:24 UTC
Updated:
30 Apr 2026 at 08:05 UTC
Jump to comment: Most recent
Problem/Motivation
Rewrite IP allowlist using a settings.php setting (ban.allowlist) service instead of the current 1.1.x-dev approach using config, which degrades performance too much in middleware. See #3578480: Worth caching allowed IP list in middleware? for details.
Also note @berdir's comment:
I'm fine with something settings or container parameter based.
Unsure when it should apply, on check or on ban, or or both. *If* the cost of checking the IP is near-zero (like settings.php), then there's even a performance improvement possible thanks to this, as the query doesn't need to run (as a follow-ip, it might be worth exploring to make either the ban ip manager in the middleware or the database connection in the ban ip manager a closure. Unfortunately, Drupal already opens the database connection when you inject the service and with this, there's a scenario where no query happens)
And yes, once such a setting exists, other modules could support it and don't block those IP's in the first place.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork ban-3582177
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
anybodyComment #3
anybodyComment #4
berdirJust to give a practical example why performance of the ban middleware matters, specifically also on blocked responses:
A few days ago, for two hours, one of our websites registered a 10x increase in requests, 90% of those were from IP's that perimeter automatically blocked, so they were all ~10ms responses. About 50% of the requests of the whole day, enough that the median response time for the whole day dropped to 11ms. This barely registered on the server resource usage because ban responses are so fast compared to regular responses, and it's important that it remains this way.
This was still on D10, but if you look at the page cache response comparison on https://www.md-systems.ch/en/blog/2025-12-16/performance-improvements-dr... (ban is similar), but on 11.2, this would have resulted in twice as much CPU being used (which is still much faster than if they would not have been blocked).
Comment #5
anybodyThanks for the enlightening insights @berdir! I totally agree and we'll probably implement this today :)
Comment #8
grevil commentedAll done, please review!
One more thing @berdir, @anybody thought about using a simple ban service, which wraps around the "Settings::get('ban_allowlist', []);", so other modules can simply use this service instead off relying on "Settings::get('ban_allowlist', []);" directly. Do you think that'd make sense? It would be again another (although really lighweight) service which needs to be injected in the Middleware.
Comment #9
anybodyThanks @grevil! Nice! I left a bunch of comments. Would be great if @berdir could also check if anything else needs to be addressed or sign this off before merge.
Comment #10
berdir> Do you think that'd make sense? It would be again another (although really lighweight) service which needs to be injected in the Middleware.
I think the overhead of one extra class/service with no dependencies is minimal. Someone could implement a more complex/dynamic allowlist-provider then if they're willing to live with the possibly slower performance.
It would make BC more complicated though, you'd then switch the passed argument with something else, would need to be optional and without type. Also makes further changes a bit more complicated. Right now only the drush ban commands seems to respect the allow list and not the UI, that seems pretty inconsistent, this might make more sense within the BanIpManger? With a setting, you can just move the code, with DI, you again need to remove and add the service in a different place. Your call if that is worth it.
Comment #11
grevil commentedAlright, all done!
Comment #12
anybodyI created #3582759: Add a ban.allowlist service as follow-up to be done whenever someone needs it. YAGNI until then ;)
Will review this finally now and merge it then for a beta2 release today, if @berdir is fine with that.
Comment #13
anybodyLeft two final comments. RTBC once fixed.
https://www.drupal.org/docs/8/understanding-drupal-version-numbers/what-... says:
so I think the parameter removal is acceptable this way with ~20 installations. Next time we should leave breaking changes in alpha for longer in such a widely used module.
Comment #14
anybodyFinally added some more IPv6 tests and mixed tests, if they go green we can merge this finally and should be really well covered :)
Comment #15
grevil commentedFixed the phpcs issues and made the pipeline failure a bit more strict. Merging!
Comment #18
anybody