Reserve unknown fields in php (#3659)

* Reserve unknown fields in upb
1) For decoding, an unknownfields will be lazily created on message,
which contains bytes of unknown fields.
2) For encoding, if the unknownfields is present on message, all bytes
contained in it will be serialized.

* Register the function to encode unknown field at decode time.

* Remove upb_handlers_setaddunknown

* Use upb_sink_putunknown in decoder

* Remove upb_pb_encoder_encode_unknown

* Do not expose encode_unknown

* Implement reserve unknown field in php Implement.

* Make buffer private to CodedInputStream

php/ext/google/protobuf/encode_decode.c

@@ -142,6 +142,23 @@ static const void* newhandlerdata(upb_handlers* h, uint32_t ofs) {
   return hd_ofs;
 }
 
+typedef size_t (*encodeunknown_handlerfunc)(void* _sink, const void* hd,
+                                            const char* ptr, size_t len,
+                                            const upb_bufhandle* handle);
+
+typedef struct {
+  encodeunknown_handlerfunc handler;
+} unknownfields_handlerdata_t;
+
+// Creates a handlerdata for unknown fields.
+static const void *newunknownfieldshandlerdata(upb_handlers* h) {
+  unknownfields_handlerdata_t* hd =
+      (unknownfields_handlerdata_t*)malloc(sizeof(unknownfields_handlerdata_t));
+  hd->handler = stringsink_string;
+  upb_handlers_addcleanup(h, hd, free);
+  return hd;
+}
+
 typedef struct {
   size_t ofs;
   const upb_msgdef *md;
@@ -944,6 +961,24 @@ static void add_handlers_for_oneof_field(upb_handlers *h,
   upb_handlerattr_uninit(&attr);
 }
 
+static bool add_unknown_handler(void* closure, const void* hd, const char* buf,
+                         size_t size) {
+  encodeunknown_handlerfunc handler =
+      ((unknownfields_handlerdata_t*)hd)->handler;
+
+  MessageHeader* msg = (MessageHeader*)closure;
+  stringsink* unknown = DEREF(message_data(msg), 0, stringsink*);
+  if (unknown == NULL) {
+    DEREF(message_data(msg), 0, stringsink*) = ALLOC(stringsink);
+    unknown = DEREF(message_data(msg), 0, stringsink*);
+    stringsink_init(unknown);
+  }
+
+  handler(unknown, NULL, buf, size, NULL);
+
+  return true;
+}
+
 static void add_handlers_for_message(const void* closure,
                                      upb_handlers* h) {
   const upb_msgdef* msgdef = upb_handlers_msgdef(h);
@@ -967,6 +1002,10 @@ static void add_handlers_for_message(const void* closure,
     desc->layout = create_layout(desc->msgdef);
   }
 
+  upb_handlerattr attr = UPB_HANDLERATTR_INITIALIZER;
+  upb_handlerattr_sethandlerdata(&attr, newunknownfieldshandlerdata(h));
+  upb_handlers_setunknown(h, add_unknown_handler, &attr);
+
   for (upb_msg_field_begin(&i, desc->msgdef);
        !upb_msg_field_done(&i);
        upb_msg_field_next(&i)) {
@@ -1278,6 +1317,11 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc,
     }
   }
 
+  stringsink* unknown = DEREF(message_data(msg), 0, stringsink*);
+  if (unknown != NULL) {
+    upb_sink_putunknown(sink, unknown->ptr, unknown->len);
+  }
+
   upb_sink_endmsg(sink, &status);
 }
 

php/ext/google/protobuf/message.c

@@ -81,6 +81,10 @@ static HashTable* message_get_properties(zval* object TSRMLS_DC);
 
 // Define object free method.
 PHP_PROTO_OBJECT_FREE_START(MessageHeader, message)
+  if (*(void**)intern->data != NULL) {
+    stringsink_uninit(*(void**)intern->data);
+    FREE(*(void**)intern->data);
+  }
   FREE(intern->data);
 PHP_PROTO_OBJECT_FREE_END
 

php/ext/google/protobuf/storage.c

@@ -583,9 +583,12 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
   upb_msg_oneof_iter oit;
   size_t off = 0;
   int i = 0;
+
+  // Reserve space for unknown fields.
+  off += sizeof(void*);
+
   TSRMLS_FETCH();
   Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msgdef));
-
   layout->fields = ALLOC_N(MessageField, nfields);
 
   for (upb_msg_field_begin(&it, msgdef); !upb_msg_field_done(&it);
@@ -744,6 +747,10 @@ void layout_init(MessageLayout* layout, void* storage,
                  zend_object* object PHP_PROTO_TSRMLS_DC) {
   int i;
   upb_msg_field_iter it;
+
+  // Init unknown fields
+  memset(storage, 0, sizeof(void*));
+
   for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it);
        upb_msg_field_next(&it), i++) {
     const upb_fielddef* field = upb_msg_iter_field(&it);

php/src/Google/Protobuf/Internal/CodedInputStream.php

@@ -78,13 +78,18 @@ class CodedInputStream
         return $this->buffer_end - $this->current;
     }
 
-    private function current()
+    public function current()
     {
         return $this->total_bytes_read -
             ($this->buffer_end - $this->current +
             $this->buffer_size_after_limit);
     }
 
+    public function substr($start, $end)
+    {
+        return substr($this->buffer, $start, $end - $start);
+    }
+
     private function recomputeBufferLimits()
     {
         $this->buffer_end += $this->buffer_size_after_limit;

php/src/Google/Protobuf/Internal/CodedOutputStream.php

@@ -101,7 +101,7 @@ class CodedOutputStream
         return true;
     }
 
-    private static function writeVarintToArray($value, &$buffer, $trim = false)
+    public static function writeVarintToArray($value, &$buffer, $trim = false)
     {
         $current = 0;
 

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

@@ -57,6 +57,7 @@ class Message
      * @ignore
      */
     private $desc;
+    private $unknown = "";
 
     /**
      * @ignore
@@ -226,13 +227,14 @@ class Message
     /**
      * @ignore
      */
-    private static function skipField($input, $tag)
+    private function skipField($input, $tag)
     {
         $number = GPBWire::getTagFieldNumber($tag);
         if ($number === 0) {
             throw new GPBDecodeException("Illegal field number zero.");
         }
 
+        $start = $input->current();
         switch (GPBWire::getTagWireType($tag)) {
             case GPBWireType::VARINT:
                 $uint64 = 0;
@@ -240,21 +242,21 @@ class Message
                     throw new GPBDecodeException(
                         "Unexpected EOF inside varint.");
                 }
-                return;
+                break;
             case GPBWireType::FIXED64:
                 $uint64 = 0;
                 if (!$input->readLittleEndian64($uint64)) {
                     throw new GPBDecodeException(
                         "Unexpected EOF inside fixed64.");
                 }
-                return;
+                break;
             case GPBWireType::FIXED32:
                 $uint32 = 0;
                 if (!$input->readLittleEndian32($uint32)) {
                     throw new GPBDecodeException(
                         "Unexpected EOF inside fixed32.");
                 }
-                return;
+                break;
             case GPBWireType::LENGTH_DELIMITED:
                 $length = 0;
                 if (!$input->readVarint32($length)) {
@@ -266,13 +268,18 @@ class Message
                     throw new GPBDecodeException(
                         "Unexpected EOF inside length delimited data.");
                 }
-                return;
+                break;
             case GPBWireType::START_GROUP:
             case GPBWireType::END_GROUP:
                 throw new GPBDecodeException("Unexpected wire type.");
             default:
                 throw new GPBDecodeException("Unexpected wire type.");
         }
+        $end = $input->current();
+
+        $bytes = str_repeat(chr(0), CodedOutputStream::MAX_VARINT64_BYTES);
+        $size = CodedOutputStream::writeVarintToArray($tag, $bytes, true);
+        $this->unknown .= substr($bytes, 0, $size) . $input->substr($start, $end);
     }
 
     /**
@@ -423,7 +430,7 @@ class Message
         }
 
         if ($value_format === GPBWire::UNKNOWN) {
-            self::skipField($input, $tag);
+            $this->skipField($input, $tag);
             return;
         } elseif ($value_format === GPBWire::NORMAL_FORMAT) {
             self::parseFieldFromStreamNoTag($input, $field, $value);
@@ -461,6 +468,7 @@ class Message
      */
     public function clear()
     {
+        $this->unknown = "";
         foreach ($this->desc->getField() as $field) {
             $setter = $field->getSetter();
             if ($field->isMap()) {
@@ -1043,6 +1051,7 @@ class Message
                 return false;
             }
         }
+        $output->writeRaw($this->unknown, strlen($this->unknown));
         return true;
     }
 
@@ -1428,6 +1437,7 @@ class Message
         foreach ($fields as $field) {
             $size += $this->fieldByteSize($field);
         }
+        $size += strlen($this->unknown);
         return $size;
     }
 

php/tests/encode_decode_test.php

@@ -441,6 +441,33 @@ class EncodeDecodeTest extends TestBase
         $m->mergeFromString(hex2bin('D205'));
     }
 
+    public function testUnknown()
+    {
+        $m = new TestMessage();
+        $from = hex2bin('F80601');
+        $m->mergeFromString($from);
+        $to = $m->serializeToString();
+        $this->assertSame(bin2hex($from), bin2hex($to));
+
+        $m = new TestMessage();
+        $from = hex2bin('F9060000000000000000');
+        $m->mergeFromString($from);
+        $to = $m->serializeToString();
+        $this->assertSame(bin2hex($from), bin2hex($to));
+
+        $m = new TestMessage();
+        $from = hex2bin('FA0600');
+        $m->mergeFromString($from);
+        $to = $m->serializeToString();
+        $this->assertSame(bin2hex($from), bin2hex($to));
+
+        $m = new TestMessage();
+        $from = hex2bin('FD0600000000');
+        $m->mergeFromString($from);
+        $to = $m->serializeToString();
+        $this->assertSame(bin2hex($from), bin2hex($to));
+    }
+
     public function testJsonEncode()
     {
         $from = new TestMessage();

php/tests/gdb_test.sh

@@ -3,7 +3,7 @@
 # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which
 # phpunit` --bootstrap autoload.php tmp_test.php
 #
-gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php well_known_test.php
+gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php
 #
 # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php
 #

php/tests/memory_leak_test.php

@@ -101,6 +101,10 @@ $n = new TestMessage();
 $n->mergeFromString($data);
 assert(1 === $n->getOneofMessage()->getA());
 
+$m = new TestMessage();
+$m->mergeFromString(hex2bin('F80601'));
+assert('F80601', bin2hex($m->serializeToString()));
+
 # $from = new TestMessage();
 # $to = new TestMessage();
 # TestUtil::setTestMessage($from);

php/tests/proto/test.proto

@@ -115,6 +115,8 @@ message TestMessage {
   message Empty {
     int32 a = 1;
   }
+
+  reserved 111;
 }
 
 enum TestEnum {