Improved split Request to headers and body phase, use apr_brigade to deal with request body

[chaizhenhua]

Added response Headers/Body phase.

[brenosilva]

Thanks for the patch. I will apply and run some tests.
Let me just know if there is an specific reason (bug you found ? ) to change ngx_chain to apr_* ?

[brenosilva]

Just doing some tests. I'm trying to upload file (+500Kb) and it is not working. Only very small files can be uploaded. Can you check?

Also when i add this rule:
SecRule REQBODY_ERROR "!@eq 0"
"id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"

I saw the message:
2013/01/15 14:07:18 [error] 11898#0: [client 192.168.0.102] ModSecurity: Warning. Match of "eq 0" against "REQBODY_ERROR" required. [file "/usr/local/nginx/conf/modsecurity.conf"] [line "54"] [id "200001"] [msg "Failed to parse request body."] [data "Multipart parsing error: Multipart: Final boundary missing."] [severity "CRITICAL"] [hostname "standalone"] [uri "/upload_file.php"] [unique_id "12345"]

[chaizhenhua]

hi,

nginx use nonblocking event driven io module, so we cant read request/response body directly in processing phase.
instead we have to buffer input/output body until all is saved and then pass to modsecurity, if we use brigade we can reduce memory copy(read cb will copy memory from nginx to modsecurity, while write cb will copy memory from modsecurity to nginx). this is why brigade conversion is used.

[chaizhenhua]

The error has been fixed with patch Fixed multipart error

[brenosilva]

Hello,

The patch looks fine. Could you merge it again to the trunk ? Trunk is a different (2.7.2) and it will fail to merge. Maybe submit a new pull ?

Thanks

Breno

[chaizhenhua]

Hi, Breno
I have submit a new pull #34.

[brenosilva]

Hello,

Just tested the new trunk with your patch and when SecRuleEngine On and a rule like SecRule REQUEST_BODY "price" "phase:2,id:113,deny" it is not blocking the transaction anymore ad return 403.

Could you take a look ?

Thanks

Breno