Fix the bug in php c extension that setting one field can change another field's value. (#3455)

* Fix the bug in php c extension that setting one field can change another
field's value.

The reason is that previously, in c extension, it was assumed that the
order that fields were declared in php is the same as the order of
fields in upb. This is not true. Now, for every field in upb, we will
look up the actual property that is corresponding to the upb field.

* Cleanup pull request

* Fix indentation

* Port to php5

* Port with php7.1

* Port to zts

4 files changed, 76 insertions, 33 deletions

diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c
index 4cb8997d..89e75d6a 100644
--- a/php/ext/google/protobuf/encode_decode.c
+++ b/php/ext/google/protobuf/encode_decode.c
@@ -716,7 +716,7 @@ static void *oneofbytes_handler(void *closure,
   DEREF(message_data(msg), oneofdata->case_ofs, uint32_t) =
       oneofdata->oneof_case_num;
   DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*) =
-      &(msg->std.properties_table)[oneofdata->property_ofs];
+      OBJ_PROP(&msg->std, oneofdata->property_ofs);
 
    return empty_php_string(DEREF(
        message_data(msg), oneofdata->ofs, CACHED_VALUE*));
@@ -747,7 +747,7 @@ static void* oneofsubmsg_handler(void* closure, const void* hd) {
 
     // Create new message.
     DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*) =
-        &(msg->std.properties_table)[oneofdata->property_ofs];
+        OBJ_PROP(&msg->std, oneofdata->property_ofs);
     ZVAL_OBJ(CACHED_PTR_TO_ZVAL_PTR(
         DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*)),
         subklass->create_object(subklass TSRMLS_CC));
diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
index 254640c7..519786dd 100644
--- a/php/ext/google/protobuf/message.c
+++ b/php/ext/google/protobuf/message.c
@@ -172,7 +172,7 @@ static zval* message_get_property(zval* object, zval* member, int type,
       zend_get_property_info(Z_OBJCE_P(object), member, true TSRMLS_CC);
   return layout_get(
       self->descriptor->layout, message_data(self), field,
-      &Z_OBJ_P(object)->properties_table[property_info->offset] TSRMLS_CC);
+      OBJ_PROP(Z_OBJ_P(object), property_info->offset) TSRMLS_CC);
 #else
   property_info =
       zend_get_property_info(Z_OBJCE_P(object), Z_STR_P(member), true);
@@ -222,7 +222,7 @@ void custom_data_init(const zend_class_entry* ce,
   // case a collection happens during object creation in layout_init().
   intern->descriptor = desc;
   layout_init(desc->layout, message_data(intern),
-              intern->std.properties_table PHP_PROTO_TSRMLS_CC);
+              &intern->std PHP_PROTO_TSRMLS_CC);
 }
 
 void build_class_from_descriptor(
@@ -265,8 +265,7 @@ PHP_METHOD(Message, clear) {
   zend_class_entry* ce = desc->klass;
 
   object_properties_init(&msg->std, ce);
-  layout_init(desc->layout, message_data(msg),
-              msg->std.properties_table TSRMLS_CC);
+  layout_init(desc->layout, message_data(msg), &msg->std TSRMLS_CC);
 }
 
 PHP_METHOD(Message, mergeFrom) {
@@ -301,7 +300,8 @@ PHP_METHOD(Message, readOneof) {
 
   int property_cache_index =
       msg->descriptor->layout->fields[upb_fielddef_index(field)].cache_index;
-  zval* property_ptr = OBJ_PROP(Z_OBJ_P(getThis()), property_cache_index);
+  zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR(
+      OBJ_PROP(Z_OBJ_P(getThis()), property_cache_index));
 
   // Unlike singular fields, oneof fields share cached property. So we cannot
   // let lay_get modify the cached property. Instead, we pass in the return
diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h
index 44a4155f..f9e9d229 100644
--- a/php/ext/google/protobuf/protobuf.h
+++ b/php/ext/google/protobuf/protobuf.h
@@ -151,7 +151,7 @@
 
 #define PHP_PROTO_GLOBAL_UNINITIALIZED_ZVAL EG(uninitialized_zval_ptr)
 
-#define OBJ_PROP(PROPERTIES, OFFSET) (PROPERTIES)->properties_table[OFFSET]
+#define OBJ_PROP(OBJECT, OFFSET) &((OBJECT)->properties_table[OFFSET])
 
 #define php_proto_zval_ptr_dtor(zval_ptr) \
   zval_ptr_dtor(&(zval_ptr))
@@ -644,7 +644,7 @@ PHP_PROTO_WRAP_OBJECT_END
 
 MessageLayout* create_layout(const upb_msgdef* msgdef);
 void layout_init(MessageLayout* layout, void* storage,
-                 CACHED_VALUE* properties_table PHP_PROTO_TSRMLS_DC);
+                 zend_object* object PHP_PROTO_TSRMLS_DC);
 zval* layout_get(MessageLayout* layout, const void* storage,
                  const upb_fielddef* field, CACHED_VALUE* cache TSRMLS_DC);
 void layout_set(MessageLayout* layout, MessageHeader* header,
diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c
index e46dbf70..4830e15f 100644
--- a/php/ext/google/protobuf/storage.c
+++ b/php/ext/google/protobuf/storage.c
@@ -275,9 +275,6 @@ void native_slot_init(upb_fieldtype_t type, void* memory, CACHED_VALUE* cache) {
       break;
     case UPB_TYPE_STRING:
     case UPB_TYPE_BYTES:
-      DEREF(memory, CACHED_VALUE*) = cache;
-      ZVAL_EMPTY_STRING(CACHED_PTR_TO_ZVAL_PTR(cache));
-      break;
     case UPB_TYPE_MESSAGE:
       DEREF(memory, CACHED_VALUE*) = cache;
       break;
@@ -586,6 +583,8 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
   upb_msg_oneof_iter oit;
   size_t off = 0;
   int i = 0;
+  TSRMLS_FETCH();
+  Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msgdef));
 
   layout->fields = ALLOC_N(MessageField, nfields);
 
@@ -612,7 +611,37 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
     layout->fields[upb_fielddef_index(field)].offset = off;
     layout->fields[upb_fielddef_index(field)].case_offset =
         MESSAGE_FIELD_NO_CASE;
-    layout->fields[upb_fielddef_index(field)].cache_index = i++;
+
+    const char* fieldname = upb_fielddef_name(field);
+
+#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0)
+    zend_class_entry* old_scope = EG(scope);
+    EG(scope) = desc->klass;
+#else
+    zend_class_entry* old_scope = EG(fake_scope);
+    EG(fake_scope) = desc->klass;
+#endif
+
+#if PHP_MAJOR_VERSION < 7
+    zval member;
+    ZVAL_STRINGL(&member, fieldname, strlen(fieldname), 0);
+    zend_property_info* property_info =
+        zend_get_property_info(desc->klass, &member, true TSRMLS_CC);
+#else
+    zend_string* member = zend_string_init(fieldname, strlen(fieldname), 1);
+    zend_property_info* property_info =
+        zend_get_property_info(desc->klass, member, true);
+    zend_string_release(member);
+#endif
+
+#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0)
+    EG(scope) = old_scope;
+#else
+    EG(fake_scope) = old_scope;
+#endif
+
+    layout->fields[upb_fielddef_index(field)].cache_index =
+        property_info->offset;
     off += field_size;
   }
 
@@ -640,11 +669,40 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) {
     // Align the offset .
     off = align_up_to( off, field_size);
     // Assign all fields in the oneof this same offset.
+    const char* oneofname = upb_oneofdef_name(oneof);
     for (upb_oneof_begin(&fit, oneof); !upb_oneof_done(&fit);
          upb_oneof_next(&fit)) {
       const upb_fielddef* field = upb_oneof_iter_field(&fit);
       layout->fields[upb_fielddef_index(field)].offset = off;
-      layout->fields[upb_fielddef_index(field)].cache_index = i;
+
+#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0)
+      zend_class_entry* old_scope = EG(scope);
+      EG(scope) = desc->klass;
+#else
+      zend_class_entry* old_scope = EG(fake_scope);
+      EG(fake_scope) = desc->klass;
+#endif
+
+#if PHP_MAJOR_VERSION < 7
+      zval member;
+      ZVAL_STRINGL(&member, oneofname, strlen(oneofname), 0);
+      zend_property_info* property_info =
+          zend_get_property_info(desc->klass, &member, true TSRMLS_CC);
+#else
+      zend_string* member = zend_string_init(oneofname, strlen(oneofname), 1);
+      zend_property_info* property_info =
+          zend_get_property_info(desc->klass, member, true);
+      zend_string_release(member);
+#endif
+
+#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0)
+      EG(scope) = old_scope;
+#else
+      EG(fake_scope) = old_scope;
+#endif
+
+      layout->fields[upb_fielddef_index(field)].cache_index =
+          property_info->offset;
     }
     i++;
     off += field_size;
@@ -683,7 +741,7 @@ void free_layout(MessageLayout* layout) {
 }
 
 void layout_init(MessageLayout* layout, void* storage,
-                 CACHED_VALUE* properties_table PHP_PROTO_TSRMLS_DC) {
+                 zend_object* object PHP_PROTO_TSRMLS_DC) {
   int i;
   upb_msg_field_iter it;
   for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it);
@@ -692,22 +750,7 @@ void layout_init(MessageLayout* layout, void* storage,
     void* memory = slot_memory(layout, storage, field);
     uint32_t* oneof_case = slot_oneof_case(layout, storage, field);
     int cache_index = slot_property_cache(layout, storage, field);
-    CACHED_VALUE* property_ptr = &properties_table[cache_index];
-
-    // Clean up initial value by generated code. In the generated code of
-    // previous versions, each php field is given an initial value. However, the
-    // order to initialize these fields may not be consistent with the order of
-    // upb fields.
-    if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(property_ptr)) == IS_STRING) {
-#if PHP_MAJOR_VERSION < 7
-      if (!IS_INTERNED(Z_STRVAL_PP(property_ptr))) {
-        FREE(Z_STRVAL_PP(property_ptr));
-      }
-#else
-      zend_string_release(Z_STR_P(property_ptr));
-#endif
-    }
-    ZVAL_NULL(CACHED_PTR_TO_ZVAL_PTR(property_ptr));
+    CACHED_VALUE* property_ptr = OBJ_PROP(object, cache_index);
 
     if (upb_fielddef_containingoneof(field)) {
       memset(memory, 0, NATIVE_SLOT_MAX_SIZE);
@@ -797,7 +840,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header,
             header->descriptor->layout->fields[upb_fielddef_index(field)]
                 .cache_index;
         DEREF(memory, CACHED_VALUE*) =
-            &(header->std.properties_table)[property_cache_index];
+            OBJ_PROP(&header->std, property_cache_index);
         memory = DEREF(memory, CACHED_VALUE*);
         break;
       }
@@ -964,7 +1007,7 @@ void layout_merge(MessageLayout* layout, MessageHeader* from,
           int property_cache_index =
               layout->fields[upb_fielddef_index(field)].cache_index;
           DEREF(to_memory, CACHED_VALUE*) =
-              &(to->std.properties_table)[property_cache_index];
+              OBJ_PROP(&to->std, property_cache_index);
           break;
         }
         default: