From a0502b89a65d24eb191a7c85bcffcf9b91454735 Mon Sep 17 00:00:00 2001 From: Andrea Faulds Date: Mon, 14 Nov 2016 18:20:45 +0000 Subject: [PATCH] Convert numeric keys in object/array casts RFC: https://wiki.php.net/rfc/convert_numeric_keys_in_object_array_casts This converts key types as appropriate in object to array and array to object casts, as well as in get_object_vars(). --- NEWS | 2 + UPGRADING | 10 ++ Zend/tests/cast_to_object.phpt | Bin 1106 -> 1112 bytes Zend/tests/object_array_cast.phpt | 12 +- Zend/tests/settype_object.phpt | Bin 1126 -> 1132 bytes Zend/zend_builtin_functions.c | 19 ++- Zend/zend_hash.c | 116 ++++++++++++++++++ Zend/zend_hash.h | 2 + Zend/zend_operators.c | 26 ++-- ext/reflection/tests/bug61388.phpt | 6 + .../tests/arrayObject___construct_basic7.phpt | 8 +- ext/standard/tests/array/var_export.phpt | 4 +- .../gettype_settype_basic.phpt | 12 +- .../tests/general_functions/type.phpt | 12 +- 14 files changed, 183 insertions(+), 46 deletions(-) diff --git a/NEWS b/NEWS index 62cc66c5b1..d92fe5175b 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ PHP NEWS operation). (Dmitry) . Implemented FR #72768 (Add ENABLE_VIRTUAL_TERMINAL_PROCESSING flag for php.exe). (Michele Locati) + . Implemented "Convert numeric keys in object/array casts" RFC, fixes + bugs #53838, #61655, #66173, #70925, #72254, etc. (Andrea) - Date: . Fixed bug #69587 (DateInterval properties and isset). (jhdxr) diff --git a/UPGRADING b/UPGRADING index 953866a9a1..138add6ae4 100644 --- a/UPGRADING +++ b/UPGRADING @@ -25,6 +25,16 @@ PHP 7.2 UPGRADE NOTES . is_object() will now return true for objects of class __PHP_Incomplete_Class. . Support for Netware operating systems have been removed. + . Casting arrays to objects (with (object) or settype()) will now covert + integer keys to string property names. This fixes the behaviour of previous + versions, where integer keys would become inaccessible properties with + integer names. + . Casting objects to arrays (with (object) or settype()), and retrieving + object properties in an array with get_object_vars(), will now convert + numeric string property names (that is, property names of the format + /^(0|(-?[1-9][0-9]*))$/ where PHP_INT_MIN <= n <= PHP_INT_MAX) to integer + keys. This fixes the behaviour of previous versions, where numeric string + property names would become inaccessible string keys. ======================================== 2. New Features diff --git a/Zend/tests/cast_to_object.phpt b/Zend/tests/cast_to_object.phpt index f8d4878475e44bde9f381693eef581bfe28f8979..04d68053e26fa2e6676ec6ce9f51711e60057d47 100644 GIT binary patch delta 54 vcmcb_af4%nHnXOZfl{ok9hZVaW?qSgp(c --EXPECT-- object(stdClass)#1 (3) { - [0]=> + ["0"]=> int(1) - [1]=> + ["1"]=> int(2) - [2]=> + ["2"]=> int(3) } array(3) { @@ -38,10 +38,10 @@ array(3) { int(3) } object(stdClass)#1 (3) { - [0]=> + ["0"]=> int(1) - [1]=> + ["1"]=> int(2) - [2]=> + ["2"]=> int(3) } diff --git a/Zend/tests/settype_object.phpt b/Zend/tests/settype_object.phpt index d619dce7e3a7d86e2d78bdd1a62f86ece92e6069..7f9e5bc64b1a4ee2abb68916b88218ebd3f56962 100644 GIT binary patch delta 69 zcmaFH@rGlAJ+rBjfl{ok9hZVaW?qSgp(cce->default_properties_count && properties == zobj->properties && !ZEND_HASH_GET_APPLY_COUNT(properties)) { /* fast copy */ if (EXPECTED(zobj->handlers == &std_object_handlers)) { - if (EXPECTED(!(GC_FLAGS(properties) & IS_ARRAY_IMMUTABLE))) { - GC_REFCOUNT(properties)++; - } - RETURN_ARR(properties); + RETURN_ARR(zend_proptable_to_symtable(properties, 0)); } - RETURN_ARR(zend_array_dup(properties)); + RETURN_ARR(zend_proptable_to_symtable(properties, 1)); } else { array_init_size(return_value, zend_hash_num_elements(properties)); @@ -1273,9 +1270,19 @@ ZEND_FUNCTION(get_object_vars) const char *prop_name, *class_name; size_t prop_len; zend_unmangle_property_name_ex(key, &class_name, &prop_name, &prop_len); + /* We assume here that a mangled property name is never + * numeric. This is probably a safe assumption, but + * theoretically someone might write an extension with + * private, numeric properties. Well, too bad. + */ zend_hash_str_add_new(Z_ARRVAL_P(return_value), prop_name, prop_len, value); } else { - zend_hash_add_new(Z_ARRVAL_P(return_value), key, value); + zend_ulong num_key; + if (ZEND_HANDLE_NUMERIC(key, num_key)) { + zend_hash_index_add_new(Z_ARRVAL_P(return_value), num_key, value); + } else { + zend_hash_add_new(Z_ARRVAL_P(return_value), key, value); + } } } } diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index c3cae2a831..836c5a5a44 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2477,6 +2477,122 @@ ZEND_API int ZEND_FASTCALL _zend_handle_numeric_str_ex(const char *key, size_t l } } +/* Takes a "symtable" hashtable (contains integer and non-numeric string keys) + * and converts it to a "proptable" (contains only string keys). + * If the symtable didn't need duplicating, its refcount is incremented. + */ +ZEND_API HashTable* ZEND_FASTCALL zend_symtable_to_proptable(HashTable *ht) +{ + zend_ulong num_key; + zend_string *str_key; + zval *zv; + + if (UNEXPECTED(HT_IS_PACKED(ht))) { + goto convert; + } + + ZEND_HASH_FOREACH_KEY_VAL(ht, num_key, str_key, zv) { + if (!str_key) { + goto convert; + } + } ZEND_HASH_FOREACH_END(); + + if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) { + GC_REFCOUNT(ht)++; + } + + return ht; + +convert: + { + HashTable *new_ht = emalloc(sizeof(HashTable)); + + zend_hash_init(new_ht, zend_hash_num_elements(ht), NULL, ZVAL_PTR_DTOR, 0); + + ZEND_HASH_FOREACH_KEY_VAL(ht, num_key, str_key, zv) { + if (!str_key) { + str_key = zend_long_to_str(num_key); + zend_string_delref(str_key); + } + do { + if (Z_OPT_REFCOUNTED_P(zv)) { + if (Z_ISREF_P(zv) && Z_REFCOUNT_P(zv) == 1) { + zv = Z_REFVAL_P(zv); + if (!Z_OPT_REFCOUNTED_P(zv)) { + break; + } + } + Z_ADDREF_P(zv); + } + } while (0); + zend_hash_update(new_ht, str_key, zv); + } ZEND_HASH_FOREACH_END(); + + return new_ht; + } +} + +/* Takes a "proptable" hashtable (contains only string keys) and converts it to + * a "symtable" (contains integer and non-numeric string keys). + * If the proptable didn't need duplicating, its refcount is incremented. + */ +ZEND_API HashTable* ZEND_FASTCALL zend_proptable_to_symtable(HashTable *ht, zend_bool always_duplicate) +{ + zend_ulong num_key; + zend_string *str_key; + zval *zv; + + ZEND_HASH_FOREACH_KEY_VAL(ht, num_key, str_key, zv) { + /* The `str_key &&` here might seem redundant: property tables should + * only have string keys. Unfortunately, this isn't true, at the very + * least because of ArrayObject, which stores a symtable where the + * property table should be. + */ + if (str_key && ZEND_HANDLE_NUMERIC(str_key, num_key)) { + goto convert; + } + } ZEND_HASH_FOREACH_END(); + + if (always_duplicate) { + return zend_array_dup(ht); + } + + if (EXPECTED(!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE))) { + GC_REFCOUNT(ht)++; + } + + return ht; + +convert: + { + HashTable *new_ht = emalloc(sizeof(HashTable)); + + zend_hash_init(new_ht, zend_hash_num_elements(ht), NULL, ZVAL_PTR_DTOR, 0); + + ZEND_HASH_FOREACH_KEY_VAL(ht, num_key, str_key, zv) { + do { + if (Z_OPT_REFCOUNTED_P(zv)) { + if (Z_ISREF_P(zv) && Z_REFCOUNT_P(zv) == 1) { + zv = Z_REFVAL_P(zv); + if (!Z_OPT_REFCOUNTED_P(zv)) { + break; + } + } + Z_ADDREF_P(zv); + } + } while (0); + /* Again, thank ArrayObject for `!str_key ||`. */ + if (!str_key || ZEND_HANDLE_NUMERIC(str_key, num_key)) { + zend_hash_index_update(new_ht, num_key, zv); + } else { + zend_hash_update(new_ht, str_key, zv); + } + } ZEND_HASH_FOREACH_END(); + + return new_ht; + } +} + /* * Local variables: * tab-width: 4 diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h index 190551d3f1..e8c01f5dbd 100644 --- a/Zend/zend_hash.h +++ b/Zend/zend_hash.h @@ -247,6 +247,8 @@ ZEND_API uint32_t zend_array_count(HashTable *ht); ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(HashTable *source); ZEND_API void ZEND_FASTCALL zend_array_destroy(HashTable *ht); ZEND_API void ZEND_FASTCALL zend_symtable_clean(HashTable *ht); +ZEND_API HashTable* ZEND_FASTCALL zend_symtable_to_proptable(HashTable *ht); +ZEND_API HashTable* ZEND_FASTCALL zend_proptable_to_symtable(HashTable *ht, zend_bool always_duplicate); ZEND_API int ZEND_FASTCALL _zend_handle_numeric_str_ex(const char *key, size_t length, zend_ulong *idx); diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 9a90250934..8005577694 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -588,25 +588,17 @@ try_again: if (obj_ht) { zend_array *arr; + /* fast copy */ if (!Z_OBJCE_P(op)->default_properties_count && obj_ht == Z_OBJ_P(op)->properties && - !ZEND_HASH_GET_APPLY_COUNT(Z_OBJ_P(op)->properties)) { - /* fast copy */ - if (EXPECTED(Z_OBJ_P(op)->handlers == &std_object_handlers)) { - arr = obj_ht; - if (EXPECTED(!(GC_FLAGS(Z_OBJ_P(op)->properties) & IS_ARRAY_IMMUTABLE))) { - GC_REFCOUNT(Z_OBJ_P(op)->properties)++; - } - } else { - arr = zend_array_dup(obj_ht); - } - zval_dtor(op); - ZVAL_ARR(op, arr); + !ZEND_HASH_GET_APPLY_COUNT(Z_OBJ_P(op)->properties) && + EXPECTED(Z_OBJ_P(op)->handlers == &std_object_handlers)) { + arr = zend_proptable_to_symtable(obj_ht, 0); } else { - arr = zend_array_dup(obj_ht); - zval_dtor(op); - ZVAL_ARR(op, arr); + arr = zend_proptable_to_symtable(obj_ht, 1); } + zval_dtor(op); + ZVAL_ARR(op, arr); return; } } else { @@ -645,10 +637,12 @@ try_again: case IS_ARRAY: { HashTable *ht = Z_ARR_P(op); - if (Z_IMMUTABLE_P(op)) { + ht = zend_symtable_to_proptable(ht); + if (GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) { /* TODO: try not to duplicate immutable arrays as well ??? */ ht = zend_array_dup(ht); } + zval_dtor(op); object_and_properties_init(op, zend_standard_class_def, ht); break; } diff --git a/ext/reflection/tests/bug61388.phpt b/ext/reflection/tests/bug61388.phpt index 75c0300151..3d6dc83fa0 100644 --- a/ext/reflection/tests/bug61388.phpt +++ b/ext/reflection/tests/bug61388.phpt @@ -24,6 +24,12 @@ Array Array ( [0] => ReflectionProperty Object + ( + [name] => 0 + [class] => stdClass + ) + + [1] => ReflectionProperty Object ( [name] => oo [class] => stdClass diff --git a/ext/spl/tests/arrayObject___construct_basic7.phpt b/ext/spl/tests/arrayObject___construct_basic7.phpt index 2474b38ce6..dd10b5fb6b 100644 --- a/ext/spl/tests/arrayObject___construct_basic7.phpt +++ b/ext/spl/tests/arrayObject___construct_basic7.phpt @@ -18,17 +18,17 @@ array(2) { int(1) } object(stdClass)#1 (2) { - [1]=> + ["1"]=> int(1) - [0]=> + ["0"]=> int(2) } object(ArrayObject)#2 (1) { ["storage":"ArrayObject":private]=> object(stdClass)#1 (2) { - [1]=> + ["1"]=> int(1) - [0]=> + ["0"]=> int(2) } } diff --git a/ext/standard/tests/array/var_export.phpt b/ext/standard/tests/array/var_export.phpt index 8caf40789f..0b1f76094d 100644 --- a/ext/standard/tests/array/var_export.phpt +++ b/ext/standard/tests/array/var_export.phpt @@ -7,7 +7,7 @@ var_export($a); ?> --EXPECT-- stdClass::__set_state(array( - 0 => 1, - 1 => 3, + '0' => 1, + '1' => 3, 'foo' => 'bar', )) diff --git a/ext/standard/tests/general_functions/gettype_settype_basic.phpt b/ext/standard/tests/general_functions/gettype_settype_basic.phpt index b2762fd83a..b082fbf5f5 100644 --- a/ext/standard/tests/general_functions/gettype_settype_basic.phpt +++ b/ext/standard/tests/general_functions/gettype_settype_basic.phpt @@ -740,11 +740,11 @@ string(5) "array" -- Iteration 1 -- bool(true) object(stdClass)#2 (3) { - [0]=> + ["0"]=> int(1) - [1]=> + ["1"]=> int(2) - [2]=> + ["2"]=> int(3) } string(6) "object" @@ -758,11 +758,11 @@ string(6) "object" -- Iteration 3 -- bool(true) object(stdClass)#2 (3) { - [0]=> + ["0"]=> int(2) - [1]=> + ["1"]=> int(3) - [2]=> + ["2"]=> int(4) } string(6) "object" diff --git a/ext/standard/tests/general_functions/type.phpt b/ext/standard/tests/general_functions/type.phpt index df2dbaf461..ac29a1bf38 100644 --- a/ext/standard/tests/general_functions/type.phpt +++ b/ext/standard/tests/general_functions/type.phpt @@ -265,11 +265,11 @@ array(0) { } bool(true) object(stdClass)#%d (3) { - [0]=> + ["0"]=> int(1) - [1]=> + ["1"]=> int(2) - [2]=> + ["2"]=> int(3) } bool(true) @@ -279,11 +279,11 @@ object(stdClass)#%d (1) { } bool(true) object(stdClass)#%d (3) { - [0]=> + ["0"]=> int(2) - [1]=> + ["1"]=> int(3) - [2]=> + ["2"]=> int(4) } bool(true) -- 2.40.0