[PHP] Fixed $msg->setMessage(null) to properly clear the message. (#8472)
* [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.
diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
index 2d9f9b4..6781349 100644
--- a/php/ext/google/protobuf/message.c
+++ b/php/ext/google/protobuf/message.c
@@ -149,6 +149,9 @@
} 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 @@
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 @@
*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 @@
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;
}
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index e74943c..19b48f0 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -240,10 +240,14 @@
$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)
diff --git a/php/src/Google/Protobuf/Internal/RepeatedField.php b/php/src/Google/Protobuf/Internal/RepeatedField.php
index 350bbb5..c0331ff 100644
--- a/php/src/Google/Protobuf/Internal/RepeatedField.php
+++ b/php/src/Google/Protobuf/Internal/RepeatedField.php
@@ -177,8 +177,7 @@
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;
diff --git a/php/tests/ArrayTest.php b/php/tests/ArrayTest.php
index b687085..9e8fcb8 100644
--- a/php/tests/ArrayTest.php
+++ b/php/tests/ArrayTest.php
@@ -603,6 +603,17 @@
}
#########################################################
+ # Test incorrect types
+ #########################################################
+
+ public function testAppendNull()
+ {
+ $this->expectException(TypeError::class);
+ $arr = new RepeatedField(GPBType::MESSAGE, TestMessage::class);
+ $arr[] = null;
+ }
+
+ #########################################################
# Test equality
#########################################################
diff --git a/php/tests/EncodeDecodeTest.php b/php/tests/EncodeDecodeTest.php
index 273010e..ac01ca1 100644
--- a/php/tests/EncodeDecodeTest.php
+++ b/php/tests/EncodeDecodeTest.php
@@ -940,6 +940,14 @@
$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();
diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php
index 5c0f0c7..9e176e8 100644
--- a/php/tests/GeneratedClassTest.php
+++ b/php/tests/GeneratedClassTest.php
@@ -476,10 +476,12 @@
$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 @@
$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());
}
#########################################################
