Skip to content

test: cover grpc-transcode empty array responses #12649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bytelazy wants to merge 3 commits into
base: master
Choose a base branch
from
+95 −0
Open

test: cover grpc-transcode empty array responses #12649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa7741f
test: cover grpc-transcode empty array responses
bytelazy Oct 4, 2025
6aac6c5
remove option: decode_default_array
bytelazy Oct 12, 2025
5a48b4b
Merge remote-tracking branch 'upstream/master' into grpc-transcode-em…
bytelazy Nov 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions apisix/plugins/grpc-transcode/response.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,47 @@ local string = string
local ngx_decode_base64 = ngx.decode_base64
local ipairs = ipairs
local pcall = pcall
local type = type
local pairs = pairs
local setmetatable = setmetatable

-- Protobuf repeated field label value
local PROTOBUF_REPEATED_LABEL = 3
local repeated_label = PROTOBUF_REPEATED_LABEL

local function fetch_proto_array_names(proto_obj)
Copy link
Member

@nic-6443 nic-6443 Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function does not take into account the possibility of duplicate names in nested fields, for example:

message Company {
    repeated string name = 1;
}
message HelloRequest {
    string name = 1;
    Company company = 2;
}

local names = {}
if type(proto_obj) == "table" then
Copy link
Contributor

@AlinsRan AlinsRan Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if type(proto_obj) == "table" then
if type(proto_obj) ~= "table" then
return
end

Copy link
Author

@bytelazy bytelazy Dec 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch_proto_array_names always returns a table so that its callers can safely iterate with pairs(sub_names). Returning nil for the base case would trigger a runtime error when pairs(nil) is evaluated, and it would not change correctness because the existing code already skips non-table values

for k,v in pairs(proto_obj) do
if type(v) == "table" then
local sub_names = fetch_proto_array_names(v)
for sub_name,_ in pairs(sub_names) do
names[sub_name] = 1
Copy link
Member

@nic-6443 nic-6443 Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
names[sub_name] = 1
names[sub_name] = true

end
end
end
if proto_obj["label"] == repeated_label then
if proto_obj["name"] then
names[proto_obj["name"]] = 1
end
end
end
return names
end

local function set_default_array(tab, array_names)
if type(tab) ~= "table" then
return
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)
Expand Down Expand Up @@ -132,6 +173,9 @@ return function(ctx, proto, service, method, pb_option, show_status_in_body, sta
return err_msg
end

local array_names = fetch_proto_array_names(proto)
Copy link
Contributor

@AlinsRan AlinsRan Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can cache it without having to reprocess it every time a request is made.

apisix/apisix/plugins/grpc-transcode/proto.lua

Line 124 in 7816e92

local function create_proto_obj(proto_id)

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"
Expand Down
51 changes: 51 additions & 0 deletions t/plugin/grpc-transcode.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,3 +761,54 @@ POST /grpctest
Content-Type: application/json
--- response_body eval
qr/"gender":2/



=== TEST 30: set route (return empty array from grpc server)
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"methods": ["GET", "POST"],
"uri": "/grpctest",
"plugins": {
"grpc-transcode": {
"proto_id": "1",
"service": "helloworld.Greeter",
"method": "SayHello"
}
},
"upstream": {
"scheme": "grpc",
"type": "roundrobin",
"nodes": {
"127.0.0.1:10051": 1
}
}
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed



=== TEST 31: hit route, response keeps empty array
--- request
POST /grpctest
{"name":"world","items":[]}
--- more_headers
Content-Type: application/json
--- response_body eval
qr/"items":\[\]/
Loading