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

Fix SecResponseBodyAccess and ctl:requestBodyAccess directives #1886

Closed
wants to merge 2 commits into from
Closed

Fix SecResponseBodyAccess and ctl:requestBodyAccess directives #1886

wants to merge 2 commits into from

Conversation

victorhora
Copy link
Contributor

Small change to allow for SecResponseBodyAccess and ctl:requestBodyAccess directives to work as intended. Should fix #1531.

@defanator
Copy link
Contributor

@victorhora this PR is breaking current ModSecurity-nginx connector tests:

Test Summary Report
-------------------
modsecurity-scoring.t        (Wstat: 512 Tests: 7 Failed: 2)
  Failed tests:  2, 5
  Non-zero exit status: 2
modsecurity.t                (Wstat: 1024 Tests: 22 Failed: 4)
  Failed tests:  2, 6, 10, 14
  Non-zero exit status: 4
modsecurity-proxy.t          (Wstat: 1024 Tests: 25 Failed: 4)
  Failed tests:  5, 9, 13, 17
  Non-zero exit status: 4
Files=8, Tests=114,  1 wallclock secs ( 0.08 usr  0.00 sys +  1.08 cusr  0.25 csys =  1.41 CPU)
Result: FAIL
Makefile:145: recipe for target 'test' failed
make: *** [test] Error 1

Looking into details now.

@defanator
Copy link
Contributor

@victorhora library tests are failing as well:

============================================================================
Testsuite summary for modsecurity 3.0
============================================================================
# TOTAL: 4760
# PASS:  4658
# SKIP:  23
# XFAIL: 0
# FAIL:  79
# XPASS: 0
# ERROR: 0

@victorhora
Copy link
Contributor Author

Funny @defanator, the tests were working for me before. There's a chance I've pushed the wrong code LOL :P. Let me have another a look at it

@victorhora
Copy link
Contributor Author

Ok, I've pushed the wrong code :P Should be good now unless I screwed up again :P. @defanator If the buildbots pass, please let me know the results of ModSecurity-nginx connector tests. (I can see now we really need Travis integration for those :))

defanator added a commit to owasp-modsecurity/ModSecurity-nginx that referenced this pull request Sep 3, 2018
@defanator
Copy link
Contributor

@victorhora yes, connector tests are now passing. I've just submitted an extension to the request body tests for nginx connector here: owasp-modsecurity/ModSecurity-nginx#124

(note that it should be merged after #1886 gets in the v3/master to avoid Travis failures)

@victorhora victorhora added pr available bug It is a confirmed bug labels Sep 4, 2018
@zimmerle
Copy link
Contributor

Merged! Thanks!

@zimmerle zimmerle closed this Sep 11, 2018
victorhora pushed a commit to owasp-modsecurity/ModSecurity-nginx that referenced this pull request Sep 20, 2018
pracj3am pushed a commit to cdn77/ModSecurity-nginx that referenced this pull request Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x bug It is a confirmed bug pr available RIP - libmodsecurity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

About SecRequestBodyAccess Off and SecResponseBodyAccess Off
3 participants