Skip to content

Don't overload ArrayObject get_properties #3574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Don't overload ArrayObject get_properties #3574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e7adf7
Allow overriding (array) cast using cast_object handler
nikic Oct 4, 2018
53ba6bc
Don't overload get_properties in ArrayObject
nikic Oct 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@ PHP 7.4 UPGRADE NOTES
supported and resulted in corrupted reflection objects. It has been
explicitly prohibited now.

- SPL:
. Calling get_object_vars() on an ArrayObject instance will now always return
the properties of the ArrayObject itself (or a subclass). Previously it
returned the values of the wrapped array/object unless the STD_PROP_LIST
flag was specified. Other affected operations are:
* ReflectionObject::getProperties()
* array_key_exists(). Use isset() or offsetExists() instead.
* reset(), current(), etc. Use Iterator methods instead.
* Potentially others working on object properties as a list.

(array) casts are *not* affected. They will continue to return either the
wrapped array, or the ArrayObject properties, depending on whether the
STD_PROP_LIST flag is used.

========================================
2. New Features
========================================
Expand Down
15 changes: 15 additions & 0 deletions Zend/zend_object_handlers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,21 @@ ZEND_API int zend_std_cast_object_tostring(zval *readobj, zval *writeobj, int ty
}
}
return FAILURE;
case IS_ARRAY:
{
HashTable *obj_ht = Z_OBJ_HT_P(readobj)->get_properties(readobj);
if (obj_ht) {
/* fast copy */
obj_ht = zend_proptable_to_symtable(obj_ht,
(Z_OBJCE_P(readobj)->default_properties_count ||
Z_OBJ_P(readobj)->handlers != &std_object_handlers ||
GC_IS_RECURSIVE(obj_ht)));
ZVAL_ARR(writeobj, obj_ht);
} else {
array_init(writeobj);
}
return SUCCESS;
}
case _IS_BOOL:
ZVAL_TRUE(writeobj);
return SUCCESS;
Expand Down
34 changes: 14 additions & 20 deletions Zend/zend_operators.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,27 +610,13 @@ 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;
}
} else {
zval dst;
convert_object_to_type(op, &dst, IS_ARRAY, convert_to_array);
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;
}
if (Z_TYPE(dst) == IS_ARRAY) {
zval_ptr_dtor(op);
ZVAL_COPY_VALUE(op, &dst);
return;
}

zval_ptr_dtor(op);
Expand Down Expand Up @@ -2094,6 +2080,10 @@ ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2)
ret = compare_function(result, op_free, op2);
zend_free_obj_get_result(op_free);
return ret;
} else if (Z_TYPE_P(op2) == IS_ARRAY) {
/* Treat array like null */
ZVAL_LONG(result, 1);
return SUCCESS;
} else if (Z_TYPE_P(op2) != IS_OBJECT && Z_OBJ_HT_P(op1)->cast_object) {
ZVAL_UNDEF(&tmp_free);
if (Z_OBJ_HT_P(op1)->cast_object(op1, &tmp_free, ((Z_TYPE_P(op2) == IS_FALSE || Z_TYPE_P(op2) == IS_TRUE) ? _IS_BOOL : Z_TYPE_P(op2))) == FAILURE) {
Expand All @@ -2113,6 +2103,10 @@ ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2)
ret = compare_function(result, op1, op_free);
zend_free_obj_get_result(op_free);
return ret;
} else if (Z_TYPE_P(op1) == IS_ARRAY) {
/* Treat array like null */
ZVAL_LONG(result, -1);
return SUCCESS;
} else if (Z_TYPE_P(op1) != IS_OBJECT && Z_OBJ_HT_P(op2)->cast_object) {
ZVAL_UNDEF(&tmp_free);
if (Z_OBJ_HT_P(op2)->cast_object(op2, &tmp_free, ((Z_TYPE_P(op1) == IS_FALSE || Z_TYPE_P(op1) == IS_TRUE) ? _IS_BOOL : Z_TYPE_P(op1))) == FAILURE) {
Expand Down
33 changes: 18 additions & 15 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5307,22 +5307,25 @@ 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);
if (obj_ht) {
/* fast copy */
obj_ht = 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);
} else {
ZVAL_EMPTY_ARRAY(result);
}
} else {
ZVAL_COPY_VALUE(result, expr);
Z_ADDREF_P(result);
convert_to_array(result);
ZVAL_UNDEF(result);
if (Z_OBJ_HT_P(expr)->cast_object) {
if (Z_OBJ_HT_P(expr)->cast_object(expr, result, IS_ARRAY) == FAILURE) {
zend_error(E_RECOVERABLE_ERROR,
"Object of class %s could not be converted to array",
ZSTR_VAL(Z_OBJCE_P(expr)->name));
}
} else if (Z_OBJ_HT_P(expr)->get) {
zval *newop = Z_OBJ_HT_P(expr)->get(expr, result);
if (Z_TYPE_P(newop) != IS_OBJECT) {
/* for safety - avoid loop */
ZVAL_COPY_VALUE(result, newop);
convert_to_array(result);
}
}
if (UNEXPECTED(Z_TYPE_P(result) != IS_ARRAY)) {
array_init(result);
}
}
} else {
ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def));
Expand Down
132 changes: 72 additions & 60 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3121,22 +3121,25 @@ 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);
if (obj_ht) {
/* fast copy */
obj_ht = 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);
} else {
ZVAL_EMPTY_ARRAY(result);
}
} else {
ZVAL_COPY_VALUE(result, expr);
Z_ADDREF_P(result);
convert_to_array(result);
ZVAL_UNDEF(result);
if (Z_OBJ_HT_P(expr)->cast_object) {
if (Z_OBJ_HT_P(expr)->cast_object(expr, result, IS_ARRAY) == FAILURE) {
zend_error(E_RECOVERABLE_ERROR,
"Object of class %s could not be converted to array",
ZSTR_VAL(Z_OBJCE_P(expr)->name));
}
} else if (Z_OBJ_HT_P(expr)->get) {
zval *newop = Z_OBJ_HT_P(expr)->get(expr, result);
if (Z_TYPE_P(newop) != IS_OBJECT) {
/* for safety - avoid loop */
ZVAL_COPY_VALUE(result, newop);
convert_to_array(result);
}
}
if (UNEXPECTED(Z_TYPE_P(result) != IS_ARRAY)) {
array_init(result);
}
}
} else {
ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def));
Expand Down Expand Up @@ -18092,22 +18095,25 @@ 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);
if (obj_ht) {
/* fast copy */
obj_ht = 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);
} else {
ZVAL_EMPTY_ARRAY(result);
}
} else {
ZVAL_COPY_VALUE(result, expr);
Z_ADDREF_P(result);
convert_to_array(result);
ZVAL_UNDEF(result);
if (Z_OBJ_HT_P(expr)->cast_object) {
if (Z_OBJ_HT_P(expr)->cast_object(expr, result, IS_ARRAY) == FAILURE) {
zend_error(E_RECOVERABLE_ERROR,
"Object of class %s could not be converted to array",
ZSTR_VAL(Z_OBJCE_P(expr)->name));
}
} else if (Z_OBJ_HT_P(expr)->get) {
zval *newop = Z_OBJ_HT_P(expr)->get(expr, result);
if (Z_TYPE_P(newop) != IS_OBJECT) {
/* for safety - avoid loop */
ZVAL_COPY_VALUE(result, newop);
convert_to_array(result);
}
}
if (UNEXPECTED(Z_TYPE_P(result) != IS_ARRAY)) {
array_init(result);
}
}
} else {
ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def));
Expand Down Expand Up @@ -21100,22 +21106,25 @@ 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);
if (obj_ht) {
/* fast copy */
obj_ht = 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);
} else {
ZVAL_EMPTY_ARRAY(result);
}
} else {
ZVAL_COPY_VALUE(result, expr);
Z_ADDREF_P(result);
convert_to_array(result);
ZVAL_UNDEF(result);
if (Z_OBJ_HT_P(expr)->cast_object) {
if (Z_OBJ_HT_P(expr)->cast_object(expr, result, IS_ARRAY) == FAILURE) {
zend_error(E_RECOVERABLE_ERROR,
"Object of class %s could not be converted to array",
ZSTR_VAL(Z_OBJCE_P(expr)->name));
}
} else if (Z_OBJ_HT_P(expr)->get) {
zval *newop = Z_OBJ_HT_P(expr)->get(expr, result);
if (Z_TYPE_P(newop) != IS_OBJECT) {
/* for safety - avoid loop */
ZVAL_COPY_VALUE(result, newop);
convert_to_array(result);
}
}
if (UNEXPECTED(Z_TYPE_P(result) != IS_ARRAY)) {
array_init(result);
}
}
} else {
ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def));
Expand Down Expand Up @@ -37467,22 +37476,25 @@ 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);
if (obj_ht) {
/* fast copy */
obj_ht = 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);
} else {
ZVAL_EMPTY_ARRAY(result);
}
} else {
ZVAL_COPY_VALUE(result, expr);
Z_ADDREF_P(result);
convert_to_array(result);
ZVAL_UNDEF(result);
if (Z_OBJ_HT_P(expr)->cast_object) {
if (Z_OBJ_HT_P(expr)->cast_object(expr, result, IS_ARRAY) == FAILURE) {
zend_error(E_RECOVERABLE_ERROR,
"Object of class %s could not be converted to array",
ZSTR_VAL(Z_OBJCE_P(expr)->name));
}
} else if (Z_OBJ_HT_P(expr)->get) {
zval *newop = Z_OBJ_HT_P(expr)->get(expr, result);
if (Z_TYPE_P(newop) != IS_OBJECT) {
/* for safety - avoid loop */
ZVAL_COPY_VALUE(result, newop);
convert_to_array(result);
}
}
if (UNEXPECTED(Z_TYPE_P(result) != IS_ARRAY)) {
array_init(result);
}
}
} else {
ZVAL_OBJ(result, zend_objects_new(zend_standard_class_def));
Expand Down
6 changes: 0 additions & 6 deletions ext/reflection/tests/bug61388.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ print_r($reflObj->getProperties(ReflectionProperty::IS_PUBLIC));
--EXPECT--
Array
(
[0] => ReflectionProperty Object
(
[name] => test
[class] => ArrayObject
)

)
Array
(
Expand Down
4 changes: 4 additions & 0 deletions ext/simplexml/simplexml.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1865,6 +1865,10 @@ static int sxe_object_cast_ex(zval *readobj, zval *writeobj, int type)

sxe = Z_SXEOBJ_P(readobj);

if (type == IS_ARRAY) {
return zend_std_cast_object_tostring(readobj, writeobj, type);
}

if (type == _IS_BOOL) {
node = php_sxe_get_first_node(sxe, NULL);
if (node) {
Expand Down
28 changes: 13 additions & 15 deletions ext/spl/spl_array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,20 +807,6 @@ SPL_METHOD(Array, getArrayCopy)
RETURN_ARR(zend_array_dup(spl_array_get_hash_table(intern)));
} /* }}} */

static HashTable *spl_array_get_properties(zval *object) /* {{{ */
{
spl_array_object *intern = Z_SPLARRAY_P(object);

if (intern->ar_flags & SPL_ARRAY_STD_PROP_LIST) {
if (!intern->std.properties) {
rebuild_object_properties(&intern->std);
}
return intern->std.properties;
}

return spl_array_get_hash_table(intern);
} /* }}} */

static HashTable* spl_array_get_debug_info(zval *obj, int *is_temp) /* {{{ */
{
zval *storage;
Expand Down Expand Up @@ -949,6 +935,18 @@ static int spl_array_compare_objects(zval *o1, zval *o2) /* {{{ */
return result;
} /* }}} */

static int spl_array_cast_object(zval *obj, zval *result, int type) /* {{{ */
{
spl_array_object *intern = Z_SPLARRAY_P(obj);

if (type != IS_ARRAY || (intern->ar_flags & SPL_ARRAY_STD_PROP_LIST)) {
return zend_std_cast_object_tostring(obj, result, type);
}

ZVAL_ARR(result, zend_array_dup(spl_array_get_hash_table(intern)));
return SUCCESS;
} /* }}} */

static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht) /* {{{ */
{
zend_string *string_key;
Expand Down Expand Up @@ -2013,7 +2011,6 @@ 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_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;
Expand All @@ -2023,6 +2020,7 @@ PHP_MINIT_FUNCTION(spl_array)
spl_handler_ArrayObject.unset_property = spl_array_unset_property;

spl_handler_ArrayObject.compare_objects = spl_array_compare_objects;
spl_handler_ArrayObject.cast_object = spl_array_cast_object;
spl_handler_ArrayObject.dtor_obj = zend_objects_destroy_object;
spl_handler_ArrayObject.free_obj = spl_array_object_free_storage;

Expand Down
Loading