Skip to content

Conversation

@hnakamur
Copy link
Contributor

what

  • This pull request prevents a parse error that occurs when SecRequestBodyLimitAction is set to ProcessPartial and the length of an XML or JSON request body exceeds the configured limit.

why

  • When SecRequestBodyLimitAction is ProcessPartial, the expected behavior is to process the partial XML, JSON, or multipart body up to the defined limit.
  • The request should then be rejected if any invalid values are found within that processed part, or passed through if the processed part is valid.
  • The previous implementation failed to handle the partial body correctly, leading to a parse error when the limit was exceeded.

references

The same modification for v3 is at: #3476

@airween
Copy link
Member

airween commented Dec 31, 2025

Hi @hnakamur,

thanks for this PR too.

  • When SecRequestBodyLimitAction is ProcessPartial, the expected behavior is to process the partial XML, JSON, or multipart body up to the defined limit.

I think SecRequestBodyLimitAction regards not only for XML, JSON or multipart bodies - but also to www-url-encoded body too. Consider this setup:

SecRequestBodyLimitAction ProcessPartial
SecRequestBodyNofilesLimit 10

and the payload:

aaa=123&bbb=123

then the aaa variable must have seen in ARGS with value 123.

@dune73 what do you think?

The request should then be rejected if any invalid values are found within that processed part, or passed through if the processed part is valid.

How can we be sure that the payload is valid, if the error is in the chunked part? Eg. consider the config above and this JSON:

{"a":1,"b":2,[1]]}

(the second ] makes the JSON non well-formed, but the key a must have seen in ARGS).

You helped me in #3456, which isn't merged yet (I'm waiting for a review and an approve). But could you add a new test case to the v2's source tree?

@hnakamur
Copy link
Contributor Author

hnakamur commented Jan 1, 2026

Hello @airween. Thanks for your comment.

I added tests for url-encoded, JSON, and XML request bodies. It appears that the URL-encoded parser already works as expected with ProcessPartial.

By the way, I realized that it would be better to add more tests for multipart requests. I think it will take me a day or two to do so.

hnakamur added a commit to hnakamur/ModSecurity that referenced this pull request Jan 1, 2026
This check is added to satisfy SonarQube Cloud Quality Gate on CI.
owasp-modsecurity#3483 (comment)
hnakamur added a commit to hnakamur/ModSecurity that referenced this pull request Jan 1, 2026
hnakamur added a commit to hnakamur/ModSecurity that referenced this pull request Jan 1, 2026
@hnakamur hnakamur force-pushed the v2/stop_processing_after_reqbody_limit_for_process_partial branch 2 times, most recently from 5b38984 to 0bfb828 Compare January 1, 2026 14:39
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 3, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@hnakamur
Copy link
Contributor Author

hnakamur commented Jan 6, 2026

It appears that the URL-encoded parser already works as expected with ProcessPartial.

I was mistaken about that. I modified the URL-encoded parser to include names and values up to the delimiter before the limit when SecRequestBodyLimitAction is set to ProcessPartial and the request body exceeds SecRequestBodyLimit.

I also modified the multipart parser to include parts whose boundary appears before the limit under the same conditions.

In addition, I added tests to cover these changes.

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.

2 participants