From 4472a93e80a1e71bba4c44b7cad75448031aca93 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 16 Aug 2022 16:25:43 -0300 Subject: [PATCH 1/4] Procedural ODBC: SQL_ATTR_CONNECTION_DEAD This is semantically appropriate and should be used whenever the driver supports it. In the event that it fails or says the connection isn't dead (which may be inaccurate in some cases), try the old heuristic. --- ext/odbc/php_odbc.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 3a821a35ea38c..9bc3b021fbe13 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2246,7 +2246,17 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent) RETCODE ret; UCHAR d_name[32]; SQLSMALLINT len; + SQLUINTEGER dead = SQL_CD_FALSE; + ret = SQLGetConnectAttr(db_conn->hdbc, + SQL_ATTR_CONNECTION_DEAD, + &dead, 0, NULL); + if (dead == SQL_CD_TRUE) { + /* Bail early here, since we know it's gone */ + zend_hash_str_del(&EG(persistent_list), hashed_details, hashed_len); + goto try_and_get_another_connection; + } + /* If the driver doesn't support it, fall back to old heuristic */ ret = SQLGetInfo(db_conn->hdbc, SQL_DATA_SOURCE_READ_ONLY, d_name, sizeof(d_name), &len); From 41595cb9b003bf23b492ab693e49409654380376 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 16 Aug 2022 16:27:32 -0300 Subject: [PATCH 2/4] PDO ODBC: SQL_ATTR_CONNECTION_DEAD This is semantically appropriate and should be used whenever the driver supports it. In the event that it fails or says the connection isn't dead (which may be inaccurate in some cases), try the old heuristic. --- ext/pdo_odbc/odbc_driver.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index 308cf7e10c94d..af9bdd73eabf6 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -396,11 +396,18 @@ static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) RETCODE ret; UCHAR d_name[32]; SQLSMALLINT len; + SQLUINTEGER dead = SQL_CD_FALSE; pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; + ret = SQLGetConnectAttr(H->dbc, SQL_ATTR_CONNECTION_DEAD, &dead, 0, NULL); + if (dead == SQL_CD_TRUE) { + /* Bail early here, since we know it's gone */ + return FAILURE; + } /* - * SQL_ATTR_CONNECTION_DEAD is tempting, but only in ODBC 3.5, - * and not all drivers implement it properly + * If the driver doesn't support SQL_ATTR_CONNECTION_DEAD, fall back + * to using SQL_DATA_SOURCE_READ_ONLY, which isn't semantically + * correct, but works with many drivers. */ ret = SQLGetInfo(H->dbc, SQL_DATA_SOURCE_READ_ONLY, d_name, sizeof(d_name), &len); From 7d527f521733c2bb866809ce725929d843a60b50 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Mon, 22 Aug 2022 12:12:28 -0300 Subject: [PATCH 3/4] Only use old heuristic on failure of new check --- ext/odbc/php_odbc.c | 2 +- ext/pdo_odbc/odbc_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 9bc3b021fbe13..5fd640cb093d7 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2251,7 +2251,7 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent) ret = SQLGetConnectAttr(db_conn->hdbc, SQL_ATTR_CONNECTION_DEAD, &dead, 0, NULL); - if (dead == SQL_CD_TRUE) { + if (ret == SQL_SUCCESS && dead == SQL_CD_TRUE) { /* Bail early here, since we know it's gone */ zend_hash_str_del(&EG(persistent_list), hashed_details, hashed_len); goto try_and_get_another_connection; diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index af9bdd73eabf6..0e1eec114c2cc 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -400,7 +400,7 @@ static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; ret = SQLGetConnectAttr(H->dbc, SQL_ATTR_CONNECTION_DEAD, &dead, 0, NULL); - if (dead == SQL_CD_TRUE) { + if (ret == SQL_SUCCESS && dead == SQL_CD_TRUE) { /* Bail early here, since we know it's gone */ return FAILURE; } From 80190823fc8aa0cd973bacf0540c73261a1eb7fe Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 23 Aug 2022 14:11:28 -0300 Subject: [PATCH 4/4] Adjust comments to clarify that CD_FALSE -> fall back --- ext/odbc/php_odbc.c | 5 ++++- ext/pdo_odbc/odbc_driver.c | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 5fd640cb093d7..5e0945e0a43de 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2256,7 +2256,10 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent) zend_hash_str_del(&EG(persistent_list), hashed_details, hashed_len); goto try_and_get_another_connection; } - /* If the driver doesn't support it, fall back to old heuristic */ + /* If the driver doesn't support it, or returns + * false (could be a false positive), fall back + * to the old heuristic. + */ ret = SQLGetInfo(db_conn->hdbc, SQL_DATA_SOURCE_READ_ONLY, d_name, sizeof(d_name), &len); diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index 0e1eec114c2cc..661cab3c04f8c 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -405,7 +405,8 @@ static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) return FAILURE; } /* - * If the driver doesn't support SQL_ATTR_CONNECTION_DEAD, fall back + * If the driver doesn't support SQL_ATTR_CONNECTION_DEAD, or if + * it returns false (which could be a false positive), fall back * to using SQL_DATA_SOURCE_READ_ONLY, which isn't semantically * correct, but works with many drivers. */