[PHP] Fixed $msg->setMessage(null) to properly clear the message. (#8472 An error occurred )

* [PHP] Fixed $msg->setMessage(null) to properly clear the message.

Fixes: https://github.com/protocolbuffers/protobuf/issues/8457

* Changed pure PHP to throw TypeError, and added a test for null.

* Added more tests and fixed null in setter for oneof.

php/ext/google/protobuf/message.c

@@ -149,6 +149,9 @@ static bool Message_set(Message *intern, const upb_fielddef *f, zval *val) {
   } else if (upb_fielddef_isseq(f)) {
     msgval.array_val = RepeatedField_GetUpbArray(val, TypeInfo_Get(f), arena);
     if (!msgval.array_val) return false;
+  } else if (upb_fielddef_issubmsg(f) && Z_TYPE_P(val) == IS_NULL) {
+    upb_msg_clearfield(intern->msg, f);
+    return true;
   } else {
     if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(f), arena)) return false;
   }
@@ -454,11 +457,6 @@ bool Message_GetUpbMessage(zval *val, const Descriptor *desc, upb_arena *arena,
     ZVAL_DEREF(val);
   }
 
-  if (Z_TYPE_P(val) == IS_NULL) {
-    *msg = NULL;
-    return true;
-  }
-
   if (Z_TYPE_P(val) == IS_OBJECT &&
       instanceof_function(Z_OBJCE_P(val), desc->class_entry)) {
     Message *intern = (Message*)Z_OBJ_P(val);
@@ -466,7 +464,8 @@ bool Message_GetUpbMessage(zval *val, const Descriptor *desc, upb_arena *arena,
     *msg = intern->msg;
     return true;
   } else {
-    zend_throw_exception_ex(NULL, 0, "Given value is not an instance of %s.",
+    zend_throw_exception_ex(zend_ce_type_error, 0,
+                            "Given value is not an instance of %s.",
                             ZSTR_VAL(desc->class_entry->name));
     return false;
   }
@@ -1051,7 +1050,10 @@ PHP_METHOD(Message, writeOneof) {
 
   f = upb_msgdef_itof(intern->desc->msgdef, field_num);
 
-  if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(f), arena)) {
+  if (upb_fielddef_issubmsg(f) && Z_TYPE_P(val) == IS_NULL) {
+    upb_msg_clearfield(intern->msg, f);
+    return;
+  } else if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(f), arena)) {
     return;
   }
 

php/src/Google/Protobuf/Internal/Message.php

@@ -240,10 +240,14 @@ class Message
         $field = $this->desc->getFieldByNumber($number);
         $oneof = $this->desc->getOneofDecl()[$field->getOneofIndex()];
         $oneof_name = $oneof->getName();
-        $oneof_field = $this->$oneof_name;
-        $oneof_field->setValue($value);
-        $oneof_field->setFieldName($field->getName());
-        $oneof_field->setNumber($number);
+        if ($value === null) {
+            $this->$oneof_name = new OneofField($oneof);
+        } else {
+            $oneof_field = $this->$oneof_name;
+            $oneof_field->setValue($value);
+            $oneof_field->setFieldName($field->getName());
+            $oneof_field->setNumber($number);
+        }
     }
 
     protected function whichOneof($oneof_name)

php/src/Google/Protobuf/Internal/RepeatedField.php

@@ -177,8 +177,7 @@ class RepeatedField implements \ArrayAccess, \IteratorAggregate, \Countable
                 break;
             case GPBType::MESSAGE:
                 if (is_null($value)) {
-                  trigger_error("RepeatedField element cannot be null.",
-                                E_USER_ERROR);
+                    throw new \TypeError("RepeatedField element cannot be null.");
                 }
                 GPBUtil::checkMessage($value, $this->klass);
                 break;

php/tests/ArrayTest.php

@@ -602,6 +602,17 @@ class ArrayTest extends TestBase
         $this->assertLessThan($start, $end);
     }
 
+    #########################################################
+    # Test incorrect types
+    #########################################################
+
+    public function testAppendNull()
+    {
+        $this->expectException(TypeError::class);
+        $arr = new RepeatedField(GPBType::MESSAGE, TestMessage::class);
+        $arr[] = null;
+    }
+
     #########################################################
     # Test equality
     #########################################################

php/tests/EncodeDecodeTest.php

@@ -940,6 +940,14 @@ class EncodeDecodeTest extends TestBase
         $this->expectFields($to);
     }
 
+    public function testJsonEncodeNullSubMessage()
+    {
+        $from = new TestMessage();
+        $from->setOptionalMessage(null);
+        $data = $from->serializeToJsonString();
+        $this->assertEquals("{}", $data);
+    }
+
     public function testDecodeDuration()
     {
         $m = new Google\Protobuf\Duration();

php/tests/GeneratedClassTest.php

@@ -476,10 +476,12 @@ class GeneratedClassTest extends TestBase
         $sub_m->setA(1);
         $m->setOptionalMessage($sub_m);
         $this->assertSame(1, $m->getOptionalMessage()->getA());
+        $this->assertTrue($m->hasOptionalMessage());
 
         $null = null;
         $m->setOptionalMessage($null);
         $this->assertNull($m->getOptionalMessage());
+        $this->assertFalse($m->hasOptionalMessage());
     }
 
     public function testLegacyMessageField()
@@ -1748,6 +1750,13 @@ class GeneratedClassTest extends TestBase
         $m->clear();
         $this->assertFalse($m->hasOneofInt32());
         $this->assertFalse($m->hasOneofString());
+
+        $sub_m = new Sub();
+        $sub_m->setA(1);
+        $m->setOneofMessage($sub_m);
+        $this->assertTrue($m->hasOneofMessage());
+        $m->setOneofMessage(null);
+        $this->assertFalse($m->hasOneofMessage());
     }
 
     #########################################################