Skip to content

fix(grpc-transcode): correctly serialize empty repeated fields to [] #12391

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.

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
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

3kis
Copy link

@3kis 3kis commented Jun 30, 2025

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.

@3kis 3kis marked this pull request as ready for review June 30, 2025 15:36
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working plugin labels Jun 30, 2025
@Baoyuantop Baoyuantop moved this to 👀 In review in ⚡️ Apache APISIX Roadmap Jul 1, 2025
Copy link
Contributor

@Baoyuantop Baoyuantop left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

Two things need to be done:

  1. Need to add corresponding tests for this fix, please feel free to ask questions if you encounter any problems.
  2. Need to pass all CI.

@3kis
Copy link
Author

3kis commented Jul 2, 2025

Thank you for your contribution!

Two things need to be done:

  1. Need to add corresponding tests for this fix, please feel free to ask questions if you encounter any problems.
  2. Need to pass all CI.

All CI have passed.
Under which directory should I add the corresponding tests for this fix ?

@Baoyuantop
Copy link
Contributor

Thank you for your contribution!
Two things need to be done:

  1. Need to add corresponding tests for this fix, please feel free to ask questions if you encounter any problems.
  2. Need to pass all CI.

All CI have passed. Under which directory should I add the corresponding tests for this fix ?

Add test case for grpc-transcode plugin in t/plugin/grpc-transcode.t

@3kis
Copy link
Author

3kis commented Jul 2, 2025

Thank you for your contribution!
Two things need to be done:

  1. Need to add corresponding tests for this fix, please feel free to ask questions if you encounter any problems.
  2. Need to pass all CI.

All CI have passed. Under which directory should I add the corresponding tests for this fix ?

Add test case for grpc-transcode plugin in t/plugin/grpc-transcode.t

I don't know how to run the test file. Can you give me some reference documents?

@Baoyuantop
Copy link
Contributor

@3kis
Copy link
Author

3kis commented Jul 3, 2025

Thank you for your contribution!
Two things need to be done:

  1. Need to add corresponding tests for this fix, please feel free to ask questions if you encounter any problems.
  2. Need to pass all CI.

All CI have passed. Under which directory should I add the corresponding tests for this fix ?

Add test case for grpc-transcode plugin in t/plugin/grpc-transcode.t

Based on the documentation you provided, after running prove -I. -I../test-nginx/inc -I../test-nginx/lib -r t/plugin/grpc-transcode.t, I encountered connect() failed (111: Connection refused) while connecting to upstream, client: 127.0.0.1.

Upon observation, I noticed that the /t directory contains a /grpc_server_example directory. Does this mean that to test t/plugin/grpc-transcode.t, I need to download Golang in the dev container and run /grpc_server_example/main.go?

@Baoyuantop
Copy link
Contributor

I need to download Golang in the dev container and run /grpc_server_example/main.go?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working plugin size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

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