Bug #66173 [Ana]: Can not access an array element by index

Edit report at https://bugs.php.net/bug.php?id=66173&edit=1 ID: 66173 Updated by: yohgaki@php.net Reported by: to dot yashin at gmail dot com Summary: Can not access an array element by index Status: Analyzed Type: Bug Package: Arrays related Operating System: ubuntu 13.10 PHP Version: 5.5.6 Block user comment: N Private report: N New Comment: Array to object conversion seems to have similar issue. http://3v4l.org/vFuWT Previous Comments: ------------------------------------------------------------------------ [2014-01-11 01:27:50] yohgaki@php.net Variable/property that has numeric name can be accessed, but object to array conversion breaks accessibility. http://3v4l.org/DY9bZ It's better to be fixed. IMO. ------------------------------------------------------------------------ [2014-01-08 02:31:36] phpmpan at mpan dot pl Maybe it should be prohibited to pass illegal names to ->{} operator? Pros: - Such names can't be accessed with -> operator anyway, so they have marginal use cases. - This bug. - Potentially bugs can arise in user code that assumes objects can't contain invalid names. ------------------------------------------------------------------------ [2014-01-08 02:01:23] yohgaki@php.net Usually, object property name follows variable name rules. http://www.php.net/manual/en/language.variables.basics.php So we cannot have digits as first char of property name. However, we may override this limitation by using {}, though. [yohgaki@dev PHP-5.3]$ php -r '$obj = new StdClass; $obj->{12} = 234; ${1} = 567; var_dump($obj, ${1});' object(stdClass)#1 (1) { ["12"]=> int(234) } int(567) http://3v4l.org/vEE1Z PHP is creating numeric property by $obj->{'1'}, but it seems odd to me. Anyway, I've added this bug to RFC. https://wiki.php.net/rfc/comparison_inconsistency#object_array_conversion_of_numeric_propertyindex ------------------------------------------------------------------------ [2013-12-05 11:31:28] inefedor at gmail dot com This is pretty old bug as I know, I had nothing to do so I made a patch, but I doubt it will be merged because this use case is so rare... diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 96c3f3d..14381b9 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -810,13 +810,20 @@ ZEND_API void zend_hash_reverse_apply(HashTable *ht, apply_func_t apply_func TSR HASH_UNPROTECT_RECURSION(ht); } - -ZEND_API void zend_hash_copy(HashTable *target, HashTable *source, copy_ctor_func_t pCopyConstructor, void *tmp, uint size) +ZEND_API void zend_hash_copy_ex(HashTable *target, HashTable *source, copy_ctor_func_t pCopyConstructor, void *tmp, uint size, int reindex_numeric_keys) { Bucket *p; void *new_entry; zend_bool setTargetPointer; + long *longKey; + double *doubleKey; + int keyType; + if (reindex_numeric_keys) { + longKey = emalloc(sizeof(long)); + doubleKey = emalloc(sizeof(double)); + } + IS_CONSISTENT(source); IS_CONSISTENT(target); @@ -826,11 +833,21 @@ ZEND_API void zend_hash_copy(HashTable *target, HashTable *source, copy_ctor_fun if (setTargetPointer && source->pInternalPointer == p) { target->pInternalPointer = NULL; } - if (p->nKeyLength) { + + if (p->nKeyLength && ( ! reindex_numeric_keys || + ! (keyType = is_numeric_string(p->arKey, p->nKeyLength, longKey, doubleKey, -1)))) { + zend_hash_quick_update(target, p->arKey, p->nKeyLength, p->h, p->pData, size, &new_entry); - } else { + } else if ( ! p->nKeyLength) { zend_hash_index_update(target, p->h, p->pData, size, &new_entry); + } else { + if (keyType == IS_DOUBLE) { + *longKey = zend_dval_to_lval(*doubleKey); + } + + zend_hash_index_update(target, *longKey, p->pData, size, &new_entry); } + if (pCopyConstructor) { pCopyConstructor(new_entry); } @@ -841,6 +858,10 @@ ZEND_API void zend_hash_copy(HashTable *target, HashTable *source, copy_ctor_fun } } +ZEND_API inline void zend_hash_copy(HashTable *target, HashTable *source, copy_ctor_func_t pCopyConstructor, void *tmp, uint size) +{ + zend_hash_copy_ex(target, source, pCopyConstructor, tmp, size, 0); +} ZEND_API void _zend_hash_merge(HashTable *target, HashTable *source, copy_ctor_func_t pCopyConstructor, void *tmp, uint size, int overwrite ZEND_FILE_LINE_DC) { diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 5c84deb2..55ab2c2 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -709,7 +709,7 @@ ZEND_API void convert_to_array(zval *op) /* {{{ */ } else if (Z_OBJ_HT_P(op)->get_properties) { HashTable *obj_ht = Z_OBJ_HT_P(op)->get_properties(op TSRMLS_CC); if (obj_ht) { - zend_hash_copy(ht, obj_ht, (copy_ctor_func_t) zval_add_ref, (void *) &tmp, sizeof(zval *)); + zend_hash_copy_ex(ht, obj_ht, (copy_ctor_func_t) zval_add_ref, (void *) &tmp, sizeof(zval *), 1); } } else { convert_object_to_type(op, IS_ARRAY, convert_to_array); ------------------------------------------------------------------------ [2013-11-29 01:52:34] phpmpan at mpan dot pl It seems that: zend_hash_copy blindly copies string key from the source object's hash table to the target array's hash table. Numeric string keys are converted to numbers on array access and zend_hash_index_find is used, which chooses wrong hash, wrong nIndex is produced and finally wrong arBucket is choosen, which leads to the element not being found. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=66173 -- Edit this bug report at https://bugs.php.net/bug.php?id=66173&edit=1