This is an automated email from the ASF dual-hosted git repository.
juzhiyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push:
new 2a5425f6f fix(grpc-web): response contains two trailer chunks (#11988)
2a5425f6f is described below
commit 2a5425f6f09f418ce10bfb3698518eb386ea0332
Author: Zeping Bai <[email protected]>
AuthorDate: Tue Feb 25 11:21:41 2025 +0800
fix(grpc-web): response contains two trailer chunks (#11988)
---
apisix/plugins/grpc-web.lua | 116 ++++++++++++++++++++++++++------------------
t/plugin/grpc-web.t | 51 ++++++++++---------
2 files changed, 94 insertions(+), 73 deletions(-)
diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua
index 260e84c4e..d69d3e665 100644
--- a/apisix/plugins/grpc-web.lua
+++ b/apisix/plugins/grpc-web.lua
@@ -68,6 +68,46 @@ function _M.check_schema(conf)
return core.schema.check(schema, conf)
end
+local function exit(ctx, status)
+ ctx.grpc_web_skip_body_filter = true
+ return status
+end
+
+--- Build gRPC-Web trailer chunk
+-- grpc-web trailer format reference:
+--
envoyproxy/envoy/source/extensions/filters/http/grpc_web/grpc_web_filter.cc
+--
+-- Format for grpc-web trailer
+-- 1 byte: 0x80
+-- 4 bytes: length of the trailer
+-- n bytes: trailer
+-- It using upstream_trailer_* variables from nginx, it is available since
NGINX version 1.13.10
+--
https://nginx.org/en/docs/http/ngx_http_upstream_module.html#var_upstream_trailer_
+--
+-- @param grpc_status number grpc status code
+-- @param grpc_message string grpc message
+-- @return string grpc-web trailer chunk in raw string
+local build_trailer = function (grpc_status, grpc_message)
+ local status_str = "grpc-status:" .. grpc_status
+ local status_msg = "grpc-message:" .. ( grpc_message or "")
+ local grpc_web_trailer = status_str .. "\r\n" .. status_msg .. "\r\n"
+ local len = #grpc_web_trailer
+
+ -- 1 byte: 0x80
+ local trailer_buf = string.char(0x80)
+ -- 4 bytes: length of the trailer
+ trailer_buf = trailer_buf .. string.char(
+ bit.band(bit.rshift(len, 24), 0xff),
+ bit.band(bit.rshift(len, 16), 0xff),
+ bit.band(bit.rshift(len, 8), 0xff),
+ bit.band(len, 0xff)
+ )
+ -- n bytes: trailer
+ trailer_buf = trailer_buf .. grpc_web_trailer
+
+ return trailer_buf
+end
+
function _M.access(conf, ctx)
-- set context variable mime
-- When processing non gRPC Web requests, `mime` can be obtained in the
context
@@ -76,19 +116,19 @@ function _M.access(conf, ctx)
local method = core.request.get_method()
if method == ALLOW_METHOD_OPTIONS then
- return 204
+ return exit(ctx, 204)
end
if method ~= ALLOW_METHOD_POST then
--
https://github.com/grpc/grpc-web/blob/master/doc/browser-features.md#cors-support
core.log.error("request method: `", method, "` invalid")
- return 400
+ return exit(ctx, 405)
end
local encoding = grpc_web_content_encoding[ctx.grpc_web_mime]
if not encoding then
core.log.error("request Content-Type: `", ctx.grpc_web_mime, "`
invalid")
- return 400
+ return exit(ctx, 400)
end
-- set context variable encoding method
@@ -98,7 +138,7 @@ function _M.access(conf, ctx)
if not (ctx.curr_req_matched and ctx.curr_req_matched[":ext"]) then
core.log.error("routing configuration error, grpc-web plugin only
supports ",
"`prefix matching` pattern routing")
- return 400
+ return exit(ctx, 400)
end
local path = ctx.curr_req_matched[":ext"]
@@ -110,16 +150,17 @@ function _M.access(conf, ctx)
-- set grpc body
local body, err = core.request.get_body()
- if err then
+ if err or not body then
core.log.error("failed to read request body, err: ", err)
- return 400
+ return exit(ctx, 400)
end
if encoding == CONTENT_ENCODING_BASE64 then
body = decode_base64(body)
+ ngx.log(ngx.WARN, "DECODE BODY: ", body)
if not body then
core.log.error("failed to decode request body")
- return 400
+ return exit(ctx, 400)
end
end
@@ -139,11 +180,19 @@ function _M.header_filter(conf, ctx)
if not ctx.cors_allow_origins then
core.response.set_header("Access-Control-Allow-Origin",
DEFAULT_CORS_ALLOW_ORIGIN)
end
- core.response.set_header("Content-Type", ctx.grpc_web_mime)
core.response.set_header("Access-Control-Expose-Headers",
DEFAULT_CORS_EXPOSE_HEADERS)
+
+ if not ctx.grpc_web_skip_body_filter then
+ core.response.set_header("Content-Type", ctx.grpc_web_mime)
+ core.response.set_header("Content-Length", nil)
+ end
end
function _M.body_filter(conf, ctx)
+ if ctx.grpc_web_skip_body_filter then
+ return
+ end
+
-- If the MIME extension type description of the gRPC-Web standard is not
obtained,
-- indicating that the request is not based on the gRPC Web specification,
-- the processing of the request body will be ignored
@@ -159,48 +208,21 @@ function _M.body_filter(conf, ctx)
ngx_arg[1] = chunk
end
- --[[
- upstream_trailer_* available since NGINX version 1.13.10 :
-
https://nginx.org/en/docs/http/ngx_http_upstream_module.html#var_upstream_trailer_
-
- grpc-web trailer format reference:
- envoyproxy/envoy/source/extensions/filters/http/grpc_web/grpc_web_filter.cc
-
- Format for grpc-web trailer
- 1 byte: 0x80
- 4 bytes: length of the trailer
- n bytes: trailer
-
- --]]
- local status = ctx.var.upstream_trailer_grpc_status
- local message = ctx.var.upstream_trailer_grpc_message
- if status ~= "" and status ~= nil then
- local status_str = "grpc-status:" .. status
- local status_msg = "grpc-message:" .. ( message or "")
- local grpc_web_trailer = status_str .. "\r\n" .. status_msg .. "\r\n"
- local len = #grpc_web_trailer
-
- -- 1 byte: 0x80
- local trailer_buf = string.char(0x80)
- -- 4 bytes: length of the trailer
- trailer_buf = trailer_buf .. string.char(
- bit.band(bit.rshift(len, 24), 0xff),
- bit.band(bit.rshift(len, 16), 0xff),
- bit.band(bit.rshift(len, 8), 0xff),
- bit.band(len, 0xff)
- )
- -- n bytes: trailer
- trailer_buf = trailer_buf .. grpc_web_trailer
+ if ngx_arg[2] then -- if eof
+ local status = ctx.var.upstream_trailer_grpc_status
+ local message = ctx.var.upstream_trailer_grpc_message
- if ctx.grpc_web_encoding == CONTENT_ENCODING_BINARY then
- ngx_arg[1] = ngx_arg[1] .. trailer_buf
- else
- ngx_arg[1] = ngx_arg[1] .. encode_base64(trailer_buf)
+ -- When the response body completes and still does not receive the
grpc status
+ local resp_ok = status ~= nil and status ~= ""
+ local trailer_buf = build_trailer(
+ resp_ok and status or 2,
+ resp_ok and message or "upstream grpc status not received"
+ )
+ if ctx.grpc_web_encoding == CONTENT_ENCODING_BASE64 then
+ trailer_buf = encode_base64(trailer_buf)
end
- -- clear trailer
- ctx.var.upstream_trailer_grpc_status = nil
- ctx.var.upstream_trailer_grpc_message = nil
+ ngx_arg[1] = ngx_arg[1] .. trailer_buf
end
end
diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t
index 1d3a3647e..ac8f689ee 100644
--- a/t/plugin/grpc-web.t
+++ b/t/plugin/grpc-web.t
@@ -69,15 +69,14 @@ passed
=== TEST 2: Proxy unary request using APISIX with trailers gRPC-Web plugin
+Status should be printed at most once per request, otherwise this would be out
of specification.
--- exec
node ./t/plugin/grpc-web/client.js BIN UNARY
node ./t/plugin/grpc-web/client.js TEXT UNARY
--- response_body
Status: { code: 0, details: '', metadata: {} }
-Status: { code: 0, details: '', metadata: {} }
{"name":"hello","path":"/hello"}
Status: { code: 0, details: '', metadata: {} }
-Status: { code: 0, details: '', metadata: {} }
{"name":"hello","path":"/hello"}
@@ -90,11 +89,9 @@ node ./t/plugin/grpc-web/client.js TEXT STREAM
{"name":"hello","path":"/hello"}
{"name":"world","path":"/world"}
Status: { code: 0, details: '', metadata: {} }
-Status: { code: 0, details: '', metadata: {} }
{"name":"hello","path":"/hello"}
{"name":"world","path":"/world"}
Status: { code: 0, details: '', metadata: {} }
-Status: { code: 0, details: '', metadata: {} }
@@ -112,7 +109,7 @@ Access-Control-Allow-Origin: *
=== TEST 5: test non-options request
--- request
GET /grpc/web/a6.RouteService/GetRoute
---- error_code: 400
+--- error_code: 405
--- response_headers
Access-Control-Allow-Origin: *
--- error_log
@@ -128,7 +125,7 @@ Content-Type: application/json
--- error_code: 400
--- response_headers
Access-Control-Allow-Origin: *
-Content-Type: application/json
+Content-Type: text/html
--- error_log
request Content-Type: `application/json` invalid
@@ -177,7 +174,7 @@ Content-Type: application/grpc-web
--- error_code: 400
--- response_headers
Access-Control-Allow-Origin: *
-Content-Type: application/grpc-web
+Content-Type: text/html
--- error_log
routing configuration error, grpc-web plugin only supports `prefix matching`
pattern routing
@@ -226,33 +223,35 @@ passed
=== TEST 10: don't override Access-Control-Allow-Origin header in response
---- request
-POST /grpc/web/a6.RouteService/GetRoute
-{}
---- more_headers
-Origin: http://test.com
-Content-Type: application/grpc-web
---- response_headers
-Access-Control-Allow-Origin: http://test.com
-Content-Type: application/grpc-web
+--- exec
+curl -iv --location 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \
+--header 'Origin: http://test.com' \
+--header 'Content-Type: application/grpc-web-text' \
+--data-raw 'AAAAAAcKBXdvcmxkCgo='
+--- response_body eval
+qr/HTTP\/1.1 200 OK/ and qr/Access-Control-Allow-Origin: http:\/\/test.com/
=== TEST 11: check for Access-Control-Expose-Headers header in response
---- request
-POST /grpc/web/a6.RouteService/GetRoute
-{}
---- more_headers
-Origin: http://test.com
-Content-Type: application/grpc-web
---- response_headers
-Access-Control-Allow-Origin: http://test.com
-Access-Control-Expose-Headers: grpc-message,grpc-status
-Content-Type: application/grpc-web
+--- exec
+curl -iv --location 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \
+--header 'Origin: http://test.com' \
+--header 'Content-Type: application/grpc-web-text' \
+--data-raw 'AAAAAAcKBXdvcmxkCgo='
+--- response_body eval
+qr/Access-Control-Expose-Headers: grpc-message,grpc-status/ and
qr/Access-Control-Allow-Origin: http:\/\/test.com/
=== TEST 12: verify trailers in response
+According to the gRPC documentation, the grpc-web proxy should not retain
trailers received from upstream when
+forwarding them, as the reference implementation envoy does, so the current
test case is status quo rather
+than "correct", which is not expected to have an impact since browsers ignore
trailers.
+Currently there is no API or hook point available in nginx/lua-nginx-module to
remove specified trailers
+on demand (grpc_hide_header can do it but it affects the grpc proxy), and some
nginx patches may be needed
+to allow for code-controlled removal of the trailer at runtime.
+When we implement that, this use case will be removed.
--- exec
curl -iv --location 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \
--header 'Content-Type: application/grpc-web+proto' \
