Fix #80575: Casting mysqli to array produces null on every field

[cmb69]

We implement a get_properties_for handler for casting to array, and
also use this instead of the get_debug_info handler.

[kingx619]

Will the patch also work in PHP 8.0.1 and upcoming versions or just the Long Term Support versions????

[cmb69]

@kingx619, see https://wiki.php.net/vcs/gitworkflow#bugfix_workflow_for_core_developers.

[nikic]

@derrabus Does Symfony Dumper need (array) to work, or would a __debugInfo() implementation also work?

[kamil-tekiela]

Will this also fix mysqli_stmt and mysqli_result casts to array?

[cmb69]

Will this also fix mysqli_stmt and mysqli_result casts to array?

Oh, indeed, it would.

[derrabus]

@nikic If __debugInfo() is implemented, VarDumper should attempt to call it, but the array cast will be performed either way.

https://github.com/symfony/symfony/blob/649fa3f8cd42ad928fb573ebe34f4aff57fe9d26/src/Symfony/Component/VarDumper/Caster/Caster.php#L49-L58

If the __debugInfo() call returned an array, the results of the array cast and the method call will be merged.

https://github.com/symfony/symfony/blob/649fa3f8cd42ad928fb573ebe34f4aff57fe9d26/src/Symfony/Component/VarDumper/Caster/Caster.php#L94-L106

[cmb69]

Any objections on merging this?

[nikic]

@cmb69 Per @derrabus comment specifying __debugInfo() should also work, so we should replace get_debug_info with __debugInfo(). This will use a standard mechanism, rather than adding more internal magic, and also fix other issues (e.g. the fact that inheriting from mysqli and overwriting __debugInfo() yourself will not do anything.)

[cmb69]

I have implemented mysqli::__debugInfo() now, but that would need to be implemented for the other mysqli classes as well, wouldn't it? Also I wonder whether this would still be okay for PHP-7.4.

[derrabus]

Implementing __debugInfo() would help VarDumper, but then again: The current behavior of (array) $mysqli is odd and unexpected: I get an array where only the property names are mapped (as keys) but not their values. I don't believe that this is the desired behavior here.

[cmb69]

So how to proceed? Would it make sense to only apply the fix for the array cast (first commit) to PHP-7.4+, and to implement __debugInfo() for PHP 8.0 or 8.1?

[nikic]

To be clear, what I had in mind is implementing only __debugInfo and dropping the internal get_debug_info/get_properties_for handler

[nikic]

To put it in userland terms, mysqli is an object that implements a bunch of properties via __get(). You would not expect an object that provides properties via __get() to return them from an (array) cast -- in fact, this possibility simply does not exist for userland code. You would instead expect it to report these properties through __debugInfo().

ZEND_PROP_PURPOSE_ARRAY_CAST exists for backwards-compatibility in cases where code has been overloading (array) for a long time, and people have come to rely on it. We should avoid introducing new usages of it, because it creates incorrect expectations. If you make the (array) cast work, then next people ask why the array cast returns different values than get_object_vars().

At least that's my view on various "property-table" overloading in internal classes.

[cmb69]

Thanks for the explanation, @nikic, and I generally agree, but aren't the properties declared in this case:

php-src/ext/mysqli/mysqli.c

Lines 628 to 647 in 38ad37a

	MYSQLI_ADD_PROPERTIES(&mysqli_link_properties, mysqli_link_property_entries);
	zend_declare_property_null(ce, "affected_rows", sizeof("affected_rows") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "client_info", sizeof("client_info") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "client_version", sizeof("client_version") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "connect_errno", sizeof("connect_errno") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "connect_error", sizeof("connect_error") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "errno", sizeof("errno") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "error", sizeof("error") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "error_list", sizeof("error_list") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "field_count", sizeof("field_count") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "host_info", sizeof("host_info") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "info", sizeof("info") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "insert_id", sizeof("insert_id") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "server_info", sizeof("server_info") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "server_version", sizeof("server_version") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "sqlstate", sizeof("sqlstate") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "protocol_version", sizeof("protocol_version") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "thread_id", sizeof("thread_id") - 1, ZEND_ACC_PUBLIC);
	zend_declare_property_null(ce, "warning_count", sizeof("warning_count") - 1, ZEND_ACC_PUBLIC);
	zend_hash_add_ptr(&classes, ce->name, &mysqli_link_properties);

As I understand it, mysqli mixed declared properties with dynamic getters/setters. :/

[kamil-tekiela]

@cmb69 It's a mix of constants, static properties and functions hidden behind __get(). I am not sure if any of them truly is an instance property.

[cmb69]

@kamil-tekiela, well, they are declared as properties with value NULL to the engine. This is why casting to array (which is not especially catered to) yields these NULL values.

Anyhow, suggestions on how to proceed here are welcome!

[kamil-tekiela]

Below I have classified all properties from the mysqli extension. They are all documented as instance properties, but internally they are static, instance, constant or dynamic properties.

mysqli

Property	Type
affected_rows	__get
connect_errno	static
connect_error	static
errno	__get
error_list	__get
error	__get
field_count	__get
client_info	constant
client_version	constant
host_info	__get
protocol_version	__get
server_info	__get
server_version	__get
info	__get
insert_id	__get
sqlstate	__get
thread_id	__get
warning_count	__get

mysqli_stmt

Property	Type
affected_rows	__get
errno	__get
error_list	__get
error	__get
field_count	__get
insert_id	__get
num_rows	__get
param_count	__get
sqlstate	__get

mysqli_result

Property	Type
current_field	__get
field_count	__get
lengths	__get
num_rows	__get

mysqli_driver

Property	Type
client_info	constant
client_version	constant
driver_version	constant
embedded	non-existent
reconnect	static
report_mode	static

mysqli_warning

Property	Type
message	__get
sqlstate	__get
errno	__get

mysqli_sql_exception

Property	Type
sqlstate	instance

The static and constant ones are still implemented via __get magical method. It would be nice if they were implemented properly.

To be honest, I don't know how to handle casting these objects to an array, or json_encode() for that matter. Since they are documented as instance properties, users might expect them to be available when cast to an array. From the implementation point of view, it does not make sense.

Therefore, I would propose to handle array cast the same as json_encode. Return an empty array for all objects when cast to an array.
As a separate step, in PHP 8.1 or 8.2, we can implement __debugInfo(). I'm not sure that this is truly needed though.

[Girgias]

Is this still relevant, or have the properties been properly declared now? If yes, a rebase seems in order.