Skip to content

Commit 354bcc9

Browse files
jmikolajmikola
committed
PHPC-1598: Fix get_gc handlers for classes with get_properties
The ReadConcern, ReadPreference, WriteConcern, and BSON classes have no properties beyond pointers to libmongoc/libbson structs; however, all of these classes use get_properties to report fields for var_export (PHPC-850, PHPC-460). The standard get_gc handler defers to get_properties to collect other zvals for GC inspection. This is problematic for the aforementioned get_properties handlers, since a HashTable of zvals intended for debugging will be returned and those properties will already be freed by our free_object handler (via FREE_HASHTABLE). Having each class define its own get_gc handler is the first step to fixing this issue. The BSON classes already defined their own get_gc handlers, but erroneously returned the internally cached properties directly. The second step to fixing this issue is ensuring that get_gc delegates to zend_std_get_properties, as is done in various PHP core extensions. Doing so ensures that we do not leak other zvals that may be assigned as public properties by the application (covered by the second regression test), since zend_std_get_properties will return those for GC inspection.
1 parent 7055b2e commit 354bcc9

File tree

19 files changed

+370
-97
lines changed

19 files changed

+370
-97
lines changed

‎php_phongo.c‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3486,6 +3486,13 @@ static zend_class_entry* php_phongo_fetch_internal_class(const char* class_name,
34863486
return NULL;
34873487
}
34883488

3489+
static HashTable* php_phongo_std_get_gc(zval* object, zval** table, int* n) /* {{{ */
3490+
{
3491+
*table = NULL;
3492+
*n = 0;
3493+
return zend_std_get_properties(object);
3494+
} /* }}} */
3495+
34893496
/* {{{ PHP_MINIT_FUNCTION */
34903497
PHP_MINIT_FUNCTION(mongodb)
34913498
{
@@ -3501,14 +3508,12 @@ PHP_MINIT_FUNCTION(mongodb)
35013508

35023509
/* Prep default object handlers to be used when we register the classes */
35033510
memcpy(&phongo_std_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
3511+
/* Disable cloning by default. Individual classes can opt in if they need to
3512+
* support this (e.g. BSON objects). */
35043513
phongo_std_object_handlers.clone_obj = NULL;
3505-
/*
3506-
phongo_std_object_handlers.get_debug_info = NULL;
3507-
phongo_std_object_handlers.compare_objects = NULL;
3508-
phongo_std_object_handlers.cast_object = NULL;
3509-
phongo_std_object_handlers.count_elements = NULL;
3510-
phongo_std_object_handlers.get_closure = NULL;
3511-
*/
3514+
/* Ensure that get_gc delegates to zend_std_get_properties directly in case
3515+
* our class defines a get_properties handler for debugging purposes. */
3516+
phongo_std_object_handlers.get_gc = php_phongo_std_get_gc;
35123517

35133518
/* Initialize zend_class_entry dependencies.
35143519
*

‎src/BSON/Binary.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -349,14 +349,6 @@ static int php_phongo_binary_compare_objects(zval* o1, zval* o2) /* {{{ */
349349
return zend_binary_strcmp(intern1->data, intern1->data_len, intern2->data, intern2->data_len);
350350
} /* }}} */
351351

352-
static HashTable* php_phongo_binary_get_gc(zval* object, zval** table, int* n) /* {{{ */
353-
{
354-
*table = NULL;
355-
*n = 0;
356-
357-
return Z_BINARY_OBJ_P(object)->properties;
358-
} /* }}} */
359-
360352
static HashTable* php_phongo_binary_get_properties_hash(zval* object, bool is_debug) /* {{{ */
361353
{
362354
php_phongo_binary_t* intern;
@@ -413,7 +405,6 @@ void php_phongo_binary_init_ce(INIT_FUNC_ARGS) /* {{{ */
413405
php_phongo_handler_binary.clone_obj = php_phongo_binary_clone_object;
414406
php_phongo_handler_binary.compare_objects = php_phongo_binary_compare_objects;
415407
php_phongo_handler_binary.get_debug_info = php_phongo_binary_get_debug_info;
416-
php_phongo_handler_binary.get_gc = php_phongo_binary_get_gc;
417408
php_phongo_handler_binary.get_properties = php_phongo_binary_get_properties;
418409
php_phongo_handler_binary.free_obj = php_phongo_binary_free_object;
419410
php_phongo_handler_binary.offset = XtOffsetOf(php_phongo_binary_t, std);

‎src/BSON/DBPointer.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -262,14 +262,6 @@ static int php_phongo_dbpointer_compare_objects(zval* o1, zval* o2) /* {{{ */
262262
return strcmp(intern1->id, intern2->id);
263263
} /* }}} */
264264

265-
static HashTable* php_phongo_dbpointer_get_gc(zval* object, zval** table, int* n) /* {{{ */
266-
{
267-
*table = NULL;
268-
*n = 0;
269-
270-
return Z_DBPOINTER_OBJ_P(object)->properties;
271-
} /* }}} */
272-
273265
HashTable* php_phongo_dbpointer_get_properties_hash(zval* object, bool is_debug) /* {{{ */
274266
{
275267
php_phongo_dbpointer_t* intern;
@@ -324,7 +316,6 @@ void php_phongo_dbpointer_init_ce(INIT_FUNC_ARGS) /* {{{ */
324316
php_phongo_handler_dbpointer.clone_obj = php_phongo_dbpointer_clone_object;
325317
php_phongo_handler_dbpointer.compare_objects = php_phongo_dbpointer_compare_objects;
326318
php_phongo_handler_dbpointer.get_debug_info = php_phongo_dbpointer_get_debug_info;
327-
php_phongo_handler_dbpointer.get_gc = php_phongo_dbpointer_get_gc;
328319
php_phongo_handler_dbpointer.get_properties = php_phongo_dbpointer_get_properties;
329320
php_phongo_handler_dbpointer.free_obj = php_phongo_dbpointer_free_object;
330321
php_phongo_handler_dbpointer.offset = XtOffsetOf(php_phongo_dbpointer_t, std);

‎src/BSON/Decimal128.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,6 @@ static zend_object* php_phongo_decimal128_clone_object(zval* object) /* {{{ */
276276
return new_object;
277277
} /* }}} */
278278

279-
static HashTable* php_phongo_decimal128_get_gc(zval* object, zval** table, int* n) /* {{{ */
280-
{
281-
*table = NULL;
282-
*n = 0;
283-
284-
return Z_DECIMAL128_OBJ_P(object)->properties;
285-
} /* }}} */
286-
287279
static HashTable* php_phongo_decimal128_get_properties_hash(zval* object, bool is_debug) /* {{{ */
288280
{
289281
php_phongo_decimal128_t* intern;
@@ -339,7 +331,6 @@ void php_phongo_decimal128_init_ce(INIT_FUNC_ARGS) /* {{{ */
339331
memcpy(&php_phongo_handler_decimal128, phongo_get_std_object_handlers(), sizeof(zend_object_handlers));
340332
php_phongo_handler_decimal128.clone_obj = php_phongo_decimal128_clone_object;
341333
php_phongo_handler_decimal128.get_debug_info = php_phongo_decimal128_get_debug_info;
342-
php_phongo_handler_decimal128.get_gc = php_phongo_decimal128_get_gc;
343334
php_phongo_handler_decimal128.get_properties = php_phongo_decimal128_get_properties;
344335
php_phongo_handler_decimal128.free_obj = php_phongo_decimal128_free_object;
345336
php_phongo_handler_decimal128.offset = XtOffsetOf(php_phongo_decimal128_t, std);

‎src/BSON/Int64.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -242,14 +242,6 @@ static int php_phongo_int64_compare_objects(zval* o1, zval* o2) /* {{{ */
242242
return 0;
243243
} /* }}} */
244244

245-
static HashTable* php_phongo_int64_get_gc(zval* object, zval** table, int* n) /* {{{ */
246-
{
247-
*table = NULL;
248-
*n = 0;
249-
250-
return Z_INT64_OBJ_P(object)->properties;
251-
} /* }}} */
252-
253245
HashTable* php_phongo_int64_get_properties_hash(zval* object, bool is_debug) /* {{{ */
254246
{
255247
php_phongo_int64_t* intern;
@@ -302,7 +294,6 @@ void php_phongo_int64_init_ce(INIT_FUNC_ARGS) /* {{{ */
302294
php_phongo_handler_int64.clone_obj = php_phongo_int64_clone_object;
303295
php_phongo_handler_int64.compare_objects = php_phongo_int64_compare_objects;
304296
php_phongo_handler_int64.get_debug_info = php_phongo_int64_get_debug_info;
305-
php_phongo_handler_int64.get_gc = php_phongo_int64_get_gc;
306297
php_phongo_handler_int64.get_properties = php_phongo_int64_get_properties;
307298
php_phongo_handler_int64.free_obj = php_phongo_int64_free_object;
308299
php_phongo_handler_int64.offset = XtOffsetOf(php_phongo_int64_t, std);

‎src/BSON/Javascript.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -380,14 +380,6 @@ static int php_phongo_javascript_compare_objects(zval* o1, zval* o2) /* {{{ */
380380
return strcmp(intern1->code, intern2->code);
381381
} /* }}} */
382382

383-
static HashTable* php_phongo_javascript_get_gc(zval* object, zval** table, int* n) /* {{{ */
384-
{
385-
*table = NULL;
386-
*n = 0;
387-
388-
return Z_JAVASCRIPT_OBJ_P(object)->properties;
389-
} /* }}} */
390-
391383
HashTable* php_phongo_javascript_get_properties_hash(zval* object, bool is_debug) /* {{{ */
392384
{
393385
php_phongo_javascript_t* intern;
@@ -462,7 +454,6 @@ void php_phongo_javascript_init_ce(INIT_FUNC_ARGS) /* {{{ */
462454
php_phongo_handler_javascript.clone_obj = php_phongo_javascript_clone_object;
463455
php_phongo_handler_javascript.compare_objects = php_phongo_javascript_compare_objects;
464456
php_phongo_handler_javascript.get_debug_info = php_phongo_javascript_get_debug_info;
465-
php_phongo_handler_javascript.get_gc = php_phongo_javascript_get_gc;
466457
php_phongo_handler_javascript.get_properties = php_phongo_javascript_get_properties;
467458
php_phongo_handler_javascript.free_obj = php_phongo_javascript_free_object;
468459
php_phongo_handler_javascript.offset = XtOffsetOf(php_phongo_javascript_t, std);

‎src/BSON/ObjectId.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -326,14 +326,6 @@ static int php_phongo_objectid_compare_objects(zval* o1, zval* o2) /* {{{ */
326326
return strcmp(intern1->oid, intern2->oid);
327327
} /* }}} */
328328

329-
static HashTable* php_phongo_objectid_get_gc(zval* object, zval** table, int* n) /* {{{ */
330-
{
331-
*table = NULL;
332-
*n = 0;
333-
334-
return Z_OBJECTID_OBJ_P(object)->properties;
335-
} /* }}} */
336-
337329
static HashTable* php_phongo_objectid_get_properties_hash(zval* object, bool is_debug) /* {{{ */
338330
{
339331
php_phongo_objectid_t* intern;
@@ -387,7 +379,6 @@ void php_phongo_objectid_init_ce(INIT_FUNC_ARGS) /* {{{ */
387379
php_phongo_handler_objectid.clone_obj = php_phongo_objectid_clone_object;
388380
php_phongo_handler_objectid.compare_objects = php_phongo_objectid_compare_objects;
389381
php_phongo_handler_objectid.get_debug_info = php_phongo_objectid_get_debug_info;
390-
php_phongo_handler_objectid.get_gc = php_phongo_objectid_get_gc;
391382
php_phongo_handler_objectid.get_properties = php_phongo_objectid_get_properties;
392383
php_phongo_handler_objectid.free_obj = php_phongo_objectid_free_object;
393384
php_phongo_handler_objectid.offset = XtOffsetOf(php_phongo_objectid_t, std);

‎src/BSON/Regex.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -360,14 +360,6 @@ static int php_phongo_regex_compare_objects(zval* o1, zval* o2) /* {{{ */
360360
return strcmp(intern1->flags, intern2->flags);
361361
} /* }}} */
362362

363-
static HashTable* php_phongo_regex_get_gc(zval* object, zval** table, int* n) /* {{{ */
364-
{
365-
*table = NULL;
366-
*n = 0;
367-
368-
return Z_REGEX_OBJ_P(object)->properties;
369-
} /* }}} */
370-
371363
static HashTable* php_phongo_regex_get_properties_hash(zval* object, bool is_debug) /* {{{ */
372364
{
373365
php_phongo_regex_t* intern;
@@ -424,7 +416,6 @@ void php_phongo_regex_init_ce(INIT_FUNC_ARGS) /* {{{ */
424416
php_phongo_handler_regex.clone_obj = php_phongo_regex_clone_object;
425417
php_phongo_handler_regex.compare_objects = php_phongo_regex_compare_objects;
426418
php_phongo_handler_regex.get_debug_info = php_phongo_regex_get_debug_info;
427-
php_phongo_handler_regex.get_gc = php_phongo_regex_get_gc;
428419
php_phongo_handler_regex.get_properties = php_phongo_regex_get_properties;
429420
php_phongo_handler_regex.free_obj = php_phongo_regex_free_object;
430421
php_phongo_handler_regex.offset = XtOffsetOf(php_phongo_regex_t, std);

‎src/BSON/Symbol.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,6 @@ static int php_phongo_symbol_compare_objects(zval* o1, zval* o2) /* {{{ */
233233
return strcmp(intern1->symbol, intern2->symbol);
234234
} /* }}} */
235235

236-
static HashTable* php_phongo_symbol_get_gc(zval* object, zval** table, int* n) /* {{{ */
237-
{
238-
*table = NULL;
239-
*n = 0;
240-
241-
return Z_SYMBOL_OBJ_P(object)->properties;
242-
} /* }}} */
243-
244236
HashTable* php_phongo_symbol_get_properties_hash(zval* object, bool is_debug) /* {{{ */
245237
{
246238
php_phongo_symbol_t* intern;
@@ -293,7 +285,6 @@ void php_phongo_symbol_init_ce(INIT_FUNC_ARGS) /* {{{ */
293285
php_phongo_handler_symbol.clone_obj = php_phongo_symbol_clone_object;
294286
php_phongo_handler_symbol.compare_objects = php_phongo_symbol_compare_objects;
295287
php_phongo_handler_symbol.get_debug_info = php_phongo_symbol_get_debug_info;
296-
php_phongo_handler_symbol.get_gc = php_phongo_symbol_get_gc;
297288
php_phongo_handler_symbol.get_properties = php_phongo_symbol_get_properties;
298289
php_phongo_handler_symbol.free_obj = php_phongo_symbol_free_object;
299290
php_phongo_handler_symbol.offset = XtOffsetOf(php_phongo_symbol_t, std);

‎src/BSON/Timestamp.c‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -395,14 +395,6 @@ static int php_phongo_timestamp_compare_objects(zval* o1, zval* o2) /* {{{ */
395395
return 0;
396396
} /* }}} */
397397

398-
static HashTable* php_phongo_timestamp_get_gc(zval* object, zval** table, int* n) /* {{{ */
399-
{
400-
*table = NULL;
401-
*n = 0;
402-
403-
return Z_TIMESTAMP_OBJ_P(object)->properties;
404-
} /* }}} */
405-
406398
static HashTable* php_phongo_timestamp_get_properties_hash(zval* object, bool is_debug) /* {{{ */
407399
{
408400
php_phongo_timestamp_t* intern;
@@ -466,7 +458,6 @@ void php_phongo_timestamp_init_ce(INIT_FUNC_ARGS) /* {{{ */
466458
php_phongo_handler_timestamp.clone_obj = php_phongo_timestamp_clone_object;
467459
php_phongo_handler_timestamp.compare_objects = php_phongo_timestamp_compare_objects;
468460
php_phongo_handler_timestamp.get_debug_info = php_phongo_timestamp_get_debug_info;
469-
php_phongo_handler_timestamp.get_gc = php_phongo_timestamp_get_gc;
470461
php_phongo_handler_timestamp.get_properties = php_phongo_timestamp_get_properties;
471462
php_phongo_handler_timestamp.free_obj = php_phongo_timestamp_free_object;
472463
php_phongo_handler_timestamp.offset = XtOffsetOf(php_phongo_timestamp_t, std);

0 commit comments

Comments
 (0)