feat(mysqlnd): send COM_RESET_CONNECTION in restart_psession

ext/mysqli/mysqli_mysqlnd.h

@@ -37,6 +37,5 @@
 #define mysqli_stmt_close(c, implicit)	mysqlnd_stmt_close((c), (implicit))
 #define mysqli_free_result(r, implicit)	mysqlnd_free_result((r), (implicit))
 #define mysqli_async_query(c, q, l)		mysqlnd_async_query((c), (q), (l))
-#define mysqli_change_user_silent(c, u, p, d, p_len)   mysqlnd_change_user_ex((c), (u), (p), (d), true, (size_t)(p_len))
 
 #endif

ext/mysqli/mysqli_nonapi.c

@@ -173,14 +173,8 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b
 							mysql->mysql = zend_ptr_stack_pop(&plist->free_links);
 
 							MyG(num_inactive_persistent)--;
-							/* reset variables */
 
-#ifndef MYSQLI_NO_CHANGE_USER_ON_PCONNECT
-							if (!mysqli_change_user_silent(mysql->mysql, username, passwd, dbname, passwd_len)) {
-#else
-							if (!mysql_ping(mysql->mysql)) {
-#endif
-								mysqlnd_restart_psession(mysql->mysql);
+							if (!mysqlnd_restart_psession(mysql->mysql)) {
 								MyG(num_active_persistent)++;
 
 								/* clear error */

ext/mysqli/tests/mysqli_pconn_preserved.phpt

@@ -0,0 +1,93 @@
+--TEST--
+mysqli: reusing a persistent connection re-applies its connection config after reset
+--EXTENSIONS--
+mysqli
+--SKIPIF--
+<?php
+require_once 'skipifconnectfailure.inc';
+?>
+--FILE--
+<?php
+// COM_RESET_CONNECTION reverts the session to server defaults; mysqlnd should
+// then re-apply the connection config, so a reused connection stays configured.
+
+require_once 'connect.inc';
+
+$host = 'p:' . $host;
+
+// Open a persistent connection carrying both a charset and an init command.
+function connect_configured($host, $user, $passwd, $db, $port, $socket) {
+    $link = mysqli_init();
+    mysqli_options($link, MYSQLI_SET_CHARSET_NAME, 'latin1');
+    mysqli_options($link, MYSQLI_INIT_COMMAND, 'SET @init_marker = 42');
+
+    if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) {
+        return false;
+    }
+
+    return $link;
+}
+
+// Establish a persistent handle with the above configuration.
+$link = connect_configured($host, $user, $passwd, $db, $port, $socket);
+
+if (!$link) {
+    printf("[001] Cannot connect\n");
+}
+
+$thread_id = mysqli_thread_id($link);
+
+// Sanity check that the charset was configured.
+if (mysqli_character_set_name($link) !== 'latin1') {
+    printf("[002] Charset should be latin1 at connect, got %s\n", mysqli_character_set_name($link));
+}
+
+// Sanity check that the init command was executed.
+$marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0];
+
+if ($marker != 42) {
+    printf("[003] Init command should have set @init_marker at connect, got %s\n", var_export($marker, true));
+}
+
+// Drift both values away from the configuration, so the checks after reuse prove
+// the config was re-applied by the reset rather than merely carried over.
+mysqli_set_charset($link, 'ascii');
+mysqli_query($link, "SET @init_marker = 99");
+
+// Return the connection to the pool.
+mysqli_close($link);
+
+// Reopen the same persistent connection, which should be reset, but preserve
+// the options from the initial configuration.
+$link = connect_configured($host, $user, $passwd, $db, $port, $socket);
+
+// Compare the thread IDs to ensure the connection was reused.
+if (mysqli_thread_id($link) !== $thread_id) {
+    printf("[004] Expected the pooled connection to be reused\n");
+}
+
+// Check the charset to ensure the configured one is back in effect.
+if (mysqli_character_set_name($link) !== 'latin1') {
+    printf("[005] Charset should be re-applied to latin1, got %s\n", mysqli_character_set_name($link));
+}
+
+// Check the server-side charset to ensure it agrees with the client.
+$server = mysqli_query($link, "SELECT @@session.character_set_connection")->fetch_row()[0];
+
+if ($server !== 'latin1') {
+    printf("[006] Server charset should be latin1, got %s\n", $server);
+}
+
+// Check the session variable to ensure the init command was re-executed.
+$marker = mysqli_query($link, "SELECT @init_marker")->fetch_row()[0];
+
+if ($marker != 42) {
+    printf("[007] Init command should be re-run after reuse, got %s\n", var_export($marker, true));
+}
+
+mysqli_close($link);
+
+echo "done!";
+?>
+--EXPECT--
+done!

ext/mysqli/tests/mysqli_pconn_reset.phpt

@@ -0,0 +1,95 @@
+--TEST--
+mysqli: reusing a persistent connection resets its session state (COM_RESET_CONNECTION)
+--EXTENSIONS--
+mysqli
+--SKIPIF--
+<?php
+require_once 'skipifconnectfailure.inc';
+?>
+--FILE--
+<?php
+// Reusing a pooled persistent connection sends COM_RESET_CONNECTION, which must
+// wipe all existing session state.
+
+require_once 'connect.inc';
+
+$host = 'p:' . $host;
+
+// Establish a persistent handle with some session state.
+$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket);
+
+if (!$link) {
+    printf("[001] Cannot connect\n");
+}
+
+$thread_id = mysqli_thread_id($link);
+
+// Pick a charset the connection does not already use, so the change is observable.
+$default = mysqli_character_set_name($link);
+
+if ($default === 'latin1') {
+    printf("[002] Test needs a default charset other than latin1, got %s\n", $default);
+}
+
+mysqli_query($link, "SET @test_var = 42");
+mysqli_query($link, "CREATE TEMPORARY TABLE test_reset_tmp (id INT)");
+
+mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx");
+mysqli_query($link, "CREATE TABLE test_reset_trx (id INT) ENGINE=InnoDB");
+
+mysqli_begin_transaction($link);
+mysqli_query($link, "INSERT INTO test_reset_trx VALUES (1)");
+
+// Change the charset after connecting, which is not part of the connection config.
+mysqli_set_charset($link, 'latin1');
+
+// Return the connection to the pool.
+mysqli_close($link);
+
+// Reopen the same persistent connection, which should be reset.
+$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket);
+
+// Compare the thread IDs to ensure the connection was reused.
+if (mysqli_thread_id($link) !== $thread_id) {
+    printf("[003] Expected the pooled connection to be reused\n");
+}
+
+// Check the session variable to ensure it is no longer present.
+$v = mysqli_query($link, "SELECT @test_var")->fetch_row()[0];
+
+if ($v !== null) {
+    printf("[004] User variable should be reset, got %s\n", var_export($v, true));
+}
+
+// Check the temporary table to ensure it is no longer present.
+if (@mysqli_query($link, "SELECT 1 FROM test_reset_tmp") !== false) {
+    printf("[005] Temporary table should not exist after reset\n");
+}
+
+// Check to see we are no longer in a transaction.
+$rows = mysqli_query($link, "SELECT COUNT(*) FROM test_reset_trx")->fetch_row()[0];
+
+if ($rows != 0) {
+    printf("[006] Transaction should have been rolled back, found %d row(s)\n", $rows);
+}
+
+// A charset set after connecting is not part of the config, so it is not restored.
+if (mysqli_character_set_name($link) !== $default) {
+    printf("[007] Charset should be reset to %s, got %s\n", $default, mysqli_character_set_name($link));
+}
+
+// mysqlnd's cached charset (used for escaping) must still agree with the server.
+$server = mysqli_query($link, "SELECT @@session.character_set_connection")->fetch_row()[0];
+
+if (mysqli_character_set_name($link) !== $server) {
+    printf("[008] Client charset %s disagrees with server %s\n", mysqli_character_set_name($link), $server);
+}
+
+// Clean up the non-temporary table.
+mysqli_query($link, "DROP TABLE IF EXISTS test_reset_trx");
+mysqli_close($link);
+
+echo "done!";
+?>
+--EXPECT--
+done!

ext/mysqlnd/mysqlnd_commands.c

@@ -143,6 +143,33 @@ MYSQLND_METHOD(mysqlnd_command, ping)(MYSQLND_CONN_DATA * const conn)
 /* }}} */
 
 
+/* {{{ mysqlnd_command::reset_connection */
+static enum_func_status
+MYSQLND_METHOD(mysqlnd_command, reset_connection)(MYSQLND_CONN_DATA * const conn)
+{
+	const func_mysqlnd_protocol_payload_decoder_factory__send_command send_command = conn->payload_decoder_factory->m.send_command;
+	const func_mysqlnd_protocol_payload_decoder_factory__send_command_handle_response send_command_handle_response = conn->payload_decoder_factory->m.send_command_handle_response;
+	enum_func_status ret = FAIL;
+
+	DBG_ENTER("mysqlnd_command::reset_connection");
+
+	ret = send_command(conn->payload_decoder_factory, COM_RESET_CONNECTION, NULL, 0, FALSE,
+					   &conn->state,
+					   conn->error_info,
+					   conn->upsert_status,
+					   conn->stats,
+					   conn->m->send_close,
+					   conn);
+	if (PASS == ret) {
+		ret = send_command_handle_response(conn->payload_decoder_factory, PROT_OK_PACKET, FALSE, COM_RESET_CONNECTION, FALSE,
+										   conn->error_info, conn->upsert_status, &conn->last_message);
+	}
+
+	DBG_RETURN(ret);
+}
+/* }}} */
+
+
 /* {{{ mysqlnd_command::statistics */
 static enum_func_status
 MYSQLND_METHOD(mysqlnd_command, statistics)(MYSQLND_CONN_DATA * const conn, zend_string ** message)
@@ -659,4 +686,5 @@ MYSQLND_CLASS_METHODS_START(mysqlnd_command)
 	MYSQLND_METHOD(mysqlnd_command, stmt_close),
 	MYSQLND_METHOD(mysqlnd_command, enable_ssl),
 	MYSQLND_METHOD(mysqlnd_command, handshake),
+	MYSQLND_METHOD(mysqlnd_command, reset_connection),
 MYSQLND_CLASS_METHODS_END;

ext/mysqlnd/mysqlnd_connection.c

@@ -354,14 +354,37 @@ MYSQLND_METHOD(mysqlnd_conn_data, set_server_option)(MYSQLND_CONN_DATA * const c
 
 
 /* {{{ mysqlnd_conn_data::restart_psession */
-static void
+static enum_func_status
 MYSQLND_METHOD(mysqlnd_conn_data, restart_psession)(MYSQLND_CONN_DATA * conn)
 {
 	DBG_ENTER("mysqlnd_conn_data::restart_psession");
-	MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_CONNECT_REUSED);
-	conn->current_result = NULL;
-	conn->last_message.s = NULL;
-	DBG_VOID_RETURN;
+
+	enum_func_status ret = conn->command->reset_connection(conn);
+	if (ret == PASS) {
+		MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_CONNECT_REUSED);
+		conn->current_result = NULL;
+		conn->last_message.s = NULL;
+
+		/* COM_RESET_CONNECTION reverts the session charset to the server
+		 * default. A charset requested as a connection option (DSN charset=,
+		 * MYSQLI_SET_CHARSET_NAME) is part of establishing the connection, so
+		 * re-apply it to match a fresh connect. A charset chosen afterwards
+		 * with set_charset() is not restored - the caller reapplies that
+		 * itself - but realign the cached charset (used for escaping) with the
+		 * now-reset connection. */
+		if (conn->options->charset_name) {
+			ret = conn->m->set_charset(conn, conn->options->charset_name);
+		} else if (conn->greet_charset) {
+			conn->charset = conn->greet_charset;
+		}
+
+		/* Re-execute any MYSQL_INIT_COMMAND commands. */
+		if (ret == PASS) {
+			ret = conn->m->execute_init_commands(conn);
+		}
+	}
+
+	DBG_RETURN(ret);
 }
 /* }}} */
 

ext/mysqlnd/mysqlnd_structs.h

@@ -306,6 +306,7 @@ typedef enum_func_status (*func_mysqlnd_execute_com_set_option)(MYSQLND_CONN_DAT
 typedef enum_func_status (*func_mysqlnd_execute_com_debug)(MYSQLND_CONN_DATA * const conn);
 typedef enum_func_status (*func_mysqlnd_execute_com_init_db)(MYSQLND_CONN_DATA * const conn, const MYSQLND_CSTRING db);
 typedef enum_func_status (*func_mysqlnd_execute_com_ping)(MYSQLND_CONN_DATA * const conn);
+typedef enum_func_status (*func_mysqlnd_execute_com_reset_connection)(MYSQLND_CONN_DATA * const conn);
 typedef enum_func_status (*func_mysqlnd_execute_com_statistics)(MYSQLND_CONN_DATA * const conn, zend_string ** message);
 typedef enum_func_status (*func_mysqlnd_execute_com_process_kill)(MYSQLND_CONN_DATA * const conn, const unsigned int process_id, const bool read_response);
 typedef enum_func_status (*func_mysqlnd_execute_com_refresh)(MYSQLND_CONN_DATA * const conn, const uint8_t options);
@@ -344,6 +345,7 @@ MYSQLND_CLASS_METHODS_TYPE(mysqlnd_command)
 	func_mysqlnd_execute_com_stmt_close stmt_close;
 	func_mysqlnd_execute_com_enable_ssl enable_ssl;
 	func_mysqlnd_execute_com_handshake handshake;
+	func_mysqlnd_execute_com_reset_connection reset_connection;
 };
 
 
@@ -482,7 +484,7 @@ typedef enum_func_status	(*func_mysqlnd_conn_data__free_reference)(MYSQLND_CONN_
 typedef enum_func_status	(*func_mysqlnd_conn_data__send_command_do_request)(MYSQLND_CONN_DATA * const conn, const enum php_mysqlnd_server_command command, const zend_uchar * const arg, const size_t arg_len, const bool silent, const bool ignore_upsert_status);
 typedef enum_func_status	(*func_mysqlnd_conn_data__send_command_handle_response)(MYSQLND_CONN_DATA * const conn, const enum mysqlnd_packet_type ok_packet, const bool silent, const enum php_mysqlnd_server_command command, const bool ignore_upsert_status);
 
-typedef void				(*func_mysqlnd_conn_data__restart_psession)(MYSQLND_CONN_DATA * conn);
+typedef enum_func_status	(*func_mysqlnd_conn_data__restart_psession)(MYSQLND_CONN_DATA * conn);
 typedef void				(*func_mysqlnd_conn_data__end_psession)(MYSQLND_CONN_DATA * conn);
 typedef enum_func_status	(*func_mysqlnd_conn_data__send_close)(MYSQLND_CONN_DATA * conn);
 

ext/pdo/pdo_dbh.c

@@ -414,12 +414,37 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 				if (le->type == php_pdo_list_entry()) {
 					pdbh = (pdo_dbh_t*)le->ptr;
 
-					/* is the connection still alive ? */
-					if (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) {
-						/* nope... need to kill it */
-						pdbh->refcount--;
-						zend_list_close(le);
-						pdbh = NULL;
+					if (pdbh->refcount == 1) {
+						/* refcount 1 means no other open handle is using this
+						 * pooled connection, so we can reset its server-side
+						 * session state before handing it out. reset_connection
+						 * doubles as a liveness check; drivers without it fall
+						 * back to check_liveness. */
+						zend_result alive = SUCCESS;
+						if (pdbh->methods->reset_connection) {
+							alive = (pdbh->methods->reset_connection)(pdbh);
+						} else if (pdbh->methods->check_liveness) {
+							alive = (pdbh->methods->check_liveness)(pdbh);
+						}
+						if (FAILURE == alive) {
+							/* Removing it from the persistent list runs the
+							 * persistent destructor, which closes and frees the
+							 * connection. Only correct because refcount is 1. */
+							zend_hash_del(&EG(persistent_list), hash_key);
+							pdbh = NULL;
+						}
+					} else {
+						/* Another open handle shares this connection, so we must
+						 * not reset it: that would discard session state (open
+						 * transactions, temporary tables, user variables) the
+						 * other handle relies on. If it has died, drop our
+						 * reference and stop reusing it; it is closed once the
+						 * last handle referencing it is released. */
+						if (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) {
+							pdbh->refcount--;
+							zend_list_close(le);
+							pdbh = NULL;
+						}
 					}
 				}
 			}

ext/pdo/php_pdo_driver.h

@@ -32,7 +32,7 @@ struct pdo_bound_param_data;
 # define FALSE 0
 #endif
 
-#define PDO_DRIVER_API	20240423
+#define PDO_DRIVER_API	20260502
 
 /* Doctrine hardcodes these constants, avoid changing their values. */
 enum pdo_param_type {
@@ -268,6 +268,15 @@ typedef int (*pdo_dbh_get_attr_func)(pdo_dbh_t *dbh, zend_long attr, zval *val);
  * You may set this handler to NULL, which is equivalent to returning SUCCESS. */
 typedef zend_result (*pdo_dbh_check_liveness_func)(pdo_dbh_t *dbh);
 
+/* called when a persistent connection is pulled from the pool for reuse, to
+ * reset any server-side session state left over from the previous request.
+ * Return SUCCESS if the connection was reset and is ready for reuse, FAILURE
+ * otherwise (PDO will then discard it). A successful reset also implies the
+ * connection is alive, so when this handler is present PDO does not also call
+ * check_liveness. You may set this handler to NULL, in which case PDO falls
+ * back to check_liveness. */
+typedef zend_result (*pdo_dbh_reset_connection_func)(pdo_dbh_t *dbh);
+
 /* called at request end for each persistent dbh; this gives the driver
  * the opportunity to safely release resources that only have per-request
  * scope */
@@ -313,6 +322,7 @@ struct pdo_dbh_methods {
 	pdo_dbh_txn_func		in_transaction;
 	pdo_dbh_get_gc_func		get_gc;
 	pdo_dbh_sql_scanner		scanner;
+	pdo_dbh_reset_connection_func	reset_connection;
 };
 
 /* }}} */