Goend commented on code in PR #13243:
URL: https://github.com/apache/apisix/pull/13243#discussion_r3135767208
##########
apisix/core/request.lua:
##########
@@ -290,15 +290,6 @@ function _M.get_body(max_size, ctx)
end
end
- -- check content-length header for http2/http3
Review Comment:
@Baoyuantop Thanks for the review. I initially took the same approach of
only bypassing the check in ext-plugin-post-resp, but after digging into the
upstream lua-nginx-module, I believe removing the check from get_body() is
actually the correct fix.
This Content-Length validation was originally added to align with
lua-nginx-module's documented limitation (see
https://github.com/openresty/lua-nginx-module/pull/2237). However, upstream has
since relaxed this restriction: -
https://github.com/openresty/lua-nginx-module/commit/e0d19f787e74ce3ffbb9b9c5abad02fd6ea81f41
partially lifted it -
https://github.com/openresty/lua-nginx-module/commit/6e29c1a96e641ea3e3498b1524baeaa28d3ab58c
further relaxed it, and the docs no longer state that ngx.req.read_body()
requires Content-Length for H2/H3
Since the
underlying layer no longer enforces this restriction, keeping the check in
get_body() makes APISIX stricter than the runtime — it blocks valid H2/H3
requests across all 20+ plugins that call get_body(), not just
ext-plugin-post-resp. Removing it here is not "too extensive" — it's removing a
guard that no longer serves its purpose.
I'll add targeted H2 tests for ext-plugin-post-resp as suggested. Let me
know your thoughts.
--
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]
