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.

Author: jmikola

php_phongo.c

@@ -3486,6 +3486,13 @@ static zend_class_entry* php_phongo_fetch_internal_class(const char* class_name,
 	return NULL;
 }
 
+static HashTable* php_phongo_std_get_gc(zval* object, zval** table, int* n) /* {{{ */
+{
+	*table = NULL;
+	*n = 0;
+	return zend_std_get_properties(object);
+} /* }}} */
+
 /* {{{ PHP_MINIT_FUNCTION */
 PHP_MINIT_FUNCTION(mongodb)
 {
@@ -3501,14 +3508,12 @@ PHP_MINIT_FUNCTION(mongodb)
 
 	/* Prep default object handlers to be used when we register the classes */
 	memcpy(&phongo_std_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
+	/* Disable cloning by default. Individual classes can opt in if they need to
+	 * support this (e.g. BSON objects). */
 	phongo_std_object_handlers.clone_obj = NULL;
-	/*
-	phongo_std_object_handlers.get_debug_info       = NULL;
-	phongo_std_object_handlers.compare_objects      = NULL;
-	phongo_std_object_handlers.cast_object          = NULL;
-	phongo_std_object_handlers.count_elements       = NULL;
-	phongo_std_object_handlers.get_closure          = NULL;
-	*/
+	/* Ensure that get_gc delegates to zend_std_get_properties directly in case
+	 * our class defines a get_properties handler for debugging purposes. */
+	phongo_std_object_handlers.get_gc = php_phongo_std_get_gc;
 
 	/* Initialize zend_class_entry dependencies.
 	 *

src/BSON/Binary.c

@@ -349,14 +349,6 @@ static int php_phongo_binary_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return zend_binary_strcmp(intern1->data, intern1->data_len, intern2->data, intern2->data_len);
 } /* }}} */
 
-static HashTable* php_phongo_binary_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_BINARY_OBJ_P(object)->properties;
-} /* }}} */
-
 static HashTable* php_phongo_binary_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_binary_t* intern;
@@ -413,7 +405,6 @@ void php_phongo_binary_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_binary.clone_obj       = php_phongo_binary_clone_object;
 	php_phongo_handler_binary.compare_objects = php_phongo_binary_compare_objects;
 	php_phongo_handler_binary.get_debug_info  = php_phongo_binary_get_debug_info;
-	php_phongo_handler_binary.get_gc          = php_phongo_binary_get_gc;
 	php_phongo_handler_binary.get_properties  = php_phongo_binary_get_properties;
 	php_phongo_handler_binary.free_obj        = php_phongo_binary_free_object;
 	php_phongo_handler_binary.offset          = XtOffsetOf(php_phongo_binary_t, std);

src/BSON/DBPointer.c

@@ -262,14 +262,6 @@ static int php_phongo_dbpointer_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return strcmp(intern1->id, intern2->id);
 } /* }}} */
 
-static HashTable* php_phongo_dbpointer_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_DBPOINTER_OBJ_P(object)->properties;
-} /* }}} */
-
 HashTable* php_phongo_dbpointer_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_dbpointer_t* intern;
@@ -324,7 +316,6 @@ void php_phongo_dbpointer_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_dbpointer.clone_obj       = php_phongo_dbpointer_clone_object;
 	php_phongo_handler_dbpointer.compare_objects = php_phongo_dbpointer_compare_objects;
 	php_phongo_handler_dbpointer.get_debug_info  = php_phongo_dbpointer_get_debug_info;
-	php_phongo_handler_dbpointer.get_gc          = php_phongo_dbpointer_get_gc;
 	php_phongo_handler_dbpointer.get_properties  = php_phongo_dbpointer_get_properties;
 	php_phongo_handler_dbpointer.free_obj        = php_phongo_dbpointer_free_object;
 	php_phongo_handler_dbpointer.offset          = XtOffsetOf(php_phongo_dbpointer_t, std);

src/BSON/Decimal128.c

@@ -276,14 +276,6 @@ static zend_object* php_phongo_decimal128_clone_object(zval* object) /* {{{ */
 	return new_object;
 } /* }}} */
 
-static HashTable* php_phongo_decimal128_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_DECIMAL128_OBJ_P(object)->properties;
-} /* }}} */
-
 static HashTable* php_phongo_decimal128_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_decimal128_t* intern;
@@ -339,7 +331,6 @@ void php_phongo_decimal128_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	memcpy(&php_phongo_handler_decimal128, phongo_get_std_object_handlers(), sizeof(zend_object_handlers));
 	php_phongo_handler_decimal128.clone_obj      = php_phongo_decimal128_clone_object;
 	php_phongo_handler_decimal128.get_debug_info = php_phongo_decimal128_get_debug_info;
-	php_phongo_handler_decimal128.get_gc         = php_phongo_decimal128_get_gc;
 	php_phongo_handler_decimal128.get_properties = php_phongo_decimal128_get_properties;
 	php_phongo_handler_decimal128.free_obj       = php_phongo_decimal128_free_object;
 	php_phongo_handler_decimal128.offset         = XtOffsetOf(php_phongo_decimal128_t, std);

src/BSON/Int64.c

@@ -242,14 +242,6 @@ static int php_phongo_int64_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return 0;
 } /* }}} */
 
-static HashTable* php_phongo_int64_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_INT64_OBJ_P(object)->properties;
-} /* }}} */
-
 HashTable* php_phongo_int64_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_int64_t* intern;
@@ -302,7 +294,6 @@ void php_phongo_int64_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_int64.clone_obj       = php_phongo_int64_clone_object;
 	php_phongo_handler_int64.compare_objects = php_phongo_int64_compare_objects;
 	php_phongo_handler_int64.get_debug_info  = php_phongo_int64_get_debug_info;
-	php_phongo_handler_int64.get_gc          = php_phongo_int64_get_gc;
 	php_phongo_handler_int64.get_properties  = php_phongo_int64_get_properties;
 	php_phongo_handler_int64.free_obj        = php_phongo_int64_free_object;
 	php_phongo_handler_int64.offset          = XtOffsetOf(php_phongo_int64_t, std);

src/BSON/Javascript.c

@@ -380,14 +380,6 @@ static int php_phongo_javascript_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return strcmp(intern1->code, intern2->code);
 } /* }}} */
 
-static HashTable* php_phongo_javascript_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_JAVASCRIPT_OBJ_P(object)->properties;
-} /* }}} */
-
 HashTable* php_phongo_javascript_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_javascript_t* intern;
@@ -462,7 +454,6 @@ void php_phongo_javascript_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_javascript.clone_obj       = php_phongo_javascript_clone_object;
 	php_phongo_handler_javascript.compare_objects = php_phongo_javascript_compare_objects;
 	php_phongo_handler_javascript.get_debug_info  = php_phongo_javascript_get_debug_info;
-	php_phongo_handler_javascript.get_gc          = php_phongo_javascript_get_gc;
 	php_phongo_handler_javascript.get_properties  = php_phongo_javascript_get_properties;
 	php_phongo_handler_javascript.free_obj        = php_phongo_javascript_free_object;
 	php_phongo_handler_javascript.offset          = XtOffsetOf(php_phongo_javascript_t, std);

src/BSON/ObjectId.c

@@ -326,14 +326,6 @@ static int php_phongo_objectid_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return strcmp(intern1->oid, intern2->oid);
 } /* }}} */
 
-static HashTable* php_phongo_objectid_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_OBJECTID_OBJ_P(object)->properties;
-} /* }}} */
-
 static HashTable* php_phongo_objectid_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_objectid_t* intern;
@@ -387,7 +379,6 @@ void php_phongo_objectid_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_objectid.clone_obj       = php_phongo_objectid_clone_object;
 	php_phongo_handler_objectid.compare_objects = php_phongo_objectid_compare_objects;
 	php_phongo_handler_objectid.get_debug_info  = php_phongo_objectid_get_debug_info;
-	php_phongo_handler_objectid.get_gc          = php_phongo_objectid_get_gc;
 	php_phongo_handler_objectid.get_properties  = php_phongo_objectid_get_properties;
 	php_phongo_handler_objectid.free_obj        = php_phongo_objectid_free_object;
 	php_phongo_handler_objectid.offset          = XtOffsetOf(php_phongo_objectid_t, std);

src/BSON/Regex.c

@@ -360,14 +360,6 @@ static int php_phongo_regex_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return strcmp(intern1->flags, intern2->flags);
 } /* }}} */
 
-static HashTable* php_phongo_regex_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_REGEX_OBJ_P(object)->properties;
-} /* }}} */
-
 static HashTable* php_phongo_regex_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_regex_t* intern;
@@ -424,7 +416,6 @@ void php_phongo_regex_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_regex.clone_obj       = php_phongo_regex_clone_object;
 	php_phongo_handler_regex.compare_objects = php_phongo_regex_compare_objects;
 	php_phongo_handler_regex.get_debug_info  = php_phongo_regex_get_debug_info;
-	php_phongo_handler_regex.get_gc          = php_phongo_regex_get_gc;
 	php_phongo_handler_regex.get_properties  = php_phongo_regex_get_properties;
 	php_phongo_handler_regex.free_obj        = php_phongo_regex_free_object;
 	php_phongo_handler_regex.offset          = XtOffsetOf(php_phongo_regex_t, std);

src/BSON/Symbol.c

@@ -233,14 +233,6 @@ static int php_phongo_symbol_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return strcmp(intern1->symbol, intern2->symbol);
 } /* }}} */
 
-static HashTable* php_phongo_symbol_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_SYMBOL_OBJ_P(object)->properties;
-} /* }}} */
-
 HashTable* php_phongo_symbol_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_symbol_t* intern;
@@ -293,7 +285,6 @@ void php_phongo_symbol_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_symbol.clone_obj       = php_phongo_symbol_clone_object;
 	php_phongo_handler_symbol.compare_objects = php_phongo_symbol_compare_objects;
 	php_phongo_handler_symbol.get_debug_info  = php_phongo_symbol_get_debug_info;
-	php_phongo_handler_symbol.get_gc          = php_phongo_symbol_get_gc;
 	php_phongo_handler_symbol.get_properties  = php_phongo_symbol_get_properties;
 	php_phongo_handler_symbol.free_obj        = php_phongo_symbol_free_object;
 	php_phongo_handler_symbol.offset          = XtOffsetOf(php_phongo_symbol_t, std);

src/BSON/Timestamp.c

@@ -395,14 +395,6 @@ static int php_phongo_timestamp_compare_objects(zval* o1, zval* o2) /* {{{ */
 	return 0;
 } /* }}} */
 
-static HashTable* php_phongo_timestamp_get_gc(zval* object, zval** table, int* n) /* {{{ */
-{
-	*table = NULL;
-	*n     = 0;
-
-	return Z_TIMESTAMP_OBJ_P(object)->properties;
-} /* }}} */
-
 static HashTable* php_phongo_timestamp_get_properties_hash(zval* object, bool is_debug) /* {{{ */
 {
 	php_phongo_timestamp_t* intern;
@@ -466,7 +458,6 @@ void php_phongo_timestamp_init_ce(INIT_FUNC_ARGS) /* {{{ */
 	php_phongo_handler_timestamp.clone_obj       = php_phongo_timestamp_clone_object;
 	php_phongo_handler_timestamp.compare_objects = php_phongo_timestamp_compare_objects;
 	php_phongo_handler_timestamp.get_debug_info  = php_phongo_timestamp_get_debug_info;
-	php_phongo_handler_timestamp.get_gc          = php_phongo_timestamp_get_gc;
 	php_phongo_handler_timestamp.get_properties  = php_phongo_timestamp_get_properties;
 	php_phongo_handler_timestamp.free_obj        = php_phongo_timestamp_free_object;
 	php_phongo_handler_timestamp.offset          = XtOffsetOf(php_phongo_timestamp_t, std);