-
Notifications
You must be signed in to change notification settings - Fork 282
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
[Perf] Don't read full response into mem if not needed NGINX #206
Comments
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days |
This should bot have been closed. Honestly the bot should not even be installed on any of the ModSec repos for how dead the community is around things currently. |
@jeremyjpj0916 by adding the 'nostale' label the issues won't be stalled. |
@jeremyjpj0916 It is very unusual to receive a contribution with pasted code along with the issue. We rather prefer to have it in the format of a pull request, even if that is only there for fomenting discussion. There is a guideline on how to do it here: Creating a pull request. About the inspection in different phases, ModScerutiy has a mechanism to control the flow of phases execution, including the allow action that can skip phases. Specifically for request body and response body, there are some configuration options that can be used:
Those may not be fully integrated within the connector yet, some are still a work in progress. Having said that, what is the benefit of what you have suggested atop of the existent features? And how exactly it is addressing the possibility of having a rule in a phase that is not being executed? |
what i did was a hack, im suggesting those competent with this lib make proper flags in the code to achieve it(I am not competent for those changes). And yes those features mentioned are not integrated within the connector yet. |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days |
Bad bot. |
Regarding this bit here:
https://github.com/SpiderLabs/ModSecurity-nginx/blob/master/src/ngx_http_modsecurity_header_filter.c#L450
Got me thinking, for users that just want to waf protect inbound client requests we really can drop all the phases that execute after the
proxy_pass
directive(besides the log phase of course) if we are talking nginx as a gateway or lb. So I did something like this in code:Basically removed all references to _body and _header filter logic
config
ngx_http_modsecurity_common.h
ngx_http_modsecurity_module.c
And in my build process I removed the filter reference c files:
Then we tested with a 20MB response body json payload(as this mostly should help on perf when dealing with large response/header data). Seemingly we saved about 20ms on average per request doing 11 TPS with the big response with preliminary data.
Rather than forcing folks to gut the code, some kinda flags we can set to achieve what I did above ^^ with the files may be good.
The text was updated successfully, but these errors were encountered: