blaisewang commented on code in PR #13763:
URL: https://github.com/apache/apisix/pull/13763#discussion_r3702460066
##########
apisix/plugins/chaitin-waf.lua:
##########
@@ -152,6 +162,23 @@ local metadata_schema = {
real_client_ip = {
type = "boolean",
default = true
+ },
+ -- report the response to the WAF detection service
+ log_resp = {
+ type = "boolean",
+ default = false
+ },
+ -- amount of the response body to report, in KB,
+ -- 0 disables body buffering
+ resp_body_size = {
+ type = "integer",
+ minimum = 0,
+ default = 4
+ },
Review Comment:
Thanks for the review — happy to change it.
One thing I want to check with you before I do, though: config.req_body_size
in this plugin is already documented and shipped as "The maximum allowed
request body size, in KB". It has been KB since the plugin was introduced in
#9838, so it is part of the public API today.
That leaves two ways to go, and I'd rather you pick than guess:
1. Only change resp_body_size to bytes. Consistent with the other plugins,
but then the two body-size options inside the same config block use different
units — req_body_size: 1024 means 1 MB while resp_body_size: 1024 means 1 KB.
That seems more confusing for users of this plugin than the current
inconsistency across plugins.
2. Change both to bytes. Consistent everywhere, but req_body_size is an
existing option, so this is a breaking change for anyone who has it configured:
a route with req_body_size: 1024 would silently go from a 1 MB limit to a 1 KB
limit. That would need some kind of migration/compat handling — e.g. renaming
to max_req_body_bytes / max_resp_body_bytes (matching the *-logger plugins) and
keeping req_body_size accepted as a deprecated alias.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
