apisix icon indicating copy to clipboard operation
apisix copied to clipboard

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

Open nickname-nil opened this issue 1 year ago • 13 comments

Current Behavior

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

when list has content , it did well . image

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

nickname-nil avatar Jul 26 '24 11:07 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

zhoujiexiong avatar Jul 30 '24 02:07 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.

nickname-nil avatar Jul 30 '24 03:07 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.

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 avatar Jul 30 '24 05:07 zhoujiexiong

@wenhaoZhao1997

Would you create PR to improve? :D

zhoujiexiong avatar Aug 05 '24 07:08 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

nickname-nil avatar Aug 13 '24 03:08 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

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

zhoujiexiong avatar Aug 13 '24 03:08 zhoujiexiong

it seems that this problem was solved in latest apisix.

apisix version:

/usr/local/openresty//luajit/bin/luajit ./apisix/cli/apisix.lua version
3.12.0

proto file:

syntax = "proto3";

package user;

option go_package = "grpc-test/proto";

enum Gender {
  GENDER_UNSPECIFIED = 0;
  GENDER_MALE = 1;
  GENDER_FEMALE = 2;
}

message UserRequest {
  string name = 1;
  int32 age = 2;
}


message UserResponse {
  Gender gender = 1;
  repeated string items = 2;
  string message = 3;
}


service UserService {
  rpc GetUserInfo(UserRequest) returns (UserResponse) {}
} 

grpc server code

array containing values

package main

import (
	"context"
	"fmt"
	"log"
	"net"

	"google.golang.org/grpc"

	"grpc-test/proto"
)

type UserServiceServer struct {
	proto.UnimplementedUserServiceServer
}


func (s *UserServiceServer) GetUserInfo(ctx context.Context, req *proto.UserRequest) (*proto.UserResponse, error) {
	return &proto.UserResponse{
		Gender: proto.Gender_GENDER_MALE,
		Items:  []string{"1", "2"},
		Message: "Hello world, name: " + req.Name + ", age: " +  fmt.Sprint(req.Age),
	}, nil
}

func main() {
	listener, err := net.Listen("tcp", ":50051")
	if err != nil {
		log.Fatalf("failed to listen: %v", err)
	}
	grpcServer := grpc.NewServer()
	proto.RegisterUserServiceServer(grpcServer, &UserServiceServer{})
	log.Println("gRPC server listening on :50051")
	if err := grpcServer.Serve(listener); err != nil {
		log.Fatalf("failed to serve: %v", err)
	}
} 

request result Image

array without values

package main

import (
	"context"
	"fmt"
	"log"
	"net"

	"google.golang.org/grpc"

	"grpc-test/proto"
)

// UserServiceServer 实现 proto 生成的接口
type UserServiceServer struct {
	proto.UnimplementedUserServiceServer
}

// GetUserInfo 实现接口
func (s *UserServiceServer) GetUserInfo(ctx context.Context, req *proto.UserRequest) (*proto.UserResponse, error) {
	return &proto.UserResponse{
		Gender: proto.Gender_GENDER_MALE,
		Items:  []string{},
		Message: "Hello world, name: " + req.Name + ", age: " +  fmt.Sprint(req.Age),
	}, nil
}

func main() {
	listener, err := net.Listen("tcp", ":50051")
	if err != nil {
		log.Fatalf("failed to listen: %v", err)
	}
	grpcServer := grpc.NewServer()
	proto.RegisterUserServiceServer(grpcServer, &UserServiceServer{})
	log.Println("gRPC server listening on :50051")
	if err := grpcServer.Serve(listener); err != nil {
		log.Fatalf("failed to serve: %v", err)
	}
} 

request result

Image

bytelazy avatar Jun 19 '25 13:06 bytelazy

Hi @3kis, I have verified that the problem still exists. Can you provide your apisix routing configuration?

Baoyuantop avatar Jun 20 '25 02:06 Baoyuantop

Hi @3kis, I have verified that the problem still exists. Can you provide your apisix routing configuration?

curl http://127.0.0.1:9180/apisix/admin/routes -H "X-API-KEY: **************"

{"total":1,"list":[{"key":"/apisix/routes/1","value":{"create_time":1750318948,"status":1,"priority":0,"update_time":1750318948,"uri":"/user.UserService/GetUserInfo","methods":["POST","GET"],"upstream":{"scheme":"grpc","hash_on":"vars","nodes":{"10.0.41.253:50051":1},"pass_host":"pass","type":"roundrobin"},"id":"1"},"modifiedIndex":28,"createdIndex":28}]}

bytelazy avatar Jun 20 '25 02:06 bytelazy

This issue is about the grpc-transcode plugin. Your route is a proxy of the gRPC service, this is different.

Baoyuantop avatar Jun 20 '25 02:06 Baoyuantop

I noticed this issue. Have you tried checking the error logs or console output? That might help narrow down the root cause. I'd be happy to help investigate if you can share more details about your environment (OS, version, etc.).

tysoncung avatar Oct 09 '25 02:10 tysoncung

Hi @tysoncung, #12649 has been created to address this issue. Welcome to review it.

Baoyuantop avatar Oct 11 '25 04:10 Baoyuantop