From edf68c2ffa2ee04ee13f19ac7c8772292c62d273 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Thu, 7 Dec 2023 19:45:47 +0500 Subject: [PATCH 01/10] changes --- apisix/plugins/grpc-web.lua | 38 ++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 18465063b343..e026d0ad7d66 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -28,10 +28,18 @@ local ALLOW_METHOD_POST = "POST" local CONTENT_ENCODING_BASE64 = "base64" local CONTENT_ENCODING_BINARY = "binary" local DEFAULT_CORS_ALLOW_ORIGIN = "*" -local DEFAULT_CORS_ALLOW_METHODS = ALLOW_METHOD_POST -local DEFAULT_CORS_ALLOW_HEADERS = "content-type,x-grpc-web,x-user-agent" +local DEFAULT_CORS_ALLOW_METHODS = "POST, OPTIONS" +local DEFAULT_CORS_ALLOW_HEADERS = "content-type,x-grpc-web,x-user-agent,grpc-accept-encoding" local DEFAULT_PROXY_CONTENT_TYPE = "application/grpc" +local DEFAULT_CORS_ALLOW_EXPOSE_HEADERS = "grpc-status,grpc-message" +local GRPC_WEB_TRAILER_FRAME_HEADER = string.char(128, 0, 0, 0) +local GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES = { + ["grpc-status"] = "0", + ["grpc-message"] = "OK" +} + +local CRLF = "\r\n" local plugin_name = "grpc-web" @@ -129,19 +137,43 @@ 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("Access-Control-Expose-Headers", DEFAULT_CORS_ALLOW_EXPOSE_HEADERS) core.response.set_header("Content-Type", ctx.grpc_web_mime) + + core.response.clear_header_as_body_modified() end function _M.body_filter(conf, ctx) -- 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 + -- If response body is not empty, in-body trailers required by gRPC-Web are added to the end of response body -- https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md -- https://github.com/grpc/grpc-web/blob/master/doc/browser-features.md#cors-support if not ctx.grpc_web_mime then return end + local response = core.response.hold_body_chunk(ctx) + if response and string.len(response) ~= 0 then + local headers = ngx.resp.get_headers() + local trailers = " " + + for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do + local trailer_value = headers[trailer_key] + + if trailer_value == nil then + trailer_value = trailer_default_value + end + + trailers = trailers .. trailer_key .. ":" .. trailer_value .. CRLF + end + + response = response .. GRPC_WEB_TRAILER_FRAME_HEADER .. trailers + ngx_arg[1] = response + end + if ctx.grpc_web_encoding == CONTENT_ENCODING_BASE64 then local chunk = ngx_arg[1] chunk = encode_base64(chunk) @@ -149,4 +181,4 @@ function _M.body_filter(conf, ctx) end end -return _M +return _M \ No newline at end of file From f6cb8986a1c6ef48557dda7ab80ec7e050a38754 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Sat, 9 Dec 2023 01:01:21 +0500 Subject: [PATCH 02/10] updated test cases --- 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 7340add60fce..7acf94be01a1 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -95,7 +95,7 @@ node ./t/plugin/grpc-web/client.js TEXT STREAM OPTIONS /grpc/web/a6.RouteService/GetRoute --- error_code: 204 --- response_headers -Access-Control-Allow-Methods: POST +Access-Control-Allow-Methods: POST, OPTIONS Access-Control-Allow-Headers: content-type,x-grpc-web,x-user-agent Access-Control-Allow-Origin: * From 5d0ed35f9ea78e7fe074ba8684bd95bedc0f1b67 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Sat, 9 Dec 2023 01:23:18 +0500 Subject: [PATCH 03/10] added test cases --- t/plugin/grpc-web.t | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index 7acf94be01a1..2a96ce09075a 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -227,3 +227,17 @@ Content-Type: application/grpc-web --- response_headers Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web + +=== TEST 11: grpc-web access control expose headers for non grpc servers that don't implment grpc-web +--- 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 +Access-Control-Expose-Headers: grpc-status,grpc-message +Access-Control-Allow-Methods: + From 784fb56793d0c5844eddc15b0899f44f25e23d34 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Sat, 9 Dec 2023 01:23:39 +0500 Subject: [PATCH 04/10] added test cases --- t/plugin/grpc-web.t | 1 - 1 file changed, 1 deletion(-) diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index 2a96ce09075a..f51ac406b879 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -239,5 +239,4 @@ Content-Type: application/grpc-web Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web Access-Control-Expose-Headers: grpc-status,grpc-message -Access-Control-Allow-Methods: From 34bf5890387498dad31dd1646bbac7d35a81a78a Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Mon, 1 Jan 2024 15:39:43 +0500 Subject: [PATCH 05/10] path based changes --- apisix/plugins/grpc-web.lua | 49 +++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index e026d0ad7d66..1dfa429205ef 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -45,7 +45,19 @@ local plugin_name = "grpc-web" local schema = { type = "object", - properties = {}, + properties = { + strip_path = { + description = "include prefix matched by path pattern in the path used for upstream call," + .. "appropriate for prefix matching path patterns with the format ./*", + type = "boolean", + default = false + }, + enable_in_body_trailers_on_success = { + description = "append standard grpc-web in-body trailers frame in response body", + type = "boolean", + default = false + }, + }, } local grpc_web_content_encoding = { @@ -99,7 +111,13 @@ function _M.access(conf, ctx) return 400 end - local path = ctx.curr_req_matched[":ext"] + local path + if conf.strip_path and ctx.curr_req_matched._path:byte(-1) == core.string.byte("*") and ctx.curr_req_matched[":ext"]:byte(1) ~= core.string.byte("/") then + path = string.sub(ctx.curr_req_matched._path, 1, -2) .. ctx.curr_req_matched[":ext"] + else + path = ctx.curr_req_matched[":ext"] + end + if path:byte(1) ~= core.string.byte("/") then path = "/" .. path end @@ -137,10 +155,8 @@ 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("Access-Control-Expose-Headers", DEFAULT_CORS_ALLOW_EXPOSE_HEADERS) core.response.set_header("Content-Type", ctx.grpc_web_mime) - core.response.clear_header_as_body_modified() end @@ -155,23 +171,24 @@ function _M.body_filter(conf, ctx) return end - local response = core.response.hold_body_chunk(ctx) - if response and string.len(response) ~= 0 then - local headers = ngx.resp.get_headers() - local trailers = " " + if conf.enable_in_body_trailers_on_success then + local response = core.response.hold_body_chunk(ctx) + if response and string.len(response) ~= 0 then + local headers = ngx.resp.get_headers() + local trailers = " " + for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do + local trailer_value = headers[trailer_key] - for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do - local trailer_value = headers[trailer_key] + if trailer_value == nil then + trailer_value = trailer_default_value + end - if trailer_value == nil then - trailer_value = trailer_default_value + trailers = trailers .. trailer_key .. ":" .. trailer_value .. CRLF end - trailers = trailers .. trailer_key .. ":" .. trailer_value .. CRLF + response = response .. GRPC_WEB_TRAILER_FRAME_HEADER .. trailers + ngx_arg[1] = response end - - response = response .. GRPC_WEB_TRAILER_FRAME_HEADER .. trailers - ngx_arg[1] = response end if ctx.grpc_web_encoding == CONTENT_ENCODING_BASE64 then From 69f8c622d1cc5cefb2415f9663f87903d5d46f58 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Mon, 8 Jan 2024 16:42:26 +0500 Subject: [PATCH 06/10] ci changes --- apisix/plugins/grpc-web.lua | 3 +-- t/plugin/grpc-web.t | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 1dfa429205ef..3cb9e2e78248 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -197,5 +197,4 @@ function _M.body_filter(conf, ctx) ngx_arg[1] = chunk end end - -return _M \ No newline at end of file +return _M diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index f51ac406b879..d4b34020776c 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -96,7 +96,7 @@ OPTIONS /grpc/web/a6.RouteService/GetRoute --- error_code: 204 --- response_headers Access-Control-Allow-Methods: POST, OPTIONS -Access-Control-Allow-Headers: content-type,x-grpc-web,x-user-agent +Access-Control-Allow-Headers: content-type,x-grpc-web,x-user-agent,grpc-accept-encoding Access-Control-Allow-Origin: * @@ -228,7 +228,7 @@ Content-Type: application/grpc-web Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web -=== TEST 11: grpc-web access control expose headers for non grpc servers that don't implment grpc-web +=== TEST 11: grpc-web access control expose headers for non grpc servers that don't implement grpc-web --- request POST /grpc/web/a6.RouteService/GetRoute {} @@ -238,5 +238,5 @@ Content-Type: application/grpc-web --- response_headers Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web -Access-Control-Expose-Headers: grpc-status,grpc-message +Access-Control-Expose-Headers: grpc-status,grpc-message, From 310460b31752f0c9f6775ba0169035818576169a Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Tue, 9 Jan 2024 10:27:43 +0500 Subject: [PATCH 07/10] ci changes --- 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 d4b34020776c..75b9a2efdd81 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -238,5 +238,5 @@ Content-Type: application/grpc-web --- response_headers Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web -Access-Control-Expose-Headers: grpc-status,grpc-message, +Access-Control-Expose-Headers: grpc-status,grpc-message From d8dbd01ba00a516bd74cc8ea31acaa37fe21cda0 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Thu, 11 Jan 2024 19:37:22 +0500 Subject: [PATCH 08/10] changed linting --- apisix/plugins/grpc-web.lua | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 3cb9e2e78248..eaef4c6a61cb 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -47,8 +47,9 @@ local schema = { type = "object", properties = { strip_path = { - description = "include prefix matched by path pattern in the path used for upstream call," - .. "appropriate for prefix matching path patterns with the format ./*", + description = "include prefix matched by path pattern in the path used for " + .. "upstream call, appropriate for prefix matching path " + .. "patterns with the format ./*", type = "boolean", default = false }, @@ -112,7 +113,8 @@ function _M.access(conf, ctx) end local path - if conf.strip_path and ctx.curr_req_matched._path:byte(-1) == core.string.byte("*") and ctx.curr_req_matched[":ext"]:byte(1) ~= core.string.byte("/") then + if conf.strip_path and ctx.curr_req_matched._path:byte(-1) == core.string.byte("*") + and ctx.curr_req_matched[":ext"]:byte(1) ~= core.string.byte("/") then path = string.sub(ctx.curr_req_matched._path, 1, -2) .. ctx.curr_req_matched[":ext"] else path = ctx.curr_req_matched[":ext"] @@ -164,7 +166,8 @@ function _M.body_filter(conf, ctx) -- 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 - -- If response body is not empty, in-body trailers required by gRPC-Web are added to the end of response body + -- If response body is not empty, in-body trailers required by gRPC-Web + -- are added to the end of response body -- https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md -- https://github.com/grpc/grpc-web/blob/master/doc/browser-features.md#cors-support if not ctx.grpc_web_mime then @@ -176,7 +179,8 @@ function _M.body_filter(conf, ctx) if response and string.len(response) ~= 0 then local headers = ngx.resp.get_headers() local trailers = " " - for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do + for trailer_key, trailer_default_value in + pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do local trailer_value = headers[trailer_key] if trailer_value == nil then From 06756819f5a837b911b89b2d739185eb28d880e6 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Thu, 11 Jan 2024 19:39:43 +0500 Subject: [PATCH 09/10] changed linting --- apisix/plugins/grpc-web.lua | 45 +++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index eaef4c6a61cb..147004a966d5 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -13,15 +13,13 @@ -- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -- See the License for the specific language governing permissions and -- limitations under the License. - -local ngx = ngx -local ngx_arg = ngx.arg -local core = require("apisix.core") -local req_set_uri = ngx.req.set_uri +local ngx = ngx +local ngx_arg = ngx.arg +local core = require("apisix.core") +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 decode_base64 = ngx.decode_base64 +local encode_base64 = ngx.encode_base64 local ALLOW_METHOD_OPTIONS = "OPTIONS" local ALLOW_METHOD_POST = "POST" @@ -47,32 +45,32 @@ local schema = { type = "object", properties = { strip_path = { - description = "include prefix matched by path pattern in the path used for " - .. "upstream call, appropriate for prefix matching path " - .. "patterns with the format ./*", - type = "boolean", - default = false + description = "include prefix matched by path pattern in the path used for " .. + "upstream call, appropriate for prefix matching path " .. + "patterns with the format ./*", + type = "boolean", + default = false }, enable_in_body_trailers_on_success = { description = "append standard grpc-web in-body trailers frame in response body", - type = "boolean", - default = false - }, - }, + type = "boolean", + default = false + } + } } local grpc_web_content_encoding = { ["application/grpc-web"] = CONTENT_ENCODING_BINARY, ["application/grpc-web-text"] = CONTENT_ENCODING_BASE64, ["application/grpc-web+proto"] = CONTENT_ENCODING_BINARY, - ["application/grpc-web-text+proto"] = CONTENT_ENCODING_BASE64, + ["application/grpc-web-text+proto"] = CONTENT_ENCODING_BASE64 } local _M = { version = 0.1, priority = 505, name = plugin_name, - schema = schema, + schema = schema } function _M.check_schema(conf) @@ -108,13 +106,13 @@ 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 local path - if conf.strip_path and ctx.curr_req_matched._path:byte(-1) == core.string.byte("*") - and ctx.curr_req_matched[":ext"]:byte(1) ~= core.string.byte("/") then + if conf.strip_path and ctx.curr_req_matched._path:byte(-1) == core.string.byte("*") and + ctx.curr_req_matched[":ext"]:byte(1) ~= core.string.byte("/") then path = string.sub(ctx.curr_req_matched._path, 1, -2) .. ctx.curr_req_matched[":ext"] else path = ctx.curr_req_matched[":ext"] @@ -179,8 +177,7 @@ function _M.body_filter(conf, ctx) if response and string.len(response) ~= 0 then local headers = ngx.resp.get_headers() local trailers = " " - for trailer_key, trailer_default_value in - pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do + for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do local trailer_value = headers[trailer_key] if trailer_value == nil then From 31580c601feec9ebf7cd29dee12d2e14e8c2e8b4 Mon Sep 17 00:00:00 2001 From: Affan Amir Mir Date: Wed, 17 Jan 2024 15:26:58 +0500 Subject: [PATCH 10/10] added feedback --- apisix/plugins/grpc-web.lua | 4 ++-- t/plugin/grpc-web.t | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apisix/plugins/grpc-web.lua b/apisix/plugins/grpc-web.lua index 147004a966d5..a528985d9a8f 100644 --- a/apisix/plugins/grpc-web.lua +++ b/apisix/plugins/grpc-web.lua @@ -32,7 +32,7 @@ local DEFAULT_PROXY_CONTENT_TYPE = "application/grpc" local DEFAULT_CORS_ALLOW_EXPOSE_HEADERS = "grpc-status,grpc-message" local GRPC_WEB_TRAILER_FRAME_HEADER = string.char(128, 0, 0, 0) -local GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES = { +local GRPC_WEB_REQ_TRAILERS_DEFAULT = { ["grpc-status"] = "0", ["grpc-message"] = "OK" } @@ -177,7 +177,7 @@ function _M.body_filter(conf, ctx) if response and string.len(response) ~= 0 then local headers = ngx.resp.get_headers() local trailers = " " - for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQUIRED_TRAILERS_DEFAULT_VALUES) do + for trailer_key, trailer_default_value in pairs(GRPC_WEB_REQ_TRAILERS_DEFAULT) do local trailer_value = headers[trailer_key] if trailer_value == nil then diff --git a/t/plugin/grpc-web.t b/t/plugin/grpc-web.t index 75b9a2efdd81..b9c35c81182a 100644 --- a/t/plugin/grpc-web.t +++ b/t/plugin/grpc-web.t @@ -228,6 +228,8 @@ Content-Type: application/grpc-web Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web + + === TEST 11: grpc-web access control expose headers for non grpc servers that don't implement grpc-web --- request POST /grpc/web/a6.RouteService/GetRoute @@ -239,4 +241,3 @@ Content-Type: application/grpc-web Access-Control-Allow-Origin: http://test.com Content-Type: application/grpc-web Access-Control-Expose-Headers: grpc-status,grpc-message -