From 5a2b251610bc3ca72f96cd8d9bf39c95d17e29ad Mon Sep 17 00:00:00 2001
From: Calvin Buckley <calvin@cmpct.info>
Date: Thu, 31 Aug 2023 16:14:30 -0300
Subject: [PATCH] Fix persistent procedural ODBC connections not getting closed

Like oci8, procedural ODBC uses an apply function on the hash list to
enumerate persistent connections and close the specific one. However,
this function take zvals, not resources. However, it was getting casted
as such, causing it to interpret the pointer incorrectly. This could
have caused other issues, but mostly manifested as failing to close the
connection even fi it matched.

The function now takes a zval and gets the resource from that. In
addition, it also removes the cast of the function pointer and moves
casting to the function body, to avoid possible confusion like this in
refactors again. It also cleans up style and uses constants in the
function body.

Closes GH-12132

Signed-off-by: George Peter Banyard <girgias@php.net>
---
 NEWS                                      |  2 +
 ext/odbc/php_odbc.c                       | 16 +++---
 ext/odbc/tests/odbc_persistent_close.phpt | 65 +++++++++++++++++++++++
 3 files changed, 76 insertions(+), 7 deletions(-)
 create mode 100644 ext/odbc/tests/odbc_persistent_close.phpt

diff --git a/NEWS b/NEWS
index ae734998d22..568ce7c0f09 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,8 @@ PHP                                                                        NEWS
 
 - ODBC:
   . Fixed memory leak with failed SQLPrepare. (NattyNarwhal)
+  . Fixed persistent procedural ODBC connections not getting closed.
+    (NattyNarwhal)
 
 - SPL:
   . Fixed bug GH-11972 (RecursiveCallbackFilterIterator regression in 8.1.18).
diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c
index 71fd3979002..b5e91da8806 100644
--- a/ext/odbc/php_odbc.c
+++ b/ext/odbc/php_odbc.c
@@ -753,12 +753,14 @@ void odbc_transact(INTERNAL_FUNCTION_PARAMETERS, int type)
 /* }}} */
 
 /* {{{ _close_pconn_with_res */
-static int _close_pconn_with_res(zend_resource *le, zend_resource *res)
+static int _close_pconn_with_res(zval *zv, void *p)
 {
-	if (le->type == le_pconn && (((odbc_connection *)(le->ptr))->res == res)){
-		return 1;
-	}else{
-		return 0;
+	zend_resource *le = Z_RES_P(zv);
+	zend_resource *res = (zend_resource*)p;
+	if (le->type == le_pconn && (((odbc_connection *)(le->ptr))->res == res)) {
+		return ZEND_HASH_APPLY_REMOVE;
+	} else {
+		return ZEND_HASH_APPLY_KEEP;
 	}
 }
 /* }}} */
@@ -837,7 +839,7 @@ PHP_FUNCTION(odbc_close_all)
 				zend_list_close(p);
 				/* Delete the persistent connection */
 				zend_hash_apply_with_argument(&EG(persistent_list),
-					(apply_func_arg_t) _close_pconn_with_res, (void *)p);
+					_close_pconn_with_res, (void *)p);
 			}
 		}
 	} ZEND_HASH_FOREACH_END();
@@ -2365,7 +2367,7 @@ PHP_FUNCTION(odbc_close)
 	zend_list_close(Z_RES_P(pv_conn));
 
 	if(is_pconn){
-		zend_hash_apply_with_argument(&EG(persistent_list),	(apply_func_arg_t) _close_pconn_with_res, (void *) Z_RES_P(pv_conn));
+		zend_hash_apply_with_argument(&EG(persistent_list), _close_pconn_with_res, (void *) Z_RES_P(pv_conn));
 	}
 }
 /* }}} */
diff --git a/ext/odbc/tests/odbc_persistent_close.phpt b/ext/odbc/tests/odbc_persistent_close.phpt
new file mode 100644
index 00000000000..bee936bf959
--- /dev/null
+++ b/ext/odbc/tests/odbc_persistent_close.phpt
@@ -0,0 +1,65 @@
+--TEST--
+odbc_pconnect(): Make sure closing a persistent connection works
+--EXTENSIONS--
+odbc
+--SKIPIF--
+<?php
+
+include 'skipif.inc';
+
+// The test can affect multiple drivers, but testing it is driver-specific.
+// Since CI runs ODBC tests with SQL Server, just use an SQL Server specific way of testing.
+$conn = odbc_connect($dsn, $user, $pass);
+$result = @odbc_exec($conn, "SELECT @@Version");
+if ($result) {
+    $array = odbc_fetch_array($result);
+    $info = (string) reset($array);
+    if (!str_contains($info, "Microsoft SQL Server")) {
+       echo "skip MS SQL specific test";
+    }
+}
+?>
+--FILE--
+<?php
+
+include 'config.inc';
+
+// set a session specific variable via CONTEXT_INFO, if we get the same connection again, it should be identical
+function set_context_info($conn, $string) {
+	$hexstring = bin2hex($string);
+	return odbc_exec($conn, "SET CONTEXT_INFO 0x$hexstring");
+}
+
+function fetch_context_info($conn) {
+	$stmt = odbc_exec($conn, "SELECT CONTEXT_INFO() AS CONTEXT_INFO");
+	if (!$stmt) {
+		return false;
+	}
+	$res = odbc_fetch_array($stmt);
+	if ($res) {
+		// this is a binary, so we get a bunch of nulls at the end
+		return $res["CONTEXT_INFO"] ? trim($res["CONTEXT_INFO"]) : null;
+	} else {
+		return false;
+	}
+}
+
+// run 1: set expectations
+$conn = odbc_pconnect($dsn, $user, $pass);
+set_context_info($conn, "PHP odbc_pconnect test");
+var_dump(fetch_context_info($conn));
+
+// run 2: reuse same connection (imagine this is another request)
+$conn = odbc_pconnect($dsn, $user, $pass);
+var_dump(fetch_context_info($conn));
+
+// run 3: close it and see if it's the same connection
+odbc_close($conn);
+$conn = odbc_pconnect($dsn, $user, $pass);
+var_dump(fetch_context_info($conn));
+
+?>
+--EXPECT--
+string(22) "PHP odbc_pconnect test"
+string(22) "PHP odbc_pconnect test"
+NULL