nic-6443 commented on code in PR #12649:
URL: https://github.com/apache/apisix/pull/12649#discussion_r3393091865
##########
apisix/plugins/grpc-transcode/proto.lua:
##########
@@ -93,6 +183,7 @@ local function compile_proto_bin(content)
local compiled = {}
compiled[proto_fake_file] = {}
compiled[proto_fake_file].index = index
+ compiled.message_index = build_message_index(files)
Review Comment:
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`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
