bug: grpc-transcode cant transcode empty array list to [] , but it did to {}

[nickname-nil]

Current Behavior

when back to front server, grpc transcode plugin cant transcode empty list (array) to [] , but to {} . use grpc example demo :

when list has content , it did well .

Expected Behavior

response empty list expect to get [] . not {}

Error Logs

No response

Steps to Reproduce

1. apisix 3.9 latest
2. grpc server golang
3. postman

Environment

1. apisix 3.9 latest
2. grpc server golang
3. postman

[zhoujiexiong]

@wenhaoZhao1997

May be there is a quick workaround: (mainly to prevent ambiguity of JSON object/array for the frontend)

1. set no_default_values pb_option of the plugin https://apisix.apache.org/docs/apisix/plugins/grpc-transcode/#options-for-pb_option

   with the option, the empty array field would be presented like this:

   {
      "items": {}
   }

   --->

   {}

   see also: https://github.com/starwing/lua-protobuf?tab=readme-ov-file#options auto_default_values(default) - act as use_default_values for proto3 and act as no_default_values for the others

2. at the frontend, test the items field if undefined or not

[nickname-nil]

@wenhaoZhao1997

May be there is a quick workaround: (mainly to prevent ambiguity of JSON object/array for the frontend)

1. set no_default_values pb_option of the plugin https://apisix.apache.org/docs/apisix/plugins/grpc-transcode/#options-for-pb_option with the option, the empty array field would be presented like this:

   {
      "items": {}
   }

   --->

   {}

   see also: https://github.com/starwing/lua-protobuf?tab=readme-ov-file#options auto_default_values(default) - act as use_default_values for proto3 and act as no_default_values for the others

2. at the frontend, test the items field if undefined or not

thx for your reply, yes i have tried pb_option no_default_values , and its indeed a solution. and i want to know wheather its a plugin lua code problem (when decoding proto file)or smth. if its a bug ,why didn't anyone else find out before.

[zhoujiexiong]

@wenhaoZhao1997 May be there is a quick workaround: (mainly to prevent ambiguity of JSON object/array for the frontend)

1. set no_default_values pb_option of the plugin https://apisix.apache.org/docs/apisix/plugins/grpc-transcode/#options-for-pb_option with the option, the empty array field would be presented like this:

   {
      "items": {}
   }

   --->

   {}

   see also: https://github.com/starwing/lua-protobuf?tab=readme-ov-file#options auto_default_values(default) - act as use_default_values for proto3 and act as no_default_values for the others

2. at the frontend, test the items field if undefined or not

thx for your reply, yes i have tried pb_option no_default_values , and its indeed a solution. and i want to know wheather its a plugin lua code problem (when decoding proto file)or smth. if its a bug ,why didn't anyone else find out before.

I think PR is needed. :D

The response logic/flow of the plugin at body_filter phase are simply like so:

1. pb.decode bytes stream from upstream to LUA table/object

2. cjson.encode the object to bytes stream then reply it to downstream

   Though cjson.decode_array_with_array_mt(true) is the default setting, but the object is decoded by pb.decode, without which tagging a table as a JSON array in case it is empty(for more to see FYI below).

   So cjson.encode output {} but not the expected [].

   FYI:

   --- decode_array_with_array_mt
   ---
   --- If enabled, JSON Arrays decoded by cjson.decode will result in Lua tables
   --- with the array_mt metatable. This can ensure a 1-to-1 relationship between
   --- arrays upon multiple encoding/decoding of your JSON data with this module.
   ---
   --- If disabled, JSON Arrays will be decoded to plain Lua tables, without the
   --- `cjson.array_mt` metatable.
   ---
   --- ## Example
   ---
   ---```lua
   --- local cjson = require "cjson"
   ---
   --- -- default behavior
   --- local my_json = [[{"my_array":[]}]]
   --- local t = cjson.decode(my_json)
   --- cjson.encode(t) -- {"my_array":{}} back to an object
   ---
   --- -- now, if this behavior is enabled
   --- cjson.decode_array_with_array_mt(true)
   ---
   --- local my_json = [[{"my_array":[]}]]
   --- local t = cjson.decode(my_json)
   --- cjson.encode(t) -- {"my_array":[]} properly re-encoded as an array
   ---```
   ---
   --- **NOTE:** This function is specific to the OpenResty cjson fork.
   ---
   ---@param enabled boolean # (default: false)
   function cjson.decode_array_with_array_mt(enabled) end

One of the solutions could be like this(FYR):

1. pb.hook to hook the ouput of pb.decode
2. at the hook function, traversal the fields of the decoded object recursively
3. setmetatable(EMPTY_FIELD_WITH_repeated_LABEL, cjson.empty_array_mt)

[zhoujiexiong]

@wenhaoZhao1997

Would you create PR to improve? :D

[nickname-nil]

@wenhaoZhao1997

Would you create PR to improve? :D

I don't think I have access to the repo， but we fix some code in plugins/grpc-transcode/response.lua , it did well in our environment , and can u review the code and create a pr ?

--
-- Licensed to the Apache Software Foundation (ASF) under one or more
-- contributor license agreements.  See the NOTICE file distributed with
-- this work for additional information regarding copyright ownership.
-- The ASF licenses this file to You under the Apache License, Version 2.0
-- (the "License"); you may not use this file except in compliance with
-- the License.  You may obtain a copy of the License at
--
--     http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- 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 util        = require("apisix.plugins.grpc-transcode.util")
local grpc_proto  = require("apisix.plugins.grpc-transcode.proto")
local core   = require("apisix.core")
local pb     = require("pb")
local ngx    = ngx
local string = string
local ngx_decode_base64 = ngx.decode_base64
local ipairs = ipairs
local pcall  = pcall

pb.option "decode_default_array"
local repeated_label = 3

function fetch_proto_array_names(proto_obj)
    local names = {}
    if type(proto_obj) == "table" then
        for k,v in pairs(proto_obj) do
            if type(v) == "table" then
               sub_names = fetch_proto_array_names(v)
               for sub_name,_ in pairs (sub_names ) do
                   names[sub_name]=1
               end
            end
        end
        if proto_obj["label"] == repeated_label then
             names[proto_obj["name"]]=1
        end
    end
    return names
end

function set_default_array(tab,array_names )
   if type(tab) ~= "table" then
       return false
   end
   for k, v  in pairs(tab) do
        if type(v) == "table" then
           if array_names[k] == 1 then
                setmetatable(v,core.json.array_mt)
           end
           set_default_array(v,array_names)
        end
    end
end

local function handle_error_response(status_detail_type, proto)
    local err_msg

    local grpc_status = ngx.header["grpc-status-details-bin"]
    if grpc_status then
        grpc_status = ngx_decode_base64(grpc_status)
        if grpc_status == nil then
            err_msg = "grpc-status-details-bin is not base64 format"
            ngx.arg[1] = err_msg
            return err_msg
        end

        local status_pb_state = grpc_proto.fetch_status_pb_state()
        local old_pb_state = pb.state(status_pb_state)

        local ok, decoded_grpc_status = pcall(pb.decode, "grpc_status.ErrorStatus", grpc_status)
        pb.state(old_pb_state)
        if not ok then
            err_msg = "failed to call pb.decode to decode grpc-status-details-bin"
            ngx.arg[1] = err_msg
            return err_msg .. ", err: " .. decoded_grpc_status
        end

        if not decoded_grpc_status then
            err_msg = "failed to decode grpc-status-details-bin"
            ngx.arg[1] = err_msg
            return err_msg
        end

        local details = decoded_grpc_status.details
        if status_detail_type and details then
            local decoded_details = {}
            for _, detail in ipairs(details) do
                local pb_old_state = pb.state(proto.pb_state)
                local ok, err_or_value = pcall(pb.decode, status_detail_type, detail.value)
                pb.state(pb_old_state)
                if not ok then
                    err_msg = "failed to call pb.decode to decode details in "
                           .. "grpc-status-details-bin"
                    ngx.arg[1] = err_msg
                    return err_msg .. ", err: " .. err_or_value
                end

                if not err_or_value then
                    err_msg = "failed to decode details in grpc-status-details-bin"
                    ngx.arg[1] = err_msg
                    return err_msg
                end

                core.table.insert(decoded_details, err_or_value)
            end

            decoded_grpc_status.details = decoded_details
        end

        local resp_body = {error = decoded_grpc_status}
        local response, err = core.json.encode(resp_body)
        if not response then
            err_msg = "failed to json_encode response body"
            ngx.arg[1] = err_msg
            return err_msg .. ", error: " .. err
        end

        ngx.arg[1] = response
    end
end

return function(ctx, proto, service, method, pb_option, show_status_in_body, status_detail_type)
    local buffer = core.response.hold_body_chunk(ctx)
    if not buffer then
        return nil
    end

    -- handle error response after the last response chunk
    if ngx.status >= 300 and show_status_in_body then
        return handle_error_response(status_detail_type, proto)
    end

    -- when body has already been read by other plugin
    -- the buffer is an empty string
    if buffer == "" and ctx.resp_body then
        buffer = ctx.resp_body
    end

    local m = util.find_method(proto, service, method)
    if not m then
        return false, "2.Undefined service method: " .. service .. "/" .. method
                      .. " end."
    end

    if not ngx.req.get_headers()["X-Grpc-Web"] then
        buffer = string.sub(buffer, 6)
    end

    local pb_old_state = pb.state(proto.pb_state)
    util.set_options(proto, pb_option)

    local err_msg
    local decoded = pb.decode(m.output_type, buffer)
    pb.state(pb_old_state)
    if not decoded then
        err_msg = "failed to decode response data by protobuf"
        ngx.arg[1] = err_msg
        return err_msg
    end

    local array_names = fetch_proto_array_names ( proto )
    set_default_array(decoded,array_names)

    local response, err = core.json.encode(decoded)
    if not response then
        err_msg = "failed to json_encode response body"
        ngx.arg[1] = err_msg
        return err_msg .. ", err: " .. err
    end

    ngx.arg[1] = response
    return nil
end

[zhoujiexiong]

@wenhaoZhao1997 Would you create PR to improve? :D

I don't think I have access to the repo， but we fix some code in plugins/grpc-transcode/response.lua , it did well in our environment , and can u review the code and create a pr ?

--
-- Licensed to the Apache Software Foundation (ASF) under one or more
-- contributor license agreements.  See the NOTICE file distributed with
-- this work for additional information regarding copyright ownership.
-- The ASF licenses this file to You under the Apache License, Version 2.0
-- (the "License"); you may not use this file except in compliance with
-- the License.  You may obtain a copy of the License at
--
--     http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- 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 util        = require("apisix.plugins.grpc-transcode.util")
local grpc_proto  = require("apisix.plugins.grpc-transcode.proto")
local core   = require("apisix.core")
local pb     = require("pb")
local ngx    = ngx
local string = string
local ngx_decode_base64 = ngx.decode_base64
local ipairs = ipairs
local pcall  = pcall

pb.option "decode_default_array"
local repeated_label = 3

function fetch_proto_array_names(proto_obj)
    local names = {}
    if type(proto_obj) == "table" then
        for k,v in pairs(proto_obj) do
            if type(v) == "table" then
               sub_names = fetch_proto_array_names(v)
               for sub_name,_ in pairs (sub_names ) do
                   names[sub_name]=1
               end
            end
        end
        if proto_obj["label"] == repeated_label then
             names[proto_obj["name"]]=1
        end
    end
    return names
end

function set_default_array(tab,array_names )
   if type(tab) ~= "table" then
       return false
   end
   for k, v  in pairs(tab) do
        if type(v) == "table" then
           if array_names[k] == 1 then
                setmetatable(v,core.json.array_mt)
           end
           set_default_array(v,array_names)
        end
    end
end

local function handle_error_response(status_detail_type, proto)
    local err_msg

    local grpc_status = ngx.header["grpc-status-details-bin"]
    if grpc_status then
        grpc_status = ngx_decode_base64(grpc_status)
        if grpc_status == nil then
            err_msg = "grpc-status-details-bin is not base64 format"
            ngx.arg[1] = err_msg
            return err_msg
        end

        local status_pb_state = grpc_proto.fetch_status_pb_state()
        local old_pb_state = pb.state(status_pb_state)

        local ok, decoded_grpc_status = pcall(pb.decode, "grpc_status.ErrorStatus", grpc_status)
        pb.state(old_pb_state)
        if not ok then
            err_msg = "failed to call pb.decode to decode grpc-status-details-bin"
            ngx.arg[1] = err_msg
            return err_msg .. ", err: " .. decoded_grpc_status
        end

        if not decoded_grpc_status then
            err_msg = "failed to decode grpc-status-details-bin"
            ngx.arg[1] = err_msg
            return err_msg
        end

        local details = decoded_grpc_status.details
        if status_detail_type and details then
            local decoded_details = {}
            for _, detail in ipairs(details) do
                local pb_old_state = pb.state(proto.pb_state)
                local ok, err_or_value = pcall(pb.decode, status_detail_type, detail.value)
                pb.state(pb_old_state)
                if not ok then
                    err_msg = "failed to call pb.decode to decode details in "
                           .. "grpc-status-details-bin"
                    ngx.arg[1] = err_msg
                    return err_msg .. ", err: " .. err_or_value
                end

                if not err_or_value then
                    err_msg = "failed to decode details in grpc-status-details-bin"
                    ngx.arg[1] = err_msg
                    return err_msg
                end

                core.table.insert(decoded_details, err_or_value)
            end

            decoded_grpc_status.details = decoded_details
        end

        local resp_body = {error = decoded_grpc_status}
        local response, err = core.json.encode(resp_body)
        if not response then
            err_msg = "failed to json_encode response body"
            ngx.arg[1] = err_msg
            return err_msg .. ", error: " .. err
        end

        ngx.arg[1] = response
    end
end

return function(ctx, proto, service, method, pb_option, show_status_in_body, status_detail_type)
    local buffer = core.response.hold_body_chunk(ctx)
    if not buffer then
        return nil
    end

    -- handle error response after the last response chunk
    if ngx.status >= 300 and show_status_in_body then
        return handle_error_response(status_detail_type, proto)
    end

    -- when body has already been read by other plugin
    -- the buffer is an empty string
    if buffer == "" and ctx.resp_body then
        buffer = ctx.resp_body
    end

    local m = util.find_method(proto, service, method)
    if not m then
        return false, "2.Undefined service method: " .. service .. "/" .. method
                      .. " end."
    end

    if not ngx.req.get_headers()["X-Grpc-Web"] then
        buffer = string.sub(buffer, 6)
    end

    local pb_old_state = pb.state(proto.pb_state)
    util.set_options(proto, pb_option)

    local err_msg
    local decoded = pb.decode(m.output_type, buffer)
    pb.state(pb_old_state)
    if not decoded then
        err_msg = "failed to decode response data by protobuf"
        ngx.arg[1] = err_msg
        return err_msg
    end

    local array_names = fetch_proto_array_names ( proto )
    set_default_array(decoded,array_names)

    local response, err = core.json.encode(decoded)
    if not response then
        err_msg = "failed to json_encode response body"
        ngx.arg[1] = err_msg
        return err_msg .. ", err: " .. err
    end

    ngx.arg[1] = response
    return nil
end

@wenhaoZhao1997

Thank you for your reply.

About creating PR:

1. fork apache/apisix to Org under your Github account
2. add changes above to the project and commit to the branch, for example, your_branch_name@YourOrg/apisix
3. new pull request from your forked repoYourOrg/apisix.

That way, you can add more(code and test cases) to the PR and we can review your update more easily.