Recursively clear unknown fields in submessages. (#3982)

* Recursively clear unknown fields in submessages.

* Recursively discard unknown fields in submsg for c extension

* Fix zts build

* Add comment for tests

* Add a TODO to add a util for encoding varint for better readability.

* Add test for oneof message field.

3 files changed, 159 insertions, 4 deletions

diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c
index ae5a61f5..b98121bb 100644
--- a/php/ext/google/protobuf/encode_decode.c
+++ b/php/ext/google/protobuf/encode_decode.c
@@ -1402,7 +1402,6 @@ static void putarray(zval* array, const upb_fielddef* f, upb_sink* sink,
   RepeatedField* intern = UNBOX(RepeatedField, array);
   HashTable *ht = PHP_PROTO_HASH_OF(intern->array);
   size = zend_hash_num_elements(ht);
-  // size = zend_hash_num_elements(PHP_PROTO_HASH_OF(intern->array));
   if (size == 0) return;
 
   upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink);
@@ -1615,11 +1614,101 @@ PHP_METHOD(Message, mergeFromJsonString) {
   }
 }
 
-PHP_METHOD(Message, discardUnknownFields) {
-  MessageHeader* msg = UNBOX(MessageHeader, getThis());
+// TODO(teboring): refactoring with putrawmsg
+static void discard_unknown_fields(MessageHeader* msg) {
+  upb_msg_field_iter it;
+
   stringsink* unknown = DEREF(message_data(msg), 0, stringsink*);
   if (unknown != NULL) {
     stringsink_uninit(unknown);
     DEREF(message_data(msg), 0, stringsink*) = NULL;
   }
+
+  // Recursively discard unknown fields of submessages.
+  Descriptor* desc = msg->descriptor;
+  TSRMLS_FETCH();
+  for (upb_msg_field_begin(&it, desc->msgdef);
+       !upb_msg_field_done(&it);
+       upb_msg_field_next(&it)) {
+    upb_fielddef* f = upb_msg_iter_field(&it);
+    uint32_t offset = desc->layout->fields[upb_fielddef_index(f)].offset;
+    bool containing_oneof = false;
+
+    if (upb_fielddef_containingoneof(f)) {
+      uint32_t oneof_case_offset =
+          desc->layout->fields[upb_fielddef_index(f)].case_offset;
+      // For a oneof, check that this field is actually present -- skip all the
+      // below if not.
+      if (DEREF(message_data(msg), oneof_case_offset, uint32_t) !=
+          upb_fielddef_number(f)) {
+        continue;
+      }
+      // Otherwise, fall through to the appropriate singular-field handler
+      // below.
+      containing_oneof = true;
+    }
+
+    if (is_map_field(f)) {
+      MapIter map_it;
+      int len, size;
+      const upb_fielddef* value_field;
+
+      value_field = map_field_value(f);
+      if (!upb_fielddef_issubmsg(value_field)) continue;
+
+      zval* map_php = CACHED_PTR_TO_ZVAL_PTR(
+          DEREF(message_data(msg), offset, CACHED_VALUE*));
+      if (map_php == NULL) continue;
+
+      Map* intern = UNBOX(Map, map_php);
+      for (map_begin(map_php, &map_it TSRMLS_CC);
+           !map_done(&map_it); map_next(&map_it)) {
+        upb_value value = map_iter_value(&map_it, &len);
+        void* memory = raw_value(upb_value_memory(&value), value_field);
+#if PHP_MAJOR_VERSION < 7
+        MessageHeader *submsg = UNBOX(MessageHeader, *(zval**)memory);
+#else
+        MessageHeader *submsg =
+            (MessageHeader*)((char*)(Z_OBJ_P((zval*)memory)) -
+                XtOffsetOf(MessageHeader, std));
+#endif
+        discard_unknown_fields(submsg);
+      }
+    } else if (upb_fielddef_isseq(f)) {
+      if (!upb_fielddef_issubmsg(f)) continue;
+
+      zval* array_php = CACHED_PTR_TO_ZVAL_PTR(
+          DEREF(message_data(msg), offset, CACHED_VALUE*));
+      if (array_php == NULL) continue;
+
+      int size, i;
+      RepeatedField* intern = UNBOX(RepeatedField, array_php);
+      HashTable *ht = PHP_PROTO_HASH_OF(intern->array);
+      size = zend_hash_num_elements(ht);
+      if (size == 0) continue;
+
+      for (i = 0; i < size; i++) {
+        void* memory = repeated_field_index_native(intern, i TSRMLS_CC);
+#if PHP_MAJOR_VERSION < 7
+        MessageHeader *submsg = UNBOX(MessageHeader, *(zval**)memory);
+#else
+        MessageHeader *submsg =
+            (MessageHeader*)((char*)(Z_OBJ_P((zval*)memory)) -
+                XtOffsetOf(MessageHeader, std));
+#endif
+        discard_unknown_fields(submsg);
+      }
+    } else if (upb_fielddef_issubmsg(f)) {
+      zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR(
+          DEREF(message_data(msg), offset, CACHED_VALUE*));
+      if (Z_TYPE_P(submsg_php) == IS_NULL) continue;
+      MessageHeader* submsg = UNBOX(MessageHeader, submsg_php);
+      discard_unknown_fields(submsg);
+    }
+  }
+}
+
+PHP_METHOD(Message, discardUnknownFields) {
+  MessageHeader* msg = UNBOX(MessageHeader, getThis());
+  discard_unknown_fields(msg);
 }
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index 26d20575..93e81c01 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -583,6 +583,34 @@ class Message
     public function discardUnknownFields()
     {
         $this->unknown = "";
+        foreach ($this->desc->getField() as $field) {
+            if ($field->getType() != GPBType::MESSAGE) {
+                continue;
+            }
+            if ($field->isMap()) {
+                $value_field = $field->getMessageType()->getFieldByNumber(2);
+                if ($value_field->getType() != GPBType::MESSAGE) {
+                    continue;
+                }
+                $getter = $field->getGetter();
+                $map = $this->$getter();
+                foreach ($map as $key => $value) {
+                    $value->discardUnknownFields();
+                }
+            } else if ($field->getLabel() === GPBLabel::REPEATED) {
+                $getter = $field->getGetter();
+                $arr = $this->$getter();
+                foreach ($arr as $sub) {
+                    $sub->discardUnknownFields();
+                }
+            } else if ($field->getLabel() === GPBLabel::OPTIONAL) {
+                $getter = $field->getGetter();
+                $sub = $this->$getter();
+                if (!is_null($sub)) {
+                    $sub->discardUnknownFields();
+                }
+            }
+        }
     }
 
     /**
diff --git a/php/tests/encode_decode_test.php b/php/tests/encode_decode_test.php
index 4512c871..74d5526d 100644
--- a/php/tests/encode_decode_test.php
+++ b/php/tests/encode_decode_test.php
@@ -443,36 +443,74 @@ class EncodeDecodeTest extends TestBase
 
     public function testUnknown()
     {
+        // Test preserve unknown for varint.
         $m = new TestMessage();
-        $from = hex2bin('F80601');
+        $from = hex2bin('F80601');  // TODO(teboring): Add a util to encode
+                                    // varint for better readability
         $m->mergeFromString($from);
         $to = $m->serializeToString();
         $this->assertSame(bin2hex($from), bin2hex($to));
 
+        // Test preserve unknown for 64-bit.
         $m = new TestMessage();
         $from = hex2bin('F9060000000000000000');
         $m->mergeFromString($from);
         $to = $m->serializeToString();
         $this->assertSame(bin2hex($from), bin2hex($to));
 
+        // Test preserve unknown for length delimited.
         $m = new TestMessage();
         $from = hex2bin('FA0600');
         $m->mergeFromString($from);
         $to = $m->serializeToString();
         $this->assertSame(bin2hex($from), bin2hex($to));
 
+        // Test preserve unknown for 32-bit.
         $m = new TestMessage();
         $from = hex2bin('FD0600000000');
         $m->mergeFromString($from);
         $to = $m->serializeToString();
         $this->assertSame(bin2hex($from), bin2hex($to));
 
+        // Test discard unknown in message.
         $m = new TestMessage();
         $from = hex2bin('F80601');
         $m->mergeFromString($from);
         $m->discardUnknownFields();
         $to = $m->serializeToString();
         $this->assertSame("", bin2hex($to));
+
+        // Test discard unknown for singular message field.
+        $m = new TestMessage();
+        $from = hex2bin('8A0103F80601');
+        $m->mergeFromString($from);
+        $m->discardUnknownFields();
+        $to = $m->serializeToString();
+        $this->assertSame("8a0100", bin2hex($to));
+
+        // Test discard unknown for repeated message field.
+        $m = new TestMessage();
+        $from = hex2bin('FA0203F80601');
+        $m->mergeFromString($from);
+        $m->discardUnknownFields();
+        $to = $m->serializeToString();
+        $this->assertSame("fa0200", bin2hex($to));
+
+        // Test discard unknown for map message value field.
+        $m = new TestMessage();
+        $from = hex2bin("BA050708011203F80601");
+        $m->mergeFromString($from);
+        $m->discardUnknownFields();
+        $to = $m->serializeToString();
+        $this->assertSame("ba050408011200", bin2hex($to));
+
+        // Test discard unknown for singular message field.
+        $m = new TestMessage();
+        $from = hex2bin('9A0403F80601');
+        $m->mergeFromString($from);
+        $m->discardUnknownFields();
+        $to = $m->serializeToString();
+        $this->assertSame("9a0400", bin2hex($to));
     }
 
     public function testJsonEncode()