Make perimeter compatible with fast404 module and use DI for ban.ip_manager

The attached patch adds the addition exception handler.

This also has some refactoring, cleanup, and also does the Symfony 4 upgrade (see 🐛 Symfony > 4.4 uses getThrowable Fixed ). I apologize for mixing so many issues. I hope that you can commit this as-is.

Attached patch is for the 2.x d9 branch.

Thank you very much! @Grevil could you review this carefully please?

Updated patches use dependency injection to create the ban.ip_manager; the 3.x patch also has a re-roll so that it applies.

Updated patch has a minor change to remove an unnecessary permission check.

@douggreen thank you, could you please use a MR instead of patches?

Ping, I made a PR a while ago, please re-review.

Wow! Great and clean code! Well done!

This will make the whole class way cleaner and tidier, thanks! 🙂

LGTM!

I would usually ask for a test, BUT since:

1. We currently only have simple installation tests.
2. The test would need fast404 as a dependency

I would be fine without one, what do you think @Anybody?

Thanks, I agree this really looks good. Best would be indeed to have tests. @douggreen could you help with that?

If not, it should please be at least tested manually to ensure nothing breaks. Did you already do that @Grevil?
Tests should be added in ✨ Write functionality tests Needs review please.

No, I have only reviewed the code, since apart from the fast404 support the general code hasn't changed much, just generally "moved" and refactored a bit, to be clearer, but surely I can test this! :)

Just tested it and it works great!

Enable the default "$settings['fast404_exts']" (including the .asp extension) in the settings php.
Enable perimeter without the patch applied and check that the .asp extension will get you banned.
Go to "/bla.asp" → it will throw a fast404 not found
Apply the patch and clear caches
Go to "/bla.asp" → you are banned

RTBC for testing!

I added a notice inside ✨ Write functionality tests Needs review to test this via test case in the future, but for now, I will merge this!

Created a new release!

Automatically closed - issue fixed for 2 weeks with no activity.