From c6c69b33718df666c6142d2ffe23223d54a134c0 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 4 Oct 2018 13:58:35 +0200 Subject: [PATCH 1/3] Introduce get_properties_for() handler This handler allows getting the object properties for a particular purpose, such as array casting, serialization, etc. --- Zend/zend.c | 9 ++---- Zend/zend_object_handlers.c | 40 ++++++++++++++++++++++++++ Zend/zend_object_handlers.h | 36 ++++++++++++++++++++++++ Zend/zend_operators.c | 36 ++++++++---------------- Zend/zend_types.h | 3 -- Zend/zend_vm_def.h | 14 ++++------ Zend/zend_vm_execute.h | 56 +++++++++++++------------------------ ext/json/json_encoder.c | 9 ++++-- ext/standard/var.c | 33 ++++++++++------------ sapi/phpdbg/phpdbg_utils.c | 10 ++----- 10 files changed, 140 insertions(+), 106 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index ee88afed02e60..6836e5af0c934 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -438,7 +438,6 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /* case IS_OBJECT: { HashTable *properties; - int is_temp; zend_string *class_name = Z_OBJ_HANDLER_P(expr, get_class_name)(Z_OBJ_P(expr)); smart_str_appends(buf, ZSTR_VAL(class_name)); @@ -449,7 +448,8 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /* smart_str_appends(buf, " *RECURSION*"); return; } - if ((properties = Z_OBJDEBUG_P(expr, is_temp)) == NULL) { + + if ((properties = zend_get_properties_for(expr, ZEND_PROP_PURPOSE_DEBUG)) == NULL) { break; } @@ -457,10 +457,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /* print_hash(buf, properties, indent, 1); Z_UNPROTECT_RECURSION_P(expr); - if (is_temp) { - zend_hash_destroy(properties); - FREE_HASHTABLE(properties); - } + zend_release_properties(properties); break; } case IS_LONG: diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 96395a755421f..5720b9455f0ae 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1737,6 +1737,45 @@ ZEND_API int zend_std_get_closure(zval *obj, zend_class_entry **ce_ptr, zend_fun } /* }}} */ +ZEND_API HashTable *zend_std_get_properties_for(zval *obj, zend_prop_purpose purpose) { + HashTable *ht; + switch (purpose) { + case ZEND_PROP_PURPOSE_DEBUG: + if (Z_OBJ_HT_P(obj)->get_debug_info) { + int is_temp; + ht = Z_OBJ_HT_P(obj)->get_debug_info(obj, &is_temp); + if (ht && !is_temp && !(GC_FLAGS(ht) & GC_IMMUTABLE)) { + GC_ADDREF(ht); + } + return ht; + } + /* break missing intentionally */ + case ZEND_PROP_PURPOSE_ARRAY_CAST: + case ZEND_PROP_PURPOSE_SERIALIZE: + case ZEND_PROP_PURPOSE_VAR_EXPORT: + case ZEND_PROP_PURPOSE_JSON: + ht = Z_OBJ_HT_P(obj)->get_properties(obj); + if (ht && !(GC_FLAGS(ht) & GC_IMMUTABLE)) { + GC_ADDREF(ht); + } + return ht; + default: + return NULL; + } +} + +ZEND_API HashTable *zend_get_properties_for(zval *obj, zend_prop_purpose purpose) { + HashTable *ht; + if (Z_OBJ_HT_P(obj)->get_properties_for) { + ht = Z_OBJ_HT_P(obj)->get_properties_for(obj, purpose); + if (ht) { + return ht; + } + } + + return zend_std_get_properties_for(obj, purpose); +} + ZEND_API const zend_object_handlers std_object_handlers = { 0, /* offset */ @@ -1768,6 +1807,7 @@ ZEND_API const zend_object_handlers std_object_handlers = { zend_std_get_gc, /* get_gc */ NULL, /* do_operation */ NULL, /* compare */ + NULL, /* get_properties_for */ }; /* diff --git a/Zend/zend_object_handlers.h b/Zend/zend_object_handlers.h index 094489a80ccd2..82606b8615cdf 100644 --- a/Zend/zend_object_handlers.h +++ b/Zend/zend_object_handlers.h @@ -93,6 +93,27 @@ typedef HashTable *(*zend_object_get_properties_t)(zval *object); typedef HashTable *(*zend_object_get_debug_info_t)(zval *object, int *is_temp); +typedef enum _zend_prop_purpose { + /* Used for debugging. Supersedes get_debug_info handler. */ + ZEND_PROP_PURPOSE_DEBUG, + /* Used for (array) casts. */ + ZEND_PROP_PURPOSE_ARRAY_CAST, + /* Used for serialization using the "O" scheme. + * Unserialization will use __wakeup(). */ + ZEND_PROP_PURPOSE_SERIALIZE, + /* Used for var_export(). + * The data will be passed to __set_state() when evaluated. */ + ZEND_PROP_PURPOSE_VAR_EXPORT, + /* Used for json_encode(). */ + ZEND_PROP_PURPOSE_JSON, + /* Dummy member to ensure that "default" is specified. */ + _ZEND_PROP_PURPOSE_NON_EXHAUSTIVE_ENUM +} zend_prop_purpose; + +/* The return value may be NULL. A non-NULL return value must be released using + * zend_release_properties(). Releasing NULL is also safe. */ +typedef zend_array *(*zend_object_get_properties_for_t)(zval *object, zend_prop_purpose purpose); + /* Used to call methods */ /* args on stack! */ /* Andi - EX(fbc) (function being called) needs to be initialized already in the INIT fcall opcode so that the parameters can be parsed the right way. We need to add another callback for this. @@ -160,6 +181,7 @@ struct _zend_object_handlers { zend_object_get_gc_t get_gc; zend_object_do_operation_t do_operation; zend_object_compare_zvals_t compare; + zend_object_get_properties_for_t get_properties_for; /* optional */ }; BEGIN_EXTERN_C() @@ -207,6 +229,20 @@ ZEND_API zend_function *zend_get_call_trampoline_func(zend_class_entry *ce, zend ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member); +/* Default behavior for get_properties_for. For use as a fallback in custom + * get_properties_for implementations. */ +ZEND_API HashTable *zend_std_get_properties_for(zval *obj, zend_prop_purpose purpose); + +/* Will call get_properties_for handler or use default behavior. For use by + * consumers of the get_properties_for API. */ +ZEND_API HashTable *zend_get_properties_for(zval *obj, zend_prop_purpose purpose); + +#define zend_release_properties(ht) do { \ + if ((ht) && !(GC_FLAGS(ht) & GC_IMMUTABLE) && !GC_DELREF(ht)) { \ + zend_array_destroy(ht); \ + } \ +} while (0) + #define zend_free_trampoline(func) do { \ if ((func) == &EG(trampoline)) { \ EG(trampoline).common.function_name = NULL; \ diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 71663ae31cd18..79ccffab5f76f 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -610,32 +610,20 @@ ZEND_API void ZEND_FASTCALL convert_to_array(zval *op) /* {{{ */ if (Z_OBJCE_P(op) == zend_ce_closure) { convert_scalar_to_array(op); } else { - if (Z_OBJ_HT_P(op)->get_properties) { - HashTable *obj_ht = Z_OBJ_HT_P(op)->get_properties(op); - if (obj_ht) { - /* fast copy */ - obj_ht = zend_proptable_to_symtable(obj_ht, - (Z_OBJCE_P(op)->default_properties_count || - Z_OBJ_P(op)->handlers != &std_object_handlers || - GC_IS_RECURSIVE(obj_ht))); - zval_ptr_dtor(op); - ZVAL_ARR(op, obj_ht); - return; - } + HashTable *obj_ht = zend_get_properties_for(op, ZEND_PROP_PURPOSE_ARRAY_CAST); + if (obj_ht) { + HashTable *new_obj_ht = zend_proptable_to_symtable(obj_ht, + (Z_OBJCE_P(op)->default_properties_count || + Z_OBJ_P(op)->handlers != &std_object_handlers || + GC_IS_RECURSIVE(obj_ht))); + zval_ptr_dtor(op); + ZVAL_ARR(op, new_obj_ht); + zend_release_properties(obj_ht); } else { - zval dst; - convert_object_to_type(op, &dst, IS_ARRAY, convert_to_array); - - if (Z_TYPE(dst) == IS_ARRAY) { - zval_ptr_dtor(op); - ZVAL_COPY_VALUE(op, &dst); - return; - } + zval_ptr_dtor(op); + /*ZVAL_EMPTY_ARRAY(op);*/ + array_init(op); } - - zval_ptr_dtor(op); - /*ZVAL_EMPTY_ARRAY(op);*/ - array_init(op); } break; case IS_NULL: diff --git a/Zend/zend_types.h b/Zend/zend_types.h index 2e773df6e96c5..de731c310efce 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -670,9 +670,6 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) { #define Z_OBJPROP(zval) Z_OBJ_HT((zval))->get_properties(&(zval)) #define Z_OBJPROP_P(zval_p) Z_OBJPROP(*(zval_p)) -#define Z_OBJDEBUG(zval,tmp) (Z_OBJ_HANDLER((zval),get_debug_info)?Z_OBJ_HANDLER((zval),get_debug_info)(&(zval),&tmp):(tmp=0,Z_OBJPROP(zval))) -#define Z_OBJDEBUG_P(zval_p,tmp) Z_OBJDEBUG(*(zval_p), tmp) - #define Z_RES(zval) (zval).value.res #define Z_RES_P(zval_p) Z_RES(*zval_p) diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index cc006024e57e7..36fb9a1779b4c 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -5307,22 +5307,18 @@ ZEND_VM_COLD_CONST_HANDLER(21, ZEND_CAST, CONST|TMP|VAR|CV, ANY, TYPE) } else { ZVAL_EMPTY_ARRAY(result); } - } else if (Z_OBJ_HT_P(expr)->get_properties) { - HashTable *obj_ht = Z_OBJ_HT_P(expr)->get_properties(expr); + } else { + HashTable *obj_ht = zend_get_properties_for(expr, ZEND_PROP_PURPOSE_ARRAY_CAST); if (obj_ht) { /* fast copy */ - obj_ht = zend_proptable_to_symtable(obj_ht, + ZVAL_ARR(result, zend_proptable_to_symtable(obj_ht, (Z_OBJCE_P(expr)->default_properties_count || Z_OBJ_P(expr)->handlers != &std_object_handlers || - GC_IS_RECURSIVE(obj_ht))); - ZVAL_ARR(result, obj_ht); + GC_IS_RECURSIVE(obj_ht)))); + zend_release_properties(obj_ht); } else { ZVAL_EMPTY_ARRAY(result); } - } else { - ZVAL_COPY_VALUE(result, expr); - Z_ADDREF_P(result); - convert_to_array(result); } } else { ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def)); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 5760612f003eb..03d767247b662 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3121,22 +3121,18 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CAST_SPEC_CONST_H } else { ZVAL_EMPTY_ARRAY(result); } - } else if (Z_OBJ_HT_P(expr)->get_properties) { - HashTable *obj_ht = Z_OBJ_HT_P(expr)->get_properties(expr); + } else { + HashTable *obj_ht = zend_get_properties_for(expr, ZEND_PROP_PURPOSE_ARRAY_CAST); if (obj_ht) { /* fast copy */ - obj_ht = zend_proptable_to_symtable(obj_ht, + ZVAL_ARR(result, zend_proptable_to_symtable(obj_ht, (Z_OBJCE_P(expr)->default_properties_count || Z_OBJ_P(expr)->handlers != &std_object_handlers || - GC_IS_RECURSIVE(obj_ht))); - ZVAL_ARR(result, obj_ht); + GC_IS_RECURSIVE(obj_ht)))); + zend_release_properties(obj_ht); } else { ZVAL_EMPTY_ARRAY(result); } - } else { - ZVAL_COPY_VALUE(result, expr); - Z_ADDREF_P(result); - convert_to_array(result); } } else { ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def)); @@ -18092,22 +18088,18 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CAST_SPEC_TMP_HANDLER(ZEND_OPC } else { ZVAL_EMPTY_ARRAY(result); } - } else if (Z_OBJ_HT_P(expr)->get_properties) { - HashTable *obj_ht = Z_OBJ_HT_P(expr)->get_properties(expr); + } else { + HashTable *obj_ht = zend_get_properties_for(expr, ZEND_PROP_PURPOSE_ARRAY_CAST); if (obj_ht) { /* fast copy */ - obj_ht = zend_proptable_to_symtable(obj_ht, + ZVAL_ARR(result, zend_proptable_to_symtable(obj_ht, (Z_OBJCE_P(expr)->default_properties_count || Z_OBJ_P(expr)->handlers != &std_object_handlers || - GC_IS_RECURSIVE(obj_ht))); - ZVAL_ARR(result, obj_ht); + GC_IS_RECURSIVE(obj_ht)))); + zend_release_properties(obj_ht); } else { ZVAL_EMPTY_ARRAY(result); } - } else { - ZVAL_COPY_VALUE(result, expr); - Z_ADDREF_P(result); - convert_to_array(result); } } else { ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def)); @@ -21100,22 +21092,18 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CAST_SPEC_VAR_HANDLER(ZEND_OPC } else { ZVAL_EMPTY_ARRAY(result); } - } else if (Z_OBJ_HT_P(expr)->get_properties) { - HashTable *obj_ht = Z_OBJ_HT_P(expr)->get_properties(expr); + } else { + HashTable *obj_ht = zend_get_properties_for(expr, ZEND_PROP_PURPOSE_ARRAY_CAST); if (obj_ht) { /* fast copy */ - obj_ht = zend_proptable_to_symtable(obj_ht, + ZVAL_ARR(result, zend_proptable_to_symtable(obj_ht, (Z_OBJCE_P(expr)->default_properties_count || Z_OBJ_P(expr)->handlers != &std_object_handlers || - GC_IS_RECURSIVE(obj_ht))); - ZVAL_ARR(result, obj_ht); + GC_IS_RECURSIVE(obj_ht)))); + zend_release_properties(obj_ht); } else { ZVAL_EMPTY_ARRAY(result); } - } else { - ZVAL_COPY_VALUE(result, expr); - Z_ADDREF_P(result); - convert_to_array(result); } } else { ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def)); @@ -37467,22 +37455,18 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CAST_SPEC_CV_HANDLER(ZEND_OPCO } else { ZVAL_EMPTY_ARRAY(result); } - } else if (Z_OBJ_HT_P(expr)->get_properties) { - HashTable *obj_ht = Z_OBJ_HT_P(expr)->get_properties(expr); + } else { + HashTable *obj_ht = zend_get_properties_for(expr, ZEND_PROP_PURPOSE_ARRAY_CAST); if (obj_ht) { /* fast copy */ - obj_ht = zend_proptable_to_symtable(obj_ht, + ZVAL_ARR(result, zend_proptable_to_symtable(obj_ht, (Z_OBJCE_P(expr)->default_properties_count || Z_OBJ_P(expr)->handlers != &std_object_handlers || - GC_IS_RECURSIVE(obj_ht))); - ZVAL_ARR(result, obj_ht); + GC_IS_RECURSIVE(obj_ht)))); + zend_release_properties(obj_ht); } else { ZVAL_EMPTY_ARRAY(result); } - } else { - ZVAL_COPY_VALUE(result, expr); - Z_ADDREF_P(result); - convert_to_array(result); } } else { ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def)); diff --git a/ext/json/json_encoder.c b/ext/json/json_encoder.c index c79e694f260b0..8ab190d075b3e 100644 --- a/ext/json/json_encoder.c +++ b/ext/json/json_encoder.c @@ -130,19 +130,21 @@ static inline void php_json_encode_double(smart_str *buf, double d, int options) static int php_json_encode_array(smart_str *buf, zval *val, int options, php_json_encoder *encoder) /* {{{ */ { int i, r, need_comma = 0; - HashTable *myht; + HashTable *myht, *prop_ht; if (Z_TYPE_P(val) == IS_ARRAY) { myht = Z_ARRVAL_P(val); + prop_ht = NULL; r = (options & PHP_JSON_FORCE_OBJECT) ? PHP_JSON_OUTPUT_OBJECT : php_json_determine_array_type(val); } else { - myht = Z_OBJPROP_P(val); + prop_ht = myht = zend_get_properties_for(val, ZEND_PROP_PURPOSE_JSON); r = PHP_JSON_OUTPUT_OBJECT; } if (myht && GC_IS_RECURSIVE(myht)) { encoder->error_code = PHP_JSON_ERROR_RECURSION; smart_str_appendl(buf, "null", 4); + zend_release_properties(prop_ht); return FAILURE; } @@ -218,6 +220,7 @@ static int php_json_encode_array(smart_str *buf, zval *val, int options, php_jso if (php_json_encode_zval(buf, data, options, encoder) == FAILURE && !(options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR)) { PHP_JSON_HASH_UNPROTECT_RECURSION(myht); + zend_release_properties(prop_ht); return FAILURE; } } ZEND_HASH_FOREACH_END(); @@ -228,6 +231,7 @@ static int php_json_encode_array(smart_str *buf, zval *val, int options, php_jso if (encoder->depth > encoder->max_depth) { encoder->error_code = PHP_JSON_ERROR_DEPTH; if (!(options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR)) { + zend_release_properties(prop_ht); return FAILURE; } } @@ -245,6 +249,7 @@ static int php_json_encode_array(smart_str *buf, zval *val, int options, php_jso smart_str_appendc(buf, '}'); } + zend_release_properties(prop_ht); return SUCCESS; } /* }}} */ diff --git a/ext/standard/var.c b/ext/standard/var.c index e4c79328680ac..6996221bb3282 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -82,7 +82,6 @@ PHPAPI void php_var_dump(zval *struc, int level) /* {{{ */ { HashTable *myht; zend_string *class_name; - int is_temp; int is_ref = 0; zend_ulong num; zend_string *key; @@ -145,7 +144,7 @@ PHPAPI void php_var_dump(zval *struc, int level) /* {{{ */ } Z_PROTECT_RECURSION_P(struc); - myht = Z_OBJDEBUG_P(struc, is_temp); + myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_DEBUG); class_name = Z_OBJ_HANDLER_P(struc, get_class_name)(Z_OBJ_P(struc)); php_printf("%sobject(%s)#%d (%d) {\n", COMMON, ZSTR_VAL(class_name), Z_OBJ_HANDLE_P(struc), myht ? zend_array_count(myht) : 0); zend_string_release_ex(class_name, 0); @@ -158,10 +157,7 @@ PHPAPI void php_var_dump(zval *struc, int level) /* {{{ */ ZEND_HASH_FOREACH_KEY_VAL_IND(myht, num, key, val) { php_object_property_dump(val, num, key, level); } ZEND_HASH_FOREACH_END(); - if (is_temp) { - zend_hash_destroy(myht); - efree(myht); - } + zend_release_properties(myht); } if (level > 1) { php_printf("%*c", level-1, ' '); @@ -249,7 +245,6 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */ { HashTable *myht = NULL; zend_string *class_name; - int is_temp = 0; int is_ref = 0; zend_ulong index; zend_string *key; @@ -299,20 +294,17 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */ if (level > 1 && !(GC_FLAGS(myht) & GC_IMMUTABLE)) { GC_UNPROTECT_RECURSION(myht); } - if (is_temp) { - zend_hash_destroy(myht); - efree(myht); - } if (level > 1) { php_printf("%*c", level - 1, ' '); } PUTS("}\n"); break; case IS_OBJECT: - myht = Z_OBJDEBUG_P(struc, is_temp); + myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_DEBUG); if (myht) { if (GC_IS_RECURSIVE(myht)) { PUTS("*RECURSION*\n"); + zend_release_properties(myht); return; } GC_PROTECT_RECURSION(myht); @@ -325,10 +317,7 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */ zval_object_property_dump(val, index, key, level); } ZEND_HASH_FOREACH_END(); GC_UNPROTECT_RECURSION(myht); - if (is_temp) { - zend_hash_destroy(myht); - efree(myht); - } + zend_release_properties(myht); } if (level > 1) { php_printf("%*c", level - 1, ' '); @@ -510,11 +499,12 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf) /* {{{ */ break; case IS_OBJECT: - myht = Z_OBJPROP_P(struc); + myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_VAR_EXPORT); if (myht) { if (GC_IS_RECURSIVE(myht)) { smart_str_appendl(buf, "NULL", 4); zend_error(E_WARNING, "var_export does not handle circular references"); + zend_release_properties(myht); return; } else { GC_PROTECT_RECURSION(myht); @@ -538,6 +528,7 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf) /* {{{ */ php_object_element_export(val, index, key, level, buf); } ZEND_HASH_FOREACH_END(); GC_UNPROTECT_RECURSION(myht); + zend_release_properties(myht); } if (level > 1) { buffer_append_spaces(buf, level - 1); @@ -743,7 +734,7 @@ static void php_var_serialize_class(smart_str *buf, zval *struc, zval *retval_pt smart_str_appendl(buf, ":{", 2); ZVAL_NULL(&nval); - propers = Z_OBJPROP_P(struc); + propers = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_SERIALIZE); ZEND_HASH_FOREACH_STR_KEY(&names, name) { zend_string *prot_name, *priv_name; @@ -809,6 +800,7 @@ static void php_var_serialize_class(smart_str *buf, zval *struc, zval *retval_pt smart_str_appendc(buf, '}'); zend_hash_destroy(&names); + zend_release_properties(propers); } /* }}} */ @@ -925,7 +917,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ i = zend_array_count(myht); } else { incomplete_class = php_var_serialize_class_name(buf, struc); - myht = Z_OBJPROP_P(struc); + myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_SERIALIZE); /* count after serializing name, since php_var_serialize_class_name * changes the count if the variable is incomplete class */ i = zend_array_count(myht); @@ -978,6 +970,9 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ } ZEND_HASH_FOREACH_END(); } smart_str_appendc(buf, '}'); + if (Z_TYPE_P(struc) == IS_OBJECT) { + zend_release_properties(myht); + } return; } case IS_REFERENCE: diff --git a/sapi/phpdbg/phpdbg_utils.c b/sapi/phpdbg/phpdbg_utils.c index 0e403a783b8df..a9b017ef566e7 100644 --- a/sapi/phpdbg/phpdbg_utils.c +++ b/sapi/phpdbg/phpdbg_utils.c @@ -660,8 +660,6 @@ PHPDBG_API void phpdbg_xml_var_dump(zval *zv) { int (*element_dump_func)(zval *zv, zend_string *key, zend_ulong num); zend_bool is_ref = 0; - int is_temp; - phpdbg_try_access { is_ref = Z_ISREF_P(zv) && GC_REFCOUNT(Z_COUNTED_P(zv)) > 1; ZVAL_DEREF(zv); @@ -696,10 +694,9 @@ PHPDBG_API void phpdbg_xml_var_dump(zval *zv) { } phpdbg_xml("", COMMON, zend_hash_num_elements(myht)); element_dump_func = phpdbg_xml_array_element_dump; - is_temp = 0; goto head_done; case IS_OBJECT: - myht = Z_OBJDEBUG_P(zv, is_temp); + myht = zend_get_properties_for(zv, ZEND_PROP_PURPOSE_DEBUG); if (myht && GC_IS_RECURSIVE(myht)) { phpdbg_xml(""); break; @@ -717,9 +714,8 @@ PHPDBG_API void phpdbg_xml_var_dump(zval *zv) { } ZEND_HASH_FOREACH_END(); zend_hash_apply_with_arguments(myht, (apply_func_args_t) element_dump_func, 0); GC_UNPROTECT_RECURSION(myht); - if (is_temp) { - zend_hash_destroy(myht); - efree(myht); + if (Z_TYPE_P(zv) == IS_OBJECT) { + zend_release_properties(myht); } } if (Z_TYPE_P(zv) == IS_ARRAY) { From d2e7506717ada86f69314078b7a0d2504e2ec05e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 4 Oct 2018 16:51:52 +0200 Subject: [PATCH 2/3] Migrate ext/date to get_properties_for where appropriate This resolves the long-standing issue where var_dump a DateTime (etc) object makes a number of additional properties accessible, which may also change other behaviors as a side-effect. --- ext/date/php_date.c | 49 ++++++++++++++--------- ext/date/tests/DateTime_clone_basic3.phpt | 36 ++++++++--------- ext/date/tests/bug52113.phpt | 20 ++++----- ext/date/tests/bug75232.phpt | 24 +++++++++++ 4 files changed, 81 insertions(+), 48 deletions(-) create mode 100644 ext/date/tests/bug75232.phpt diff --git a/ext/date/php_date.c b/ext/date/php_date.c index d1f94dcd2211d..3c080767db5db 100644 --- a/ext/date/php_date.c +++ b/ext/date/php_date.c @@ -667,12 +667,12 @@ static zend_object *date_object_clone_period(zval *this_ptr); static int date_object_compare_date(zval *d1, zval *d2); static HashTable *date_object_get_gc(zval *object, zval **table, int *n); -static HashTable *date_object_get_properties(zval *object); +static HashTable *date_object_get_properties_for(zval *object, zend_prop_purpose purpose); static HashTable *date_object_get_gc_interval(zval *object, zval **table, int *n); static HashTable *date_object_get_properties_interval(zval *object); static HashTable *date_object_get_gc_period(zval *object, zval **table, int *n); static HashTable *date_object_get_properties_period(zval *object); -static HashTable *date_object_get_properties_timezone(zval *object); +static HashTable *date_object_get_properties_for_timezone(zval *object, zend_prop_purpose purpose); static HashTable *date_object_get_gc_timezone(zval *object, zval **table, int *n); static HashTable *date_object_get_debug_info_timezone(zval *object, int *is_temp); static void php_timezone_to_string(php_timezone_obj *tzobj, zval *zv); @@ -2123,7 +2123,7 @@ static void date_register_classes(void) /* {{{ */ date_object_handlers_date.free_obj = date_object_free_storage_date; date_object_handlers_date.clone_obj = date_object_clone_date; date_object_handlers_date.compare_objects = date_object_compare_date; - date_object_handlers_date.get_properties = date_object_get_properties; + date_object_handlers_date.get_properties_for = date_object_get_properties_for; date_object_handlers_date.get_gc = date_object_get_gc; zend_class_implements(date_ce_date, 1, date_ce_interface); @@ -2133,7 +2133,7 @@ static void date_register_classes(void) /* {{{ */ memcpy(&date_object_handlers_immutable, &std_object_handlers, sizeof(zend_object_handlers)); date_object_handlers_immutable.clone_obj = date_object_clone_date; date_object_handlers_immutable.compare_objects = date_object_compare_date; - date_object_handlers_immutable.get_properties = date_object_get_properties; + date_object_handlers_immutable.get_properties_for = date_object_get_properties_for; date_object_handlers_immutable.get_gc = date_object_get_gc; zend_class_implements(date_ce_immutable, 1, date_ce_interface); @@ -2144,7 +2144,7 @@ static void date_register_classes(void) /* {{{ */ date_object_handlers_timezone.offset = XtOffsetOf(php_timezone_obj, std); date_object_handlers_timezone.free_obj = date_object_free_storage_timezone; date_object_handlers_timezone.clone_obj = date_object_clone_timezone; - date_object_handlers_timezone.get_properties = date_object_get_properties_timezone; + date_object_handlers_timezone.get_properties_for = date_object_get_properties_for_timezone; date_object_handlers_timezone.get_gc = date_object_get_gc_timezone; date_object_handlers_timezone.get_debug_info = date_object_get_debug_info_timezone; @@ -2273,17 +2273,23 @@ static HashTable *date_object_get_gc_timezone(zval *object, zval **table, int *n return zend_std_get_properties(object); } /* }}} */ -static HashTable *date_object_get_properties(zval *object) /* {{{ */ +static HashTable *date_object_get_properties_for(zval *object, zend_prop_purpose purpose) /* {{{ */ { HashTable *props; zval zv; - php_date_obj *dateobj; + php_date_obj *dateobj; + switch (purpose) { + case ZEND_PROP_PURPOSE_DEBUG: + case ZEND_PROP_PURPOSE_SERIALIZE: + case ZEND_PROP_PURPOSE_VAR_EXPORT: + break; + default: + return zend_std_get_properties_for(object, purpose); + } dateobj = Z_PHPDATE_P(object); - - props = zend_std_get_properties(object); - + props = zend_array_dup(zend_std_get_properties(object)); if (!dateobj->time) { return props; } @@ -2387,16 +2393,23 @@ static void php_timezone_to_string(php_timezone_obj *tzobj, zval *zv) } } -static HashTable *date_object_get_properties_timezone(zval *object) /* {{{ */ +static HashTable *date_object_get_properties_for_timezone(zval *object, zend_prop_purpose purpose) /* {{{ */ { HashTable *props; zval zv; - php_timezone_obj *tzobj; - - tzobj = Z_PHPTIMEZONE_P(object); + php_timezone_obj *tzobj; - props = zend_std_get_properties(object); + switch (purpose) { + case ZEND_PROP_PURPOSE_DEBUG: + case ZEND_PROP_PURPOSE_SERIALIZE: + case ZEND_PROP_PURPOSE_VAR_EXPORT: + break; + default: + return zend_std_get_properties_for(object, purpose); + } + tzobj = Z_PHPTIMEZONE_P(object); + props = zend_array_dup(zend_std_get_properties(object)); if (!tzobj->initialized) { return props; } @@ -2468,12 +2481,10 @@ static HashTable *date_object_get_properties_interval(zval *object) /* {{{ */ { HashTable *props; zval zv; - php_interval_obj *intervalobj; + php_interval_obj *intervalobj; intervalobj = Z_PHPINTERVAL_P(object); - props = zend_std_get_properties(object); - if (!intervalobj->initialized) { return props; } @@ -5094,9 +5105,7 @@ static HashTable *date_object_get_properties_period(zval *object) /* {{{ */ php_period_obj *period_obj; period_obj = Z_PHPPERIOD_P(object); - props = zend_std_get_properties(object); - if (!period_obj->start) { return props; } diff --git a/ext/date/tests/DateTime_clone_basic3.phpt b/ext/date/tests/DateTime_clone_basic3.phpt index f3d9c142fb515..ac117958699e6 100644 --- a/ext/date/tests/DateTime_clone_basic3.phpt +++ b/ext/date/tests/DateTime_clone_basic3.phpt @@ -41,40 +41,34 @@ object(DateTime)#%d (3) { -- Add some properties -- object(DateTime)#%d (5) { - ["date"]=> - string(26) "2009-02-03 12:34:41.000000" - ["timezone_type"]=> - int(2) - ["timezone"]=> - string(3) "GMT" ["property1"]=> int(99) ["property2"]=> string(5) "Hello" -} - --- clone it -- -object(DateTime)#%d (5) { ["date"]=> string(26) "2009-02-03 12:34:41.000000" ["timezone_type"]=> int(2) ["timezone"]=> string(3) "GMT" +} + +-- clone it -- +object(DateTime)#%d (5) { ["property1"]=> int(99) ["property2"]=> string(5) "Hello" -} - --- Add some more properties -- -object(DateTime)#%d (7) { ["date"]=> string(26) "2009-02-03 12:34:41.000000" ["timezone_type"]=> int(2) ["timezone"]=> string(3) "GMT" +} + +-- Add some more properties -- +object(DateTime)#%d (7) { ["property1"]=> int(99) ["property2"]=> @@ -83,16 +77,16 @@ object(DateTime)#%d (7) { bool(true) ["property4"]=> float(10.5) -} - --- clone it -- -object(DateTime)#%d (7) { ["date"]=> string(26) "2009-02-03 12:34:41.000000" ["timezone_type"]=> int(2) ["timezone"]=> string(3) "GMT" +} + +-- clone it -- +object(DateTime)#%d (7) { ["property1"]=> int(99) ["property2"]=> @@ -101,5 +95,11 @@ object(DateTime)#%d (7) { bool(true) ["property4"]=> float(10.5) + ["date"]=> + string(26) "2009-02-03 12:34:41.000000" + ["timezone_type"]=> + int(2) + ["timezone"]=> + string(3) "GMT" } ===DONE=== diff --git a/ext/date/tests/bug52113.phpt b/ext/date/tests/bug52113.phpt index dfc7d6112d353..f6d0962625db4 100644 --- a/ext/date/tests/bug52113.phpt +++ b/ext/date/tests/bug52113.phpt @@ -32,8 +32,8 @@ $p = new DatePeriod($start, $diff_un, 2); var_dump($unser, $p); ?> ---EXPECT-- -object(DateInterval)#3 (16) { +--EXPECTF-- +object(DateInterval)#%d (16) { ["y"]=> int(0) ["m"]=> @@ -85,7 +85,7 @@ DateInterval::__set_state(array( 'special_amount' => 0, 'have_weekday_relative' => 0, 'have_special_relative' => 0, -))object(DateInterval)#5 (16) { +))object(DateInterval)#%d (16) { ["y"]=> int(0) ["m"]=> @@ -119,9 +119,9 @@ DateInterval::__set_state(array( ["have_special_relative"]=> int(0) } -object(DatePeriod)#6 (6) { +object(DatePeriod)#%d (6) { ["start"]=> - object(DateTime)#4 (3) { + object(DateTime)#%d (3) { ["date"]=> string(26) "2003-01-02 08:00:00.000000" ["timezone_type"]=> @@ -134,7 +134,7 @@ object(DatePeriod)#6 (6) { ["end"]=> NULL ["interval"]=> - object(DateInterval)#7 (16) { + object(DateInterval)#%d (16) { ["y"]=> int(0) ["m"]=> @@ -173,7 +173,7 @@ object(DatePeriod)#6 (6) { ["include_start_date"]=> bool(true) } -object(DateInterval)#8 (16) { +object(DateInterval)#%d (16) { ["y"]=> int(7) ["m"]=> @@ -207,9 +207,9 @@ object(DateInterval)#8 (16) { ["have_special_relative"]=> int(0) } -object(DatePeriod)#9 (6) { +object(DatePeriod)#%d (6) { ["start"]=> - object(DateTime)#6 (3) { + object(DateTime)#%d (3) { ["date"]=> string(26) "2003-01-02 08:00:00.000000" ["timezone_type"]=> @@ -222,7 +222,7 @@ object(DatePeriod)#9 (6) { ["end"]=> NULL ["interval"]=> - object(DateInterval)#7 (16) { + object(DateInterval)#%d (16) { ["y"]=> int(0) ["m"]=> diff --git a/ext/date/tests/bug75232.phpt b/ext/date/tests/bug75232.phpt new file mode 100644 index 0000000000000..2873bb93ffb20 --- /dev/null +++ b/ext/date/tests/bug75232.phpt @@ -0,0 +1,24 @@ +--TEST-- +Bug #75232: print_r of DateTime creating side-effect +--FILE-- +date, "\n"; + +$d2 = DateTime::createFromFormat("Ymd\THis\Z", '20170920T091600Z'); +print_r($d2); +echo $d2->date, "\n"; + +?> +--EXPECTF-- +Notice: Undefined property: DateTime::$date in %s on line %d + +DateTime Object +( + [date] => 2017-09-20 09:16:00.000000 + [timezone_type] => 3 + [timezone] => UTC +) + +Notice: Undefined property: DateTime::$date in %s on line %d From ee0dcce7a871e11fb0b08f86b7ab1e7bed04bd99 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 5 Oct 2018 15:55:21 +0200 Subject: [PATCH 3/3] Don't overload get_properties for ArrayObject Instead overload get_properties_for for a few specific cases such as array casts. This resolves the issue where ArrayObject get_properties may violate engine invariants in some cases. --- ext/reflection/tests/bug61388.phpt | 6 -- ext/spl/spl_array.c | 36 ++++++++-- .../tests/ArrayObject_get_object_vars.phpt | 30 ++++++++ ext/spl/tests/array_017.phpt | 72 +++++++++++-------- ext/spl/tests/bug61347.phpt | 4 +- tests/classes/arrayobject_001.phpt | 13 ---- 6 files changed, 105 insertions(+), 56 deletions(-) create mode 100644 ext/spl/tests/ArrayObject_get_object_vars.phpt delete mode 100644 tests/classes/arrayobject_001.phpt diff --git a/ext/reflection/tests/bug61388.phpt b/ext/reflection/tests/bug61388.phpt index 3d6dc83fa0de6..b9fe6bfaaa91b 100644 --- a/ext/reflection/tests/bug61388.phpt +++ b/ext/reflection/tests/bug61388.phpt @@ -14,12 +14,6 @@ print_r($reflObj->getProperties(ReflectionProperty::IS_PUBLIC)); --EXPECT-- Array ( - [0] => ReflectionProperty Object - ( - [name] => test - [class] => ArrayObject - ) - ) Array ( diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 63345e6e331d8..e2b3ee410858e 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -807,18 +807,40 @@ SPL_METHOD(Array, getArrayCopy) RETURN_ARR(zend_array_dup(spl_array_get_hash_table(intern))); } /* }}} */ -static HashTable *spl_array_get_properties(zval *object) /* {{{ */ +static HashTable *spl_array_get_properties_for(zval *object, zend_prop_purpose purpose) /* {{{ */ { spl_array_object *intern = Z_SPLARRAY_P(object); + HashTable *ht; + zend_bool dup; if (intern->ar_flags & SPL_ARRAY_STD_PROP_LIST) { - if (!intern->std.properties) { - rebuild_object_properties(&intern->std); - } - return intern->std.properties; + return zend_std_get_properties_for(object, purpose); + } + + /* We are supposed to be the only owner of the internal hashtable. + * The "dup" flag decides whether this is a "long-term" use where + * we need to duplicate, or a "temporary" one, where we can expect + * that no operations on the ArrayObject will be performed in the + * meantime. */ + switch (purpose) { + case ZEND_PROP_PURPOSE_ARRAY_CAST: + dup = 1; + break; + case ZEND_PROP_PURPOSE_VAR_EXPORT: + case ZEND_PROP_PURPOSE_JSON: + dup = 0; + break; + default: + return zend_std_get_properties_for(object, purpose); } - return spl_array_get_hash_table(intern); + ht = spl_array_get_hash_table(intern); + if (dup) { + ht = zend_array_dup(ht); + } else { + GC_ADDREF(ht); + } + return ht; } /* }}} */ static HashTable* spl_array_get_debug_info(zval *obj, int *is_temp) /* {{{ */ @@ -2013,7 +2035,7 @@ PHP_MINIT_FUNCTION(spl_array) spl_handler_ArrayObject.has_dimension = spl_array_has_dimension; spl_handler_ArrayObject.count_elements = spl_array_object_count_elements; - spl_handler_ArrayObject.get_properties = spl_array_get_properties; + spl_handler_ArrayObject.get_properties_for = spl_array_get_properties_for; spl_handler_ArrayObject.get_debug_info = spl_array_get_debug_info; spl_handler_ArrayObject.get_gc = spl_array_get_gc; spl_handler_ArrayObject.read_property = spl_array_read_property; diff --git a/ext/spl/tests/ArrayObject_get_object_vars.phpt b/ext/spl/tests/ArrayObject_get_object_vars.phpt new file mode 100644 index 0000000000000..a80add6b95ad7 --- /dev/null +++ b/ext/spl/tests/ArrayObject_get_object_vars.phpt @@ -0,0 +1,30 @@ +--TEST-- +get_object_vars() on ArrayObject works on the properties of the ArrayObject itself +--FILE-- +getObjectVars()); + +?> +--EXPECT-- +array(0) { +} +array(1) { + ["test"]=> + NULL +} diff --git a/ext/spl/tests/array_017.phpt b/ext/spl/tests/array_017.phpt index ed1286332cd95..0fde103627a0a 100644 --- a/ext/spl/tests/array_017.phpt +++ b/ext/spl/tests/array_017.phpt @@ -139,13 +139,17 @@ array(3) { ["Flags"]=> int(0) ["OVars"]=> - array(3) { - [0]=> - int(1) - ["a"]=> - int(25) + array(5) { ["pub1"]=> - int(42) + int(1) + ["pro1"]=> + int(2) + ["pri1"]=> + int(3) + ["imp1"]=> + int(4) + ["dyn1"]=> + int(5) } ["$this"]=> object(ArrayObjectEx)#%d (6) { @@ -178,13 +182,17 @@ array(3) { ["Flags"]=> int(0) ["OVars"]=> - array(3) { - [0]=> + array(5) { + ["pub2"]=> int(1) - ["a"]=> - int(25) - ["pub1"]=> - int(42) + ["pro2"]=> + int(2) + ["pri2"]=> + int(3) + ["imp2"]=> + int(4) + ["dyn2"]=> + int(5) } ["$this"]=> object(ArrayIteratorEx)#%d (6) { @@ -242,13 +250,17 @@ array(3) { ["Flags"]=> int(0) ["OVars"]=> - array(3) { - [0]=> + array(5) { + ["pub2"]=> int(1) - ["a"]=> - int(25) - ["pub1"]=> - int(42) + ["pro2"]=> + int(2) + ["pri2"]=> + int(3) + ["imp2"]=> + int(4) + ["dyn2"]=> + int(5) } ["$this"]=> object(ArrayIteratorEx)#%d (6) { @@ -541,14 +553,16 @@ array(3) { ["Flags"]=> int(0) ["OVars"]=> - array(4) { - ["pub1"]=> + array(5) { + ["pub2"]=> int(1) - ["pro1"]=> + ["pro2"]=> int(2) - ["imp1"]=> + ["pri2"]=> + int(3) + ["imp2"]=> int(4) - ["dyn1"]=> + ["dyn2"]=> int(5) } ["$this"]=> @@ -598,14 +612,16 @@ array(3) { ["Flags"]=> int(0) ["OVars"]=> - array(4) { - ["pub1"]=> + array(5) { + ["pub2"]=> int(1) - ["pro1"]=> + ["pro2"]=> int(2) - ["imp1"]=> + ["pri2"]=> + int(3) + ["imp2"]=> int(4) - ["dyn1"]=> + ["dyn2"]=> int(5) } ["$this"]=> diff --git a/ext/spl/tests/bug61347.phpt b/ext/spl/tests/bug61347.phpt index cb091858ae50d..720cf5b0f250f 100644 --- a/ext/spl/tests/bug61347.phpt +++ b/ext/spl/tests/bug61347.phpt @@ -12,7 +12,7 @@ var_dump(isset($b['no_exists'])); //false var_dump(empty($b['b'])); //true var_dump(empty($b[37])); //true -var_dump(array_key_exists('b', $b)); //true +var_dump(array_key_exists('b', $b)); //false var_dump($b['b']); $a = array('b' => '', 37 => false); @@ -31,7 +31,7 @@ bool(false) bool(false) bool(true) bool(true) -bool(true) +bool(false) NULL bool(true) bool(true) diff --git a/tests/classes/arrayobject_001.phpt b/tests/classes/arrayobject_001.phpt deleted file mode 100644 index b75f8c7ab3cc7..0000000000000 --- a/tests/classes/arrayobject_001.phpt +++ /dev/null @@ -1,13 +0,0 @@ ---TEST-- -Ensure that ArrayObject acts like an array ---FILE-- - ---EXPECT-- -bar1bar