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.
pull/15280/head
Joshua Haberman 11 months ago
parent 31313b1652
commit 8268c9a6dc
  1. 14
      ruby/ext/google/protobuf_c/message.c
  2. 24
      ruby/tests/basic_proto2.rb

@ -975,9 +975,6 @@ static VALUE Message_decode_json(int argc, VALUE* argv, VALUE klass) {
int options = 0; int options = 0;
upb_Status status; upb_Status status;
// TODO: use this message's pool instead.
const upb_DefPool* symtab = DescriptorPool_GetSymtab(generated_pool);
if (argc < 1 || argc > 2) { if (argc < 1 || argc > 2) {
rb_raise(rb_eArgError, "Expected 1 or 2 arguments."); rb_raise(rb_eArgError, "Expected 1 or 2 arguments.");
} }
@ -1011,8 +1008,9 @@ static VALUE Message_decode_json(int argc, VALUE* argv, VALUE klass) {
} }
upb_Status_Clear(&status); upb_Status_Clear(&status);
const upb_DefPool* pool = upb_FileDef_Pool(upb_MessageDef_File(msg->msgdef));
if (!upb_JsonDecode(RSTRING_PTR(data), RSTRING_LEN(data), if (!upb_JsonDecode(RSTRING_PTR(data), RSTRING_LEN(data),
(upb_Message*)msg->msg, msg->msgdef, symtab, options, (upb_Message*)msg->msg, msg->msgdef, pool, options,
Arena_get(msg->arena), &status)) { Arena_get(msg->arena), &status)) {
rb_raise(cParseError, "Error occurred during parsing: %s", rb_raise(cParseError, "Error occurred during parsing: %s",
upb_Status_ErrorMessage(&status)); upb_Status_ErrorMessage(&status));
@ -1091,9 +1089,6 @@ static VALUE Message_encode_json(int argc, VALUE* argv, VALUE klass) {
size_t size; size_t size;
upb_Status status; upb_Status status;
// TODO: use this message's pool instead.
const upb_DefPool* symtab = DescriptorPool_GetSymtab(generated_pool);
if (argc < 1 || argc > 2) { if (argc < 1 || argc > 2) {
rb_raise(rb_eArgError, "Expected 1 or 2 arguments."); rb_raise(rb_eArgError, "Expected 1 or 2 arguments.");
} }
@ -1128,7 +1123,8 @@ static VALUE Message_encode_json(int argc, VALUE* argv, VALUE klass) {
} }
upb_Status_Clear(&status); upb_Status_Clear(&status);
size = upb_JsonEncode(msg->msg, msg->msgdef, symtab, options, buf, const upb_DefPool* pool = upb_FileDef_Pool(upb_MessageDef_File(msg->msgdef));
size = upb_JsonEncode(msg->msg, msg->msgdef, pool, options, buf,
sizeof(buf), &status); sizeof(buf), &status);
if (!upb_Status_IsOk(&status)) { if (!upb_Status_IsOk(&status)) {
@ -1139,7 +1135,7 @@ static VALUE Message_encode_json(int argc, VALUE* argv, VALUE klass) {
VALUE ret; VALUE ret;
if (size >= sizeof(buf)) { if (size >= sizeof(buf)) {
char* buf2 = malloc(size + 1); char* buf2 = malloc(size + 1);
upb_JsonEncode(msg->msg, msg->msgdef, symtab, options, buf2, size + 1, upb_JsonEncode(msg->msg, msg->msgdef, pool, options, buf2, size + 1,
&status); &status);
ret = rb_str_new(buf2, size); ret = rb_str_new(buf2, size);
free(buf2); free(buf2);

@ -263,6 +263,30 @@ module BasicTestProto2
assert_equal 42, extension.get(message) assert_equal 42, extension.get(message)
end end
def test_extension_json
message = TestExtensions.decode_json '{"[basic_test_proto2.optional_int32_extension]": 123}'
extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.optional_int32_extension'
assert_instance_of Google::Protobuf::FieldDescriptor, extension
assert_equal 123, extension.get(message)
end
def test_extension_json_separate_pool
pool = Google::Protobuf::DescriptorPool.new
# This serialized descriptor is a subset of basic_test_proto2.proto that
# contains only the TestExtensions message, but no actual extensions.
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"
pool.add_serialized_file(descriptor_data)
message_class = pool.lookup("basic_test_proto2.TestExtensions").msgclass
extension = pool.lookup 'basic_test_proto2.different_optional_int32_extension'
message = message_class.decode_json '{"[basic_test_proto2.different_optional_int32_extension]": 123}'
assert_equal 123, extension.get(message)
message2 = message_class.decode_json(message_class.encode_json(message))
assert_equal 123, extension.get(message2)
end
def test_nested_extension def test_nested_extension
message = TestExtensions.new message = TestExtensions.new
extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestNestedExtension.test' extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestNestedExtension.test'

Loading…
Cancel
Save