From 7fa212bba15da2fd725860dcc2e511a89eadba89 Mon Sep 17 00:00:00 2001 From: Jason Lunn Date: Wed, 16 Mar 2022 13:37:46 -0400 Subject: [PATCH] Fix NPE during encoding and add regression test for issue 9507. (cherry picked from commit 58e320a7323b55f65137222064ed0e72fe423e23) --- .../google/protobuf/jruby/RubyMessage.java | 18 +++++++----- ruby/tests/basic.rb | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index f55eb9b285..0d76116097 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -62,6 +62,9 @@ import java.util.List; import java.util.Map; public class RubyMessage extends RubyObject { + private final String DEFAULT_VALUE = "google.protobuf.FieldDescriptorProto.default_value"; + private final String TYPE = "type"; + public RubyMessage(Ruby runtime, RubyClass klazz, Descriptor descriptor) { super(runtime, klazz); @@ -677,6 +680,8 @@ public class RubyMessage extends RubyObject { throw context.runtime.newRuntimeError("Recursion limit exceeded during encoding."); } + RubySymbol typeBytesSymbol = RubySymbol.newSymbol(context.runtime, "TYPE_BYTES"); + // Handle the typical case where the fields.keySet contain the fieldDescriptors for (FieldDescriptor fieldDescriptor : fields.keySet()) { IRubyObject value = fields.get(fieldDescriptor); @@ -707,13 +712,12 @@ public class RubyMessage extends RubyObject { * stringDefaultValue}. */ boolean isDefaultStringForBytes = false; - FieldDescriptor enumFieldDescriptorForType = - this.builder.getDescriptorForType().findFieldByName("type"); - String type = enumFieldDescriptorForType == null ? - null : fields.get(enumFieldDescriptorForType).toString(); - if (type != null && type.equals("TYPE_BYTES") && - fieldDescriptor.getFullName().equals("google.protobuf.FieldDescriptorProto.default_value")) { - isDefaultStringForBytes = true; + if (DEFAULT_VALUE.equals(fieldDescriptor.getFullName())) { + FieldDescriptor enumFieldDescriptorForType = + this.builder.getDescriptorForType().findFieldByName(TYPE); + if (typeBytesSymbol.equals(fields.get(enumFieldDescriptorForType))) { + isDefaultStringForBytes = true; + } } builder.setField(fieldDescriptor, convert(context, fieldDescriptor, value, depth, recursionLimit, isDefaultStringForBytes)); } diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index cdcd295085..4ff1c15583 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -79,6 +79,34 @@ module BasicTest assert_equal 8, msg.id end + def test_issue_9507 + pool = Google::Protobuf::DescriptorPool.new + pool.build do + add_message "NpeMessage" do + optional :type, :enum, 1, "TestEnum" + optional :other, :string, 2 + end + add_enum "TestEnum" do + value :Something, 0 + end + end + + msgclass = pool.lookup("NpeMessage").msgclass + + m = msgclass.new( + other: "foo" # must be set, but can be blank + ) + + begin + encoded = msgclass.encode(m) + rescue java.lang.NullPointerException => e + flunk "NPE rescued" + end + decoded = msgclass.decode(encoded) + decoded.inspect + decoded.to_proto + end + def test_has_field m = TestSingularFields.new assert !m.has_singular_msg?