From ff775cb463f2b007d993e0e37332ad9a0a0b5eba Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Fri, 19 Jan 2024 00:02:37 +0530 Subject: [PATCH 1/8] fix grpc-web trailers Signed-off-by: Mohammad Shehar Yaar Tausif --- apisix/plugins/grpc-web.lua | 42 ++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 18465063b343..5b4afff83c6b 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -87,7 +87,7 @@ function _M.access(conf, ctx) -- set grpc path 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") + "`prefix matching` pattern routing") return 400 end @@ -130,6 +130,7 @@ function _M.header_filter(conf, ctx) 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", "grpc-message,grpc-status") end function _M.body_filter(conf, ctx) @@ -147,6 +148,45 @@ function _M.body_filter(conf, ctx) chunk = encode_base64(chunk) 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 + --]] + + local status = ctx.var.upstream_trailer_grpc_status + if status ~= "" and status ~= nil then + -- format for grpc-web trailer + -- 1 byte: 0x80 + -- 4 bytes: length of the trailer + -- n bytes: trailer + + -- 1 byte: 0x80 + ngx_arg[1] = ngx_arg[1] .. string.char(0x80) + + local status_str = "grpc-status:" .. tonumber(status) + local status_msg = "grpc-message:" .. ctx.var.upstream_trailer_grpc_message + local grpc_web_trailer = status_str .. "\r\n" .. status_msg .. "\r\n" + + -- 4 bytes: length of the trailer + local len = #grpc_web_trailer + ngx_arg[1] = ngx_arg[1] .. 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 + ngx_arg[1] = ngx_arg[1] .. grpc_web_trailer + + -- clear trailer + ctx.var.upstream_trailer_grpc_status = nil + ctx.var.upstream_trailer_grpc_message = nil + end end return _M From 080839d881399dd3c710a6e986948fd05700e52e Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Fri, 19 Jan 2024 18:11:09 +0530 Subject: [PATCH 2/8] fix: grpc-web trailer logic and add test case Signed-off-by: Mohammad Shehar Yaar Tausif --- apisix/plugins/grpc-web.lua | 38 ++++++++++++++++++++----------------- t/plugin/grpc-web.t | 26 +++++++++++++++++++++++-- t/plugin/grpc-web/client.js | 6 ++++++ 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 5b4afff83c6b..1e759418d6aa 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -150,38 +150,42 @@ function _M.body_filter(conf, ctx) end --[[ - upstream_trailer_* available since NGINX version 1.13.10 : - https://nginx.org/en/docs/http/ngx_http_upstream_module.html#var_upstream_trailer_ + 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 - --]] + 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 if status ~= "" and status ~= nil then - -- format for grpc-web trailer - -- 1 byte: 0x80 - -- 4 bytes: length of the trailer - -- n bytes: trailer - - -- 1 byte: 0x80 - ngx_arg[1] = ngx_arg[1] .. string.char(0x80) - local status_str = "grpc-status:" .. tonumber(status) local status_msg = "grpc-message:" .. ctx.var.upstream_trailer_grpc_message 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 - local len = #grpc_web_trailer - ngx_arg[1] = ngx_arg[1] .. string.char( + 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 - ngx_arg[1] = ngx_arg[1] .. grpc_web_trailer + trailer_buf = trailer_buf .. grpc_web_trailer + + 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) + end -- clear trailer ctx.var.upstream_trailer_grpc_status = nil diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index 7340add60fce..a5d67ac18531 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -68,25 +68,33 @@ passed -=== TEST 2: Proxy unary request using APISIX gRPC-Web plugin +=== TEST 2: Proxy unary request using APISIX with trailers gRPC-Web plugin --- 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"} -=== TEST 3: Proxy server-side streaming request using APISIX gRPC-Web plugin +=== TEST 3: Proxy server-side streaming request using APISIX with trailers gRPC-Web plugin --- exec node ./t/plugin/grpc-web/client.js BIN STREAM node ./t/plugin/grpc-web/client.js TEXT STREAM --- response_body {"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: {} } @@ -227,3 +235,17 @@ Content-Type: application/grpc-web --- response_headers Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web + + + +=== 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 diff --git a/t/plugin/grpc-web/client.js b/t/plugin/grpc-web/client.js index 3f10a80bc100..9ec044124c95 100644 --- a/t/plugin/grpc-web/client.js +++ b/t/plugin/grpc-web/client.js @@ -49,6 +49,8 @@ class gRPCWebClient { return } console.log(JSON.stringify(response.toObject())); + }).on("status", function (status) { + console.log("Status:", status); }); } @@ -62,6 +64,10 @@ class gRPCWebClient { stream.on('end', function(end) { stream.cancel(); }); + + stream.on("status", function (status) { + console.log("Status:", status); + }); } } From 66977ab793552ecdd3de4fc20bdd2affe8e3a916 Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Sat, 20 Jan 2024 00:08:20 +0530 Subject: [PATCH 3/8] fix: workflow Signed-off-by: Mohammad Shehar Yaar Tausif --- apisix/plugins/grpc-web.lua | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 1e759418d6aa..b0038bd71da7 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -21,6 +21,9 @@ local req_set_uri = ngx.req.set_uri local req_set_body_data = ngx.req.set_body_data local decode_base64 = ngx.decode_base64 local encode_base64 = ngx.encode_base64 +local tonumber = tonumber +local bit = require("bit") +local string = string local ALLOW_METHOD_OPTIONS = "OPTIONS" From 7a4ebede5c20d7d64fc45b39104a9a1419c053e1 Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Mon, 22 Jan 2024 14:24:37 +0530 Subject: [PATCH 4/8] fix: remove tonumber Signed-off-by: Mohammad Shehar Yaar Tausif --- apisix/plugins/grpc-web.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index b0038bd71da7..325cbe43c476 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -167,7 +167,7 @@ function _M.body_filter(conf, ctx) --]] local status = ctx.var.upstream_trailer_grpc_status if status ~= "" and status ~= nil then - local status_str = "grpc-status:" .. tonumber(status) + local status_str = "grpc-status:" .. status local status_msg = "grpc-message:" .. ctx.var.upstream_trailer_grpc_message local grpc_web_trailer = status_str .. "\r\n" .. status_msg .. "\r\n" local len = #grpc_web_trailer From 07812e48ba71dcec6c230e5bb21878b24ad4cc80 Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Tue, 23 Jan 2024 17:08:19 +0530 Subject: [PATCH 5/8] fix: lint Signed-off-by: Mohammad Shehar Yaar Tausif --- apisix/plugins/grpc-web.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 325cbe43c476..e262a3694758 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -21,7 +21,6 @@ local req_set_uri = ngx.req.set_uri local req_set_body_data = ngx.req.set_body_data local decode_base64 = ngx.decode_base64 local encode_base64 = ngx.encode_base64 -local tonumber = tonumber local bit = require("bit") local string = string From 38a722c04fe9876ec8c80a186d868a55850c4e9d Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Wed, 24 Jan 2024 19:16:33 +0530 Subject: [PATCH 6/8] fix: add test case Signed-off-by: Mohammad Shehar Yaar Tausif --- apisix/plugins/grpc-web.lua | 3 ++- t/plugin/grpc-web.t | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index e262a3694758..5771604e74f0 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -165,9 +165,10 @@ function _M.body_filter(conf, ctx) --]] 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:" .. ctx.var.upstream_trailer_grpc_message + local status_msg = "grpc-message:" .. ( message or "") local grpc_web_trailer = status_str .. "\r\n" .. status_msg .. "\r\n" local len = #grpc_web_trailer diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index a5d67ac18531..accbdfd1cdf7 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -249,3 +249,17 @@ Content-Type: application/grpc-web Access-Control-Allow-Origin: http://test.com Access-Control-Expose-Headers: grpc-message,grpc-status Content-Type: application/grpc-web + + + +=== TEST 12: verify trailers in response +--- exec +printf '\u0000\u0000\u0000\u0000\u0007\n\u0005world' | curl 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \ + -H 'Accept: */*' \ + -H 'Content-Type: application/grpc-web+proto' \ + -H 'X-Grpc-Web: 1' \ + -H 'X-User-Agent: grpc-web-javascript/0.1' \ + --data-binary '@-' \ + --compressed -o - +--- response_body eval +qr/grpc-status:0\x0d\x0agrpc-message:/ \ No newline at end of file From 77cc6a8d09d20a20ea1a9eccfcc45cc67e0a74d8 Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Thu, 25 Jan 2024 22:38:42 +0530 Subject: [PATCH 7/8] fix: ci and lint Signed-off-by: Mohammad Shehar Yaar Tausif --- t/plugin/grpc-web.t | 13 +++++-------- t/plugin/grpc-web/req.bin | Bin 0 -> 14 bytes 2 files changed, 5 insertions(+), 8 deletions(-) create mode 100644 t/plugin/grpc-web/req.bin diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index accbdfd1cdf7..a33d3c85d7ca 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -254,12 +254,9 @@ Content-Type: application/grpc-web === TEST 12: verify trailers in response --- exec -printf '\u0000\u0000\u0000\u0000\u0007\n\u0005world' | curl 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \ - -H 'Accept: */*' \ - -H 'Content-Type: application/grpc-web+proto' \ - -H 'X-Grpc-Web: 1' \ - -H 'X-User-Agent: grpc-web-javascript/0.1' \ - --data-binary '@-' \ - --compressed -o - +curl -iv --location 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \ +--header 'Content-Type: application/grpc-web+proto' \ +--header ''\''X-Grpc-Web: 1' \ +--data-binary '@./t/plugin/grpc-web/req.bin' --- response_body eval -qr/grpc-status:0\x0d\x0agrpc-message:/ \ No newline at end of file +qr/grpc-status:0\x0d\x0agrpc-message:/ diff --git a/t/plugin/grpc-web/req.bin b/t/plugin/grpc-web/req.bin new file mode 100644 index 0000000000000000000000000000000000000000..908c829c608e4907d37a89eb2185f7b5da5be1a2 GIT binary patch literal 14 VcmZQzU|?Y9VlB@v%1Pnk0ssr;0#g70 literal 0 HcmV?d00001 From 1e17947107d0db65d59494c69f9959d8cc7af889 Mon Sep 17 00:00:00 2001 From: Mohammad Shehar Yaar Tausif Date: Thu, 25 Jan 2024 22:41:51 +0530 Subject: [PATCH 8/8] fix: minor change Signed-off-by: Mohammad Shehar Yaar Tausif --- t/plugin/grpc-web.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index a33d3c85d7ca..7069a8c2ccb8 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -256,7 +256,7 @@ Content-Type: application/grpc-web --- exec curl -iv --location 'http://127.0.0.1:1984/grpc/web/a6.RouteService/GetRoute' \ --header 'Content-Type: application/grpc-web+proto' \ ---header ''\''X-Grpc-Web: 1' \ +--header 'X-Grpc-Web: 1' \ --data-binary '@./t/plugin/grpc-web/req.bin' --- response_body eval qr/grpc-status:0\x0d\x0agrpc-message:/