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

1 files changed, 67 insertions, 24 deletions

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: