Skip to content

Commit a8b8ea0

Browse files
habermancopybara-github
haberman
authored and
copybara-github
committed
Breaking Change: fixed json_encode/json_decode to use the message's pool.
This bug arises only in the uncommon case where there is more than one DescriptorPool. In such a case, JSON encode/decode should always use the pool of the message being encoded/decoded, not the generated pool. Closes #15281 COPYBARA_INTEGRATE_REVIEW=#15281 from protocolbuffers:ruby-json-pool-fix 91e2dc5 PiperOrigin-RevId: 596027770
1 parent 55260d8 commit a8b8ea0

2 files changed

Lines changed: 45 additions & 10 deletions

File tree

‎ruby/ext/google/protobuf_c/message.c‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -975,9 +975,6 @@ static VALUE Message_decode_json(int argc, VALUE* argv, VALUE klass) {
975975
int options = 0;
976976
upb_Status status;
977977

978-
// TODO: use this message's pool instead.
979-
const upb_DefPool* symtab = DescriptorPool_GetSymtab(generated_pool);
980-
981978
if (argc < 1 || argc > 2) {
982979
rb_raise(rb_eArgError, "Expected 1 or 2 arguments.");
983980
}
@@ -1011,8 +1008,9 @@ static VALUE Message_decode_json(int argc, VALUE* argv, VALUE klass) {
10111008
}
10121009

10131010
upb_Status_Clear(&status);
1011+
const upb_DefPool* pool = upb_FileDef_Pool(upb_MessageDef_File(msg->msgdef));
10141012
if (!upb_JsonDecode(RSTRING_PTR(data), RSTRING_LEN(data),
1015-
(upb_Message*)msg->msg, msg->msgdef, symtab, options,
1013+
(upb_Message*)msg->msg, msg->msgdef, pool, options,
10161014
Arena_get(msg->arena), &status)) {
10171015
rb_raise(cParseError, "Error occurred during parsing: %s",
10181016
upb_Status_ErrorMessage(&status));
@@ -1091,9 +1089,6 @@ static VALUE Message_encode_json(int argc, VALUE* argv, VALUE klass) {
10911089
size_t size;
10921090
upb_Status status;
10931091

1094-
// TODO: use this message's pool instead.
1095-
const upb_DefPool* symtab = DescriptorPool_GetSymtab(generated_pool);
1096-
10971092
if (argc < 1 || argc > 2) {
10981093
rb_raise(rb_eArgError, "Expected 1 or 2 arguments.");
10991094
}
@@ -1128,8 +1123,9 @@ static VALUE Message_encode_json(int argc, VALUE* argv, VALUE klass) {
11281123
}
11291124

11301125
upb_Status_Clear(&status);
1131-
size = upb_JsonEncode(msg->msg, msg->msgdef, symtab, options, buf,
1132-
sizeof(buf), &status);
1126+
const upb_DefPool* pool = upb_FileDef_Pool(upb_MessageDef_File(msg->msgdef));
1127+
size = upb_JsonEncode(msg->msg, msg->msgdef, pool, options, buf, sizeof(buf),
1128+
&status);
11331129

11341130
if (!upb_Status_IsOk(&status)) {
11351131
rb_raise(cParseError, "Error occurred during encoding: %s",
@@ -1139,7 +1135,7 @@ static VALUE Message_encode_json(int argc, VALUE* argv, VALUE klass) {
11391135
VALUE ret;
11401136
if (size >= sizeof(buf)) {
11411137
char* buf2 = malloc(size + 1);
1142-
upb_JsonEncode(msg->msg, msg->msgdef, symtab, options, buf2, size + 1,
1138+
upb_JsonEncode(msg->msg, msg->msgdef, pool, options, buf2, size + 1,
11431139
&status);
11441140
ret = rb_str_new(buf2, size);
11451141
free(buf2);

‎ruby/tests/basic_proto2.rb‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,45 @@ def test_extension
263263
assert_equal 42, extension.get(message)
264264
end
265265

266+
def test_extension_json
267+
omit "Java Protobuf JsonFormat does not handle Proto2 extensions" if defined? JRUBY_VERSION and :NATIVE == Google::Protobuf::IMPLEMENTATION
268+
message = TestExtensions.decode_json '{"[basic_test_proto2.optional_int32_extension]": 123}'
269+
extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.optional_int32_extension'
270+
assert_instance_of Google::Protobuf::FieldDescriptor, extension
271+
assert_equal 123, extension.get(message)
272+
end
273+
274+
def test_extension_json_separate_pool
275+
omit "Java Protobuf JsonFormat does not handle Proto2 extensions" if defined? JRUBY_VERSION and :NATIVE == Google::Protobuf::IMPLEMENTATION
276+
pool = Google::Protobuf::DescriptorPool.new
277+
278+
# This serialized descriptor is a subset of basic_test_proto2.proto:
279+
#
280+
# syntax = "proto2";
281+
# package basic_test_proto2;
282+
#
283+
# message TestExtensions {
284+
# extensions 1 to max;
285+
# }
286+
#
287+
# extend TestExtensions {
288+
# # Same extension as basic_test_proto2.proto, but with a different
289+
# # name.
290+
# optional int32 different_optional_int32_extension = 1;
291+
# }
292+
#
293+
descriptor_data = "\n\x17\x62\x61sic_test_proto2.proto\x12\x11\x62\x61sic_test_proto2\"\x1a\n\x0eTestExtensions*\x08\x08\x01\x10\x80\x80\x80\x80\x02:M\n\"different_optional_int32_extension\x12!.basic_test_proto2.TestExtensions\x18\x01 \x01(\x05"
294+
pool.add_serialized_file(descriptor_data)
295+
message_class = pool.lookup("basic_test_proto2.TestExtensions").msgclass
296+
extension = pool.lookup 'basic_test_proto2.different_optional_int32_extension'
297+
298+
message = message_class.decode_json '{"[basic_test_proto2.different_optional_int32_extension]": 123}'
299+
assert_equal 123, extension.get(message)
300+
301+
message2 = message_class.decode_json(message_class.encode_json(message))
302+
assert_equal 123, extension.get(message2)
303+
end
304+
266305
def test_nested_extension
267306
message = TestExtensions.new
268307
extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestNestedExtension.test'

0 commit comments

Comments
 (0)