-
Notifications
You must be signed in to change notification settings - Fork 300
added disable error log support #360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
modified old PR owasp-modsecurity#327 Signed-off-by: Fatih USTA <fatihusta86@gmail.com>
Hi @fatihusta,
many thanks for update the mentioned patch.
Could you add a new test case to the CI workflow, like this one? Just turn on this new feature, send an attack which triggers a rule, and check that the error.log is empty.
Thanks!
Thank you for this. I'm already using it :)
My only feedback would be that the name of the variable is a bit confusing, you turn it ON to turn something OFF. modsecurity_use_error_log with default on would maybe make more sense?
Just my two cents
Signed-off-by: Fatih USTA <fatihusta86@gmail.com>
@fatihusta thanks for adding the test.
What do you think about @tomsommer's idea. I think you should consider it - I agree with him, the current implementation has a bit weird logic.
Also, after we agreed what should be the final keyword, please add the documentation into our README (README is part of the repository).
- tests are changed with new directive name - nginx.conf updated with new directive name - added doc Signed-off-by: Fatih USTA <fatihusta86@gmail.com>
Quality Gate Passed Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Hi
I changed the directive name as modsecurity_use_error_log. Default is on.
Thanks @tomsommer @airween
Hi I changed the directive name as modsecurity_use_error_log. Default is on.
Thanks - I'm going to check this soon. Until then, could you add this new keyword into README.md?
Hi I changed the directive name as modsecurity_use_error_log. Default is on.
Thanks - I'm going to check this soon. Until then, could you add this new keyword into README.md?
I already added modsecurity_use_error_log key into README.md.
modified old PR
#327