Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions apisix/core/request.lua
Original file line number Diff line number Diff line change
Expand Up @@ -290,15 +290,6 @@ function _M.get_body(max_size, ctx)
end
end

-- check content-length header for http2/http3
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modification is too extensive, we recommend:

  1. Narrow the fix to ext-plugin-post-resp.lua: bypass core.request.get_body() and use ngx.req.read_body() + ngx.req.get_body_data() directly for the upstream replay request
  2. 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
  3. Add targeted tests: add H2 test cases specifically for ext-plugin-post-resp to cover the reported scenario, and verify no regression in other H2-dependent plugins (e.g., grpc-web, batch-requests)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

do
local var = ctx and ctx.var or ngx.var
local content_length = tonumber(var.http_content_length)
if (var.server_protocol == "HTTP/2.0" or var.server_protocol == "HTTP/3.0")
and not content_length then
return nil, "HTTP2/HTTP3 request without a Content-Length header"
end
end
req_read_body()

local req_body = req_get_body_data()
Expand Down
7 changes: 3 additions & 4 deletions t/plugin/azure-functions.t
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,9 @@ passed



=== TEST 15: http2 failed to check response body and headers
=== TEST 15: http2 success to without Content-Length
--- http2
--- request
GET /azure
--- error_code: 400
--- error_log
HTTP2/HTTP3 request without a Content-Length header,
--- response_body
faas invoked
Loading