Merge pull request #8363 from haberman/ruby-nil-fix

Fixed SEGV when users pass nil messages. This also disallows `nil` in RepeatedField or Map.

ruby/compatibility_tests/v3.0.0/tests/repeated_field_test.rb

@@ -326,18 +326,6 @@ class RepeatedFieldTest < Test::Unit::TestCase
     end
   end
 
-  def test_compact!
-    m = TestMessage.new
-    m.repeated_msg << TestMessage2.new(:foo => 1)
-    m.repeated_msg << nil
-    m.repeated_msg << TestMessage2.new(:foo => 2)
-    reference_arr = m.repeated_string.to_a
-
-    check_self_modifying_method(m.repeated_string, reference_arr) do |arr|
-      arr.compact!
-    end
-  end
-
   def test_delete
     m = TestMessage.new
     reference_arr = %w(foo bar baz)

ruby/ext/google/protobuf_c/message.c

@@ -1250,7 +1250,9 @@ upb_msg* Message_deep_copy(const upb_msg* msg, const upb_msgdef* m,
 
 const upb_msg* Message_GetUpbMessage(VALUE value, const upb_msgdef* m,
                                      const char* name, upb_arena* arena) {
-  if (value == Qnil) return NULL;
+  if (value == Qnil) {
+    rb_raise(cTypeError, "nil message not allowed here.");
+  }
 
   VALUE klass = CLASS_OF(value);
   VALUE desc_rb = rb_ivar_get(klass, descriptor_instancevar_interned);

ruby/tests/basic.rb

@@ -52,10 +52,15 @@ module BasicTest
 
       outer = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("Outer").msgclass
 
-      outer_proto = outer.new(
+      outer.new(
           inners: []
-      )
-      outer_proto['inners'].to_s
+      )['inners'].to_s
+
+      assert_raise Google::Protobuf::TypeError do
+        outer.new(
+            inners: [nil]
+        ).to_s
+      end
     end
 
     def test_has_field

ruby/tests/repeated_field_test.rb

@@ -339,18 +339,6 @@ class RepeatedFieldTest < Test::Unit::TestCase
     end
   end
 
-  def test_compact!
-    m = TestMessage.new
-    m.repeated_msg << TestMessage2.new(:foo => 1)
-    m.repeated_msg << nil
-    m.repeated_msg << TestMessage2.new(:foo => 2)
-    reference_arr = m.repeated_string.to_a
-
-    check_self_modifying_method(m.repeated_string, reference_arr) do |arr|
-      arr.compact!
-    end
-  end
-
   def test_delete
     m = TestMessage.new
     reference_arr = %w(foo bar baz)