fix(grpc-transcode): serialize empty repeated fields as JSON arrays instead of objects

[bytelazy]

Description

In the grpc-transcode plugin, when a repeated field in the gRPC response is empty, it is incorrectly encoded as an empty JSON object ({}) instead of an empty JSON array ([]). This behavior does not comply with the gRPC-JSON transcoding specification and can cause type-related errors on the client-side, which expects an array.

Fixes #11440

Checklist

• I have explained the need for this PR and the problem it solves
• I have explained the changes or the new features added to this PR
• I have added tests corresponding to this change
• I have updated the documentation to reflect this change
• I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)

Changes or the new features added to this PR
The main changes are in the response.lua file to correctly handle the serialization of repeated fields.

Identify repeated fields: A new function fetch_proto_array_names has been added. It recursively traverses the loaded Protobuf schema definition to identify and collect the names of all fields marked as repeated.

Ensure correct JSON array serialization:

A new function set_default_array has been introduced. After the gRPC response is decoded into a Lua table, this function traverses the table.

For any field that was identified as repeated, it sets a specific metatable (core.json.array_mt).

This metatable forces the core.json.encode function to serialize the corresponding Lua table as a JSON array ([]), even when it is empty.

This logic is applied just before the decoded response is encoded into the final JSON output, ensuring the structure is correct.

[Baoyuantop]

Hi @bytelazy, could you merge the latest main branch?

[bytelazy]

Hi @bytelazy, could you merge the latest main branch?

sure~

[membphis]

pls merge the master branch,

fix the ci case:
#12761

[Baoyuantop]

Hi @bytelazy, please check these comments.

[bytelazy]

pls merge the master branch,

fix the ci case: #12761

Thanks for the heads-up! I've merged the master branch

[github-actions]

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

[Copilot]

Pull request overview

This PR fixes a bug in the grpc-transcode plugin where empty repeated fields in gRPC responses were incorrectly serialized as empty JSON objects ({}) instead of empty JSON arrays ([]). The fix ensures compliance with the gRPC-JSON transcoding specification and prevents type-related errors on the client side.

Changes:

• Added logic to build a message index from proto definitions during compilation, caching field metadata including repeated field information
• Implemented set_default_array function to recursively traverse decoded gRPC responses and set the array metatable on repeated fields
• Added comprehensive unit tests to validate the fix handles nested messages, maps, and field name scoping correctly

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
t/plugin/grpc-transcode.t	Adds integration test (TEST 30-31) to verify empty arrays in gRPC responses are correctly transcoded to JSON arrays
t/plugin/grpc-transcode-arrays.t	New unit test file with comprehensive tests for repeated field handling, including field name scoping and map field semantics
apisix/plugins/grpc-transcode/response.lua	Implements set_default_array function to recursively set array metatables on repeated fields, properly handling nested messages and maps
apisix/plugins/grpc-transcode/proto.lua	Adds message index building during proto compilation to cache field metadata, enabling efficient identification of repeated and map fields

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[moonming]

Hi @bytelazy, thank you for fixing the grpc-transcode empty array issue (#11440)!

One thing to fix: The PR title says "test: cover grpc-transcode empty array responses" but the PR includes actual code changes beyond tests. Please update the commit message/title to reflect the actual change (e.g., "fix(grpc-transcode): handle empty array responses correctly").

Could you also rebase on the latest master? Thank you!

[bytelazy]

Hi @bytelazy, thank you for fixing the grpc-transcode empty array issue (#11440)!

One thing to fix: The PR title says "test: cover grpc-transcode empty array responses" but the PR includes actual code changes beyond tests. Please update the commit message/title to reflect the actual change (e.g., "fix(grpc-transcode): handle empty array responses correctly").

Could you also rebase on the latest master? Thank you!

@moonming Thanks for the review.

I see that my branch currently includes several merge commits from upstream/master. To confirm, when you asked me to rebase onto the latest master, would you prefer that I also rewrite the branch into a linear history?

If so, I can squash the commits and update the PR accordingly.

[Baoyuantop]

Hi @bytelazy, that's okay. A squash will be performed during the final merge. Please check all the review information above. If it has been addressed, please mark it as resolved; this will help with future reviews.

[Baoyuantop]

Hi @bytelazy, there are still some review comments that need to be processed.

[bytelazy]

Hi @bytelazy, there are still some review comments that need to be processed.

Hi @bytelazy, there are still some review comments that need to be processed.

@Baoyuantop Hi~ Thanks for the reminder. I have addressed the remaining review comments in b2be049 and resolved the corresponding conversations.

[Baoyuantop]

Hi @bytelazy, please fix the failed CI

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

PR description mentions adding a fetch_proto_array_names function, but the implementation actually builds proto.message_index and relies on set_default_array + descriptor metadata to enforce JSON array encoding. Please update the PR description (or rename references) to reflect the current approach so reviewers/users aren't looking for a function that doesn't exist.

[nic-6443]

This doesn't work for protos uploaded as binary descriptor sets. pb.decode("google.protobuf.FileDescriptorSet", content) decodes the label and type enums as name strings (lua-protobuf defaults to enum_as_name), so field.label here is "LABEL_REPEATED" and field.type is "TYPE_MESSAGE". The numeric comparisons in response.lua (label == 3, type == 11) then never match, so no array tagging happens at all on this path — I verified this against lua-protobuf 0.5.3 (the version pinned in the rockspec). It's also nondeterministic: pb options are global to the worker, so if another route's pb_option switched to enum_as_value before this proto gets compiled, you'd get numbers instead.

The text path is unaffected because protoc.lua builds descriptor tables with numeric label/type, which is why the new tests pass. If you keep this approach, the label/type values need to be normalized here, and it would be good to add a test case using a binary proto like the existing ones in t/plugin/grpc-transcode2.t (t/grpc_server_example/proto.pb).

[nic-6443]

I think there is a much simpler way to achieve this that lets lua-protobuf do all the heavy lifting. The version we pin (0.5.3) supports a special *array default metatable (docs): if we call pb.defaults("*array", core.json.array_mt) once in compile_proto() (inside the pb.state window, right before compiled.pb_state = pb.state(old_pb_state)), then pb.decode attaches the array metatable to every repeated field table by itself.

I verified this locally against lua-protobuf 0.5.3 with the plugin's default options: empty repeated fields encode as [], map fields correctly stay {} (lua-protobuf distinguishes maps from arrays natively), nested repeated fields inside map values and repeated messages also get [], and it works for both the text proto path and the binary descriptor path. So it would replace build_message_index + set_default_array entirely with one line, with zero per-request traversal cost.

One caveat: pb.defaults is tied to the active pb state, so it must be set per compiled proto rather than at module load time. What do you think?

[AlinsRan]

Thanks for digging into this @bytelazy — the diagnosis in this PR is right, and the repro cases were useful.

#11440 has been fixed by #13678, so I'm closing this one. Since we ended up with a different implementation, here's the reasoning rather than just a "superseded".

The binary descriptor path isn't actually fixed here. set_default_array() keys off field_info.label == 3 and field.type == 11. Those numbers only hold for the text-proto path, where protoc:parse() builds the descriptor directly. compile_proto_bin() instead goes through pb.decode("google.protobuf.FileDescriptorSet", content), and under the default enum_as_name that yields strings:

TEXT path  label=3               type=9
BIN  path  label=LABEL_REPEATED  type=TYPE_STRING

So for any proto configured as a base64 .pb descriptor set, the comparisons never match and empty repeated fields still encode as {} — silently, with no error. That's the main reason for going another way, not a style preference.

Two smaller things. Building a message index and walking the whole decoded table per response costs something on large bodies, and it re-implements bookkeeping that lua-protobuf already does at decode time. And turning response.lua into setmetatable(_M, {__call = ...}) just to expose _M._TEST.set_default_array changes the module's calling convention to make an internal helper testable.

What #13678 does instead: lua-protobuf 0.5.3 (the version pinned in the rockspec) supports a *array default metatable, so pb.decode tags repeated fields itself:

pb.defaults("*array", core.json.array_mt)

Set once per compiled proto state. Maps keep object semantics (lua-protobuf distinguishes maps from arrays natively), nested/repeated messages and map values are covered, both the text and binary paths work, and there's no per-request traversal.