php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #75147 add_assoc_zval() may still create integer keys in object HashTables
Submitted: 2017-09-01 20:21 UTC Modified: 2017-09-01 21:09 UTC
From: jmikola@php.net Assigned:
Status: Not a bug Package: Class/Object related
PHP Version: 7.2.0RC1 OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: jmikola@php.net
New email:
PHP Version: OS:

 

 [2017-09-01 20:21 UTC] 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()?



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-09-01 20:32 UTC] nikic@php.net
-Status: Open +Status: Feedback
 [2017-09-01 20:32 UTC] 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 21:06 UTC] jmikola@php.net
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.
 [2017-09-01 21:09 UTC] jmikola@php.net
-Status: Feedback +Status: Not a bug
 [2017-09-01 21:09 UTC] jmikola@php.net
Thank you for taking the time to write to us, but this is not
a bug. Please double-check the documentation available at
http://www.php.net/manual/ and the instructions on how to report
a bug at http://bugs.php.net/how-to-report.php

Resolving as "not a bug", as add_assoc_zval() performs exactly as it should.
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Wed Oct 28 15:01:23 2020 UTC