Bug #75147 [Com]: add_assoc_zval() may still create integer keys in object HashTables
| From: | jmikola@php.net | Date: | Fri, 01 Sep 2017 21:07:06 +0000 |
| Subject: | Bug #75147 [Com]: add_assoc_zval() may still create integer keys in object HashTables | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-210909@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=75147&edit=1
ID: 75147
Comment by: jmikola@php.net
Reported by: jmikola@php.net
Summary: add_assoc_zval() may still create integer keys in
object HashTables
Status: Feedback
Type: Bug
Package: Class/Object related
PHP Version: 7.2.0RC1
Block user comment: N
Private report: N
New Comment:
Thanks for the quick response.
I wrongfully assumed that the zval we passed to add_assoc_zval() was either an array or object.
It's clear to me that there's no way that could work given the struct definitions of
zend_array (basically a HashTable alias) and zend_object (where HashTable is under the fifth
"properties" field).
The argument we pass to add_assoc_zval() is always an array, which we initialize in
php_phongo_bson_visit_[document|array] <https://github.com/mongodb/mongo-php-driver/blob/598e8e2845c5de09a3591276bcb6d328be48a697/src/bson.c#L522>.
It's not until after all of our visitor functions add zvals into the array that we create an
object by calling object_and_properties_init() with the array's HashTable <https://github.com/mongodb/mongo-php-driver/blob/598e8e2845c5de09a3591276bcb6d328be48a697/src/bson.c#L580>.
Looking at Andrea's patch in https://github.com/php/php-src/pull/2142, it
appears we can fix this by utilizing the new zend_symtable_to_proptable() function <https://github.com/php/php-src/commit/a0502b89a65d24eb191a7c85bcffcf9b91454735#diff-fd78a0a3f78ea28c6907f907f25b908eR2484>,
or otherwise casting the array to an object.
Previous Comments:
------------------------------------------------------------------------
[2017-09-01 20:32:56] nikic@php.net
Can you please point to the code where you're using the function? add_assoc_zval() is a
function working on arrays, I'm not sure how you managed to make it work on an object. There
are different functions (the zend_update_property_* and add_property_* families for example) for
working with objects.
------------------------------------------------------------------------
[2017-09-01 20:21:52] jmikola@php.net
Description:
------------
While running the mongodb extension's test suite with PHP 7.2.0RC1, I noticed several new
failures that appear to be related to the BC incompatible changes in https://github.com/php/php-src/blob/php-7.2.0RC1/UPGRADING#L28.
Specifically:
> Casting arrays to objects (with (object) or settype()) will now convert integer keys to string
> property names.
One of the more concise tests that now fail is: https://github.com/mongodb/mongo-php-driver/blob/598e8e2845c5de09a3591276bcb6d328be48a697/tests/bson/bson-utcdatetime-001.phpt#L36
In this test, we create a numerically-indexed array that contains a single zval (a BSON class in
this particular case, but that's not relevant) and convert it to BSON and back to PHP. The
interim BSON representation is a document/object with a "0" string key. When converting
back to PHP, we use add_assoc_zval() with "0" to assign the converted zval back to the
output.
add_assoc_zval() ultimately calls zend_symtable_str_update() <https://github.com/php/php-src/blob/php-7.2.0RC1/Zend/zend_hash.h#L456>,
which then converts our "0" string key to integer 0 via _zend_handle_numeric_str_ex().
Effectively, we end up creating an inaccessible numeric property on the object, which is exactly
what the change in 7.2.0's array->object casting intended to address.
My question is whether add_assoc_zval_ex() should be changed to no longer convert numeric string
keys to integer keys. I'll note that this decision would likely have to be made in
add_assoc_zval_ex() because zend_symtable_str_update() is only provided the HashTable* and by that
point we have no way of determining if the corresponding zval for that HashTable is an array or
object type.
Alternatively, is it the mongodb extension's responsibility to detect this case an manually
call zend_hash_str_update() instead of relying on add_assoc_zval()?
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=75147&edit=1