Skip to content
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

headers/modsecurity/rules_properties.h - temporary prevent (O)n issue… #1667

Closed

Conversation

vaLski
Copy link

@vaLski vaLski commented Jan 26, 2018

… while parsing modsec rules - this should fix #1663

@zimmerle zimmerle self-assigned this Feb 23, 2018
@zimmerle zimmerle added this to the v3.0.1 milestone Feb 23, 2018
@zimmerle
Copy link
Contributor

Hi @vaLski,

Thank you for the patch.

Disable the rule id duplication may not be the right approach here as it is necessary due to other reasons. Also, the integer comparison inside the loop should not be the responsible for the numbers on #1663. It may trigger a problem in a race condition, but the problem is likely to be elsewhere. Therefore, I am closing this issue without merge.

@zimmerle zimmerle closed this Feb 23, 2018
@vaLski
Copy link
Author

vaLski commented Feb 26, 2018

@zimmerle I totally agree that disabling this code is not the best approach to solve this issue. Hence my C skills are limited this was the easiest approach for me to try.

Despite that fact, I am sure that this is the code that is triggering the issue I described in #1663 and more specifically the problem with the slow syntax check (nginx -t) and slow nginx start/reload.

Since I applied this patch (code comment) I am not experiencing this issue. Try this with 3k mod security rules and 1000+ vhosts and you will see that too. I bet a beer this is the code that is causing the slowdown during start/restart/reload/configtest.

@p0pr0ck5
Copy link
Contributor

p0pr0ck5 commented Apr 9, 2018

x-ref: #1735.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Huge memory consumption and slow nginx configtest/start/restart with ModSecurity v3 and ModSecurity-nginx
3 participants