fix(ext-plugin-post-resp): avoid H2/H3 Content-Length check when read…#13243
fix(ext-plugin-post-resp): avoid H2/H3 Content-Length check when read…#13243Goend wants to merge 1 commit intoapache:masterfrom
Conversation
| end | ||
| end | ||
|
|
||
| -- check content-length header for http2/http3 |
There was a problem hiding this comment.
This modification is too extensive, we recommend:
- Narrow the fix to
ext-plugin-post-resp.lua: bypasscore.request.get_body()and usengx.req.read_body()+ngx.req.get_body_data()directly for the upstream replay request - Or refine the check in
get_body(): skip the Content-Length check for request methods that typically have no body (GET, HEAD, DELETE, OPTIONS), while keeping it for POST/PUT/PATCH - Add targeted tests: add H2 test cases specifically for
ext-plugin-post-respto cover the reported scenario, and verify no regression in other H2-dependent plugins (e.g.,grpc-web,batch-requests)
There was a problem hiding this 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 openresty/lua-nginx-module#2237). However, upstream has since relaxed this restriction: - openresty/lua-nginx-module@e0d19f7 partially lifted it - openresty/lua-nginx-module@6e29c1a 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.
Description
This PR fixes an issue in
ext-plugin-post-respwhen handling HTTP/2 and HTTP/3 requests without aContent-Lengthheader.Currently,
ext-plugin-post-respreplays the original client request to upstream and reads the request body viacore.request.get_body(). After the behavior introduced in #10887,core.request.get_body()rejectsHTTP/2 and HTTP/3 requests that do not contain a
Content-Lengthheader.This causes valid body-less requests such as normal
GETrequests over HTTP/2 to fail unexpectedly with502whenext-plugin-post-respis enabled.This PR avoids using
core.request.get_body()inext-plugin-post-respand reads the request body directly from the Nginx request APIs instead. As a result, body-less HTTP/2 and HTTP/3 requests are handledcorrectly, while requests that actually contain a body can still be forwarded as expected.
Which issue(s) this PR fixes:
Fixes #13237
Checklist