PHPC:2498: Accept integer types for Document array access #1781
Conversation
jmikola
force-pushed
the
1.x-phpc-2498
branch
from
ca423d7 to
f8925ff
Compare
| @@ -204,10 +234,8 @@ static PHP_METHOD(MongoDB_BSON_Document, get) | |||
|
|
|||
| intern = Z_DOCUMENT_OBJ_P(getThis()); | |||
|
|
|||
| if (!php_phongo_document_get(intern, key, key_len, return_value, false)) { | |||
| // Exception already thrown | |||
| RETURN_NULL(); | |||
There was a problem hiding this comment.
This was redundant, as null is already the default value for the return_value zval.
| } | ||
|
|
||
| zend_string* tmp_str; | ||
| zend_string* str = zval_try_get_tmp_string(key, &tmp_str); |
There was a problem hiding this comment.
C99 allows declarations anywhere, but if there is a concern I can position these at the top of the function for consistency (granted, I don't think we have anything to enforce that moving forward since it's no longer a build error).
| @@ -192,6 +192,36 @@ static bool php_phongo_document_get(php_phongo_document_t* intern, char* key, si | |||
| return true; | |||
| } | |||
|
|
|||
| static bool php_phongo_document_get_by_zval(php_phongo_document_t* intern, zval* key, zval* return_value, bool null_if_missing) | |||
There was a problem hiding this comment.
I created two by_zval variants for get and has, which are used by the ArrayAccess methods and dimension handlers. These both abstraction the zval_try_get_tmp_string logic, which was necessary to avoid possibly modifying the zval argument with a basic cast (see: Casts and Operations).
I also added a test for this using an assigned variable instead of a literal to assert that we don't modify anything inadvertently.
| // Use a variable to assert that conversion doesn't affect the original zval | ||
| $key = 1; |
There was a problem hiding this comment.
Interesting, how the variable could have been updated?
There was a problem hiding this comment.
Since we are operating directly on a zval, calling ZVAL_NULL(key) would update the variable, even when it wasn't passed by reference. There's no good reason to do this on purpose (despite it being used in the legacy driver), but it can happen by accident. I believe the last occurrence of us wrongly messing with the key when we were inadvertently freeing interned strings that occurred in the type map. We've gotten more careful since.
There was a problem hiding this comment.
Manager::__construct() is another good example of this, where we need to separate the zvals to ensure any conversions don't inadvertently change the input arguments.
This is also discussed in the Casts and Operations article I linked in #1781 (comment)
1.21.0 What's Changed * PHPC-2343: Require PHP 8.1 by @alcaeus in mongodb/mongo-php-driver#1631 * Update branch names for GHA workflows by @alcaeus in mongodb/mongo-php-driver#1646 * Bump version to 1.21-dev by @alcaeus in mongodb/mongo-php-driver#1661 * Document how to run part of the test suite by @GromNaN in mongodb/mongo-php-driver#1690 * PHPC-2458: Deprecate float arg for UTCDateTime constructor by @jmikola in mongodb/mongo-php-driver#1695 * PHPC-2464: Emit deprecation notice for negative "limit" Query option by @jmikola in mongodb/mongo-php-driver#1710 * PHPC-2460: Use zend_zval_type_name instead of internal macros by @jmikola in mongodb/mongo-php-driver#1714 * PHPC-1247: Update links to PHP.net docs by @jmikola in mongodb/mongo-php-driver#1735 * Test PPC and Zseries on RHEL 9 by @alcaeus in mongodb/mongo-php-driver#1740 * PHPC-2472: Update to libmongocrypt 1.12.0 by @jmikola in mongodb/mongo-php-driver#1743 * PHPC-2473: Bump to libmongoc 1.29.0 by @jmikola in mongodb/mongo-php-driver#1748 * PHPC-2471: Sync BSON corpus tests for Y10K date parsing by @jmikola in mongodb/mongo-php-driver#1742 * Use correct arch name in Windows artifacts. by @mickverm in mongodb/mongo-php-driver#1762 * PHPC-2499: Update to libmongoc 1.29.2 by @jmikola in mongodb/mongo-php-driver#1766 * PHPC-2496: WriteException stub should inherit ServerException by @jmikola in mongodb/mongo-php-driver#1769 * PHPC-2477: Remove unused libmongoc and libbson constants by @jmikola in mongodb/mongo-php-driver#1768 * PHPC-2502: Remove XFAIL in server-executeQuery-012.phpt by @jmikola in mongodb/mongo-php-driver#1772 * PHPC-2489: Deprecate passing WriteConcern and ReadPreference objects to execute methods by @jmikola in mongodb/mongo-php-driver#1770 * PHPC-2501: Conditionally define MONGOC_CYRUS_PLUGIN_PATH_PREFIX on Windows by @jmikola in mongodb/mongo-php-driver#1777 * PHPC-2503: Conditionally allow more concise output for libbson 1.30 by @jmikola in mongodb/mongo-php-driver#1779 * PHPC:2498: Accept integer types for Document array access by @jmikola in mongodb/mongo-php-driver#1781 * PHPC-2508 and PHPC-2506: Bump libmongoc to 1.30 by @jmikola in mongodb/mongo-php-driver#1785 * PHPC-2513: Check for returned _id before appending insert by @jmikola in mongodb/mongo-php-driver#1788 * PHPC-2519: Bump libmongoc to 1.30.1 by @jmikola in mongodb/mongo-php-driver#1792 * PHPC-2518: Support sort option for updateOne and replaceOne by @jmikola in mongodb/mongo-php-driver#1794 * Use pre-packaged-source for pie downloads by @asgrim in mongodb/mongo-php-driver#1782 New Contributors * @asgrim made their first contribution in mongodb/mongo-php-driver#1782
https://jira.mongodb.org/browse/PHPC-2498