From f5a990d1dda7d10b1aa8c24b09e9ce98e02a75a4 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Wed, 27 Sep 2023 17:38:28 +0900 Subject: [PATCH 1/5] Optimized the handling of authentication information for odbc_sqlconnect. Declare and initialize on one line changed to use php_memnistr store strlen(db) in a variable Added a semicolon to the end of dsn. If there is a semicolon at the end of the original dsn, it will be duplicated, so it will be removed. --- ext/odbc/php_odbc.c | 67 ++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 443232894b287..cb86e5b4f03c7 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2095,32 +2095,55 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int /* a connection string may have = but not ; - i.e. "DSN=PHP" */ if (strstr((char*)db, "=")) { direct = 1; + + size_t db_len = strlen(db); + char *db_end = db + db_len; + bool use_uid_arg = !php_memnistr(db, "uid=", strlen("uid="), db_end); + bool use_pwd_arg = !php_memnistr(db, "pwd=", strlen("pwd="), db_end); + /* Force UID and PWD to be set in the DSN */ - bool is_uid_set = uid && *uid - && !strstr(db, "uid=") - && !strstr(db, "UID="); - bool is_pwd_set = pwd && *pwd - && !strstr(db, "pwd=") - && !strstr(db, "PWD="); - if (is_uid_set && is_pwd_set) { + if (use_uid_arg || use_pwd_arg) { + db_end--; + if ((unsigned char)*(db_end) == ';') { + *db_end = '\0'; + } + char *uid_quoted = NULL, *pwd_quoted = NULL; - bool should_quote_uid = !php_odbc_connstr_is_quoted(uid) && php_odbc_connstr_should_quote(uid); - bool should_quote_pwd = !php_odbc_connstr_is_quoted(pwd) && php_odbc_connstr_should_quote(pwd); - if (should_quote_uid) { - size_t estimated_length = php_odbc_connstr_estimate_quote_length(uid); - uid_quoted = emalloc(estimated_length); - php_odbc_connstr_quote(uid_quoted, uid, estimated_length); - } else { - uid_quoted = uid; + bool should_quote_uid, should_quote_pwd; + if (use_uid_arg) { + should_quote_uid = !php_odbc_connstr_is_quoted(uid) && php_odbc_connstr_should_quote(uid); + if (should_quote_uid) { + size_t estimated_length = php_odbc_connstr_estimate_quote_length(uid); + uid_quoted = emalloc(estimated_length); + php_odbc_connstr_quote(uid_quoted, uid, estimated_length); + } else { + uid_quoted = uid; + } + + if (!use_pwd_arg) { + spprintf(&ldb, 0, "%s;UID=%s;", db, uid_quoted); + } } - if (should_quote_pwd) { - size_t estimated_length = php_odbc_connstr_estimate_quote_length(pwd); - pwd_quoted = emalloc(estimated_length); - php_odbc_connstr_quote(pwd_quoted, pwd, estimated_length); - } else { - pwd_quoted = pwd; + + if (use_pwd_arg) { + should_quote_pwd = !php_odbc_connstr_is_quoted(pwd) && php_odbc_connstr_should_quote(pwd); + if (should_quote_pwd) { + size_t estimated_length = php_odbc_connstr_estimate_quote_length(pwd); + pwd_quoted = emalloc(estimated_length); + php_odbc_connstr_quote(pwd_quoted, pwd, estimated_length); + } else { + pwd_quoted = pwd; + } + + if (!use_uid_arg) { + spprintf(&ldb, 0, "%s;PWD=%s;", db, pwd_quoted); + } + } + + if (use_uid_arg && use_pwd_arg) { + spprintf(&ldb, 0, "%s;UID=%s;PWD=%s;", db, uid_quoted, pwd_quoted); } - spprintf(&ldb, 0, "%s;UID=%s;PWD=%s", db, uid_quoted, pwd_quoted); + if (uid_quoted && should_quote_uid) { efree(uid_quoted); } From 797ceece441e325c26b06ba04112906820b00f56 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Wed, 11 Oct 2023 12:38:43 +0900 Subject: [PATCH 2/5] Changed the same as odbc_connect Add condition when authentication information is null --- ext/pdo_odbc/odbc_driver.c | 86 +++++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index a9d5befdac8f2..69ce9dde5323f 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -500,36 +500,65 @@ static int pdo_odbc_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{ use_direct = 1; - /* Force UID and PWD to be set in the DSN */ - bool is_uid_set = dbh->username && *dbh->username - && !strstr(dbh->data_source, "uid=") - && !strstr(dbh->data_source, "UID="); - bool is_pwd_set = dbh->password && *dbh->password - && !strstr(dbh->data_source, "pwd=") - && !strstr(dbh->data_source, "PWD="); - if (is_uid_set && is_pwd_set) { - char *uid = NULL, *pwd = NULL; - bool should_quote_uid = !php_odbc_connstr_is_quoted(dbh->username) && php_odbc_connstr_should_quote(dbh->username); - bool should_quote_pwd = !php_odbc_connstr_is_quoted(dbh->password) && php_odbc_connstr_should_quote(dbh->password); - if (should_quote_uid) { - size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->username); - uid = emalloc(estimated_length); - php_odbc_connstr_quote(uid, dbh->username, estimated_length); - } else { - uid = dbh->username; + size_t db_len = strlen(dbh->data_source); + bool use_uid_arg = dbh->username != NULL && !php_memnistr(dbh->data_source, "uid=", strlen("uid="), dbh->data_source + db_len); + bool use_pwd_arg = dbh->password != NULL && !php_memnistr(dbh->data_source, "pwd=", strlen("pwd="), dbh->data_source + db_len); + + if (use_uid_arg || use_pwd_arg) { + char *db = (char*) emalloc(db_len + 1); + strcpy(db, dbh->data_source); + char *db_end = db + db_len; + db_end--; + if ((unsigned char)*(db_end) == ';') { + *db_end = '\0'; } - if (should_quote_pwd) { - size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->password); - pwd = emalloc(estimated_length); - php_odbc_connstr_quote(pwd, dbh->password, estimated_length); - } else { - pwd = dbh->password; + + char *uid = NULL, *pwd = NULL, *dsn; + bool should_quote_uid, should_quote_pwd; + size_t new_dsn_size; + + if (use_uid_arg) { + should_quote_uid = !php_odbc_connstr_is_quoted(dbh->username) && php_odbc_connstr_should_quote(dbh->username); + if (should_quote_uid) { + size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->username); + uid = emalloc(estimated_length); + php_odbc_connstr_quote(uid, dbh->username, estimated_length); + } else { + uid = dbh->username; + } + + if (!use_pwd_arg) { + new_dsn_size = strlen(db) + strlen(uid) + strlen(";UID=;") + 1; + dsn = pemalloc(new_dsn_size, dbh->is_persistent); + snprintf(dsn, new_dsn_size, "%s;UID=%s;", db, uid); + } + } + + if (use_pwd_arg) { + should_quote_pwd = !php_odbc_connstr_is_quoted(dbh->password) && php_odbc_connstr_should_quote(dbh->password); + if (should_quote_pwd) { + size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->password); + pwd = emalloc(estimated_length); + php_odbc_connstr_quote(pwd, dbh->password, estimated_length); + } else { + pwd = dbh->password; + } + + if (!use_uid_arg) { + new_dsn_size = strlen(db) + strlen(pwd) + strlen(";PWD=;") + 1; + dsn = pemalloc(new_dsn_size, dbh->is_persistent); + snprintf(dsn, new_dsn_size, "%s;PWD=%s;", db, pwd); + } } - size_t new_dsn_size = strlen(dbh->data_source) - + strlen(uid) + strlen(pwd) - + strlen(";UID=;PWD=") + 1; - char *dsn = pemalloc(new_dsn_size, dbh->is_persistent); - snprintf(dsn, new_dsn_size, "%s;UID=%s;PWD=%s", dbh->data_source, uid, pwd); + + if (use_uid_arg && use_pwd_arg) { + new_dsn_size = strlen(db) + + strlen(uid) + strlen(pwd) + + strlen(";UID=;PWD=;") + 1; + dsn = pemalloc(new_dsn_size, dbh->is_persistent); + snprintf(dsn, new_dsn_size, "%s;UID=%s;PWD=%s;", db, uid, pwd); + } + pefree((char*)dbh->data_source, dbh->is_persistent); dbh->data_source = dsn; if (uid && should_quote_uid) { @@ -538,6 +567,7 @@ static int pdo_odbc_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{ if (pwd && should_quote_pwd) { efree(pwd); } + efree(db); } rc = SQLDriverConnect(H->dbc, NULL, (SQLCHAR *) dbh->data_source, strlen(dbh->data_source), From 7bb69d00112bf8e2a95626e8fbee0ca3dfe0a3c8 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Sat, 14 Oct 2023 19:28:23 +0900 Subject: [PATCH 3/5] add basic test add basic test for pdo odbc connection. fixed the way to get credentials and dsn fixed skipif fixed var dsn --- ext/odbc/tests/odbc_connect_001.phpt | 50 +++++++++++++++ ext/pdo_odbc/tests/basic_connection.phpt | 81 ++++++++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 ext/odbc/tests/odbc_connect_001.phpt create mode 100644 ext/pdo_odbc/tests/basic_connection.phpt diff --git a/ext/odbc/tests/odbc_connect_001.phpt b/ext/odbc/tests/odbc_connect_001.phpt new file mode 100644 index 0000000000000..7d19cc5d2bd31 --- /dev/null +++ b/ext/odbc/tests/odbc_connect_001.phpt @@ -0,0 +1,50 @@ +--TEST-- +odbc_connect(): Basic test for odbc_connect(). (When not using a DSN alias) +--EXTENSIONS-- +odbc +--SKIPIF-- + +--FILE-- + +--EXPECT-- +dsn without credentials / correct user / correct password +Connected. + +dsn with correct user / incorrect user / correct password +Connected. + +dsn with correct password / correct user / incorrect password +Connected. + +dsn with correct credentials / incorrect user / incorrect password +Connected. diff --git a/ext/pdo_odbc/tests/basic_connection.phpt b/ext/pdo_odbc/tests/basic_connection.phpt new file mode 100644 index 0000000000000..255eb03516ff6 --- /dev/null +++ b/ext/pdo_odbc/tests/basic_connection.phpt @@ -0,0 +1,81 @@ +--TEST-- +Basic test for connection. (When not using a DSN alias) +--EXTENSIONS-- +pdo_odbc +--SKIPIF-- + +--FILE-- + PDO::ERRMODE_EXCEPTION]); + echo "Connected.\n\n"; + $db = null; +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} + +echo "dsn with credentials / no user / no password\n"; +try { + $db = new PDO("{$dsn};uid={$user};pwd={$password}", null, null, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]); + echo "Connected.\n\n"; + $db = null; +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} + +echo "dsn with correct user / incorrect user / correct password\n"; +try { + $db = new PDO("{$dsn};UID={$user}", 'hoge', $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]); + echo "Connected.\n\n"; + $db = null; +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} + +echo "dsn with correct password / correct user / incorrect password\n"; +try { + $db = new PDO("{$dsn};PWD={$password}", $user, 'fuga', [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]); + echo "Connected.\n\n"; + $db = null; +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} + +echo "dsn with correct credentials / incorrect user / incorrect password\n"; +try { + $db = new PDO("{$dsn};Uid={$user};Pwd={$password}", 'hoge', 'fuga', [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]); + echo "Connected.\n"; + $db = null; +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} +?> +--EXPECT-- +dsn without credentials / correct user / correct password +Connected. + +dsn with credentials / no user / no password +Connected. + +dsn with correct user / incorrect user / correct password +Connected. + +dsn with correct password / correct user / incorrect password +Connected. + +dsn with correct credentials / incorrect user / incorrect password +Connected. From 96faed44b0c45d2ce82d37d8c2c74a8bb2b04c06 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Thu, 26 Oct 2023 00:00:28 +0900 Subject: [PATCH 4/5] Made uid and pwd of odbc_connect and odbc_pconnect nullable. add test case fix test --- ext/odbc/odbc.stub.php | 4 ++-- ext/odbc/odbc_arginfo.h | 8 ++++---- ext/odbc/php_odbc.c | 19 ++++++++++--------- ext/odbc/tests/odbc_connect_001.phpt | 16 ++++++++++++++++ 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/ext/odbc/odbc.stub.php b/ext/odbc/odbc.stub.php index 9606a38633f34..732b9d01fd86f 100644 --- a/ext/odbc/odbc.stub.php +++ b/ext/odbc/odbc.stub.php @@ -382,12 +382,12 @@ function odbc_free_result($statement): bool {} /** * @return resource|false */ -function odbc_connect(string $dsn, string $user, #[\SensitiveParameter] string $password, int $cursor_option = SQL_CUR_USE_DRIVER) {} +function odbc_connect(string $dsn, ?string $user = null, #[\SensitiveParameter] ?string $password = null, int $cursor_option = SQL_CUR_USE_DRIVER) {} /** * @return resource|false */ -function odbc_pconnect(string $dsn, string $user, #[\SensitiveParameter] string $password, int $cursor_option = SQL_CUR_USE_DRIVER) {} +function odbc_pconnect(string $dsn, ?string $user = null, #[\SensitiveParameter] ?string $password = null, int $cursor_option = SQL_CUR_USE_DRIVER) {} /** @param resource $odbc */ function odbc_close($odbc): void {} diff --git a/ext/odbc/odbc_arginfo.h b/ext/odbc/odbc_arginfo.h index 574d829daad42..7a431780ce81f 100644 --- a/ext/odbc/odbc_arginfo.h +++ b/ext/odbc/odbc_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 0417b68a519527b0ee916bad75116ffe4a3ad304 */ + * Stub hash: ff33935d54ae09f494fe957ca0b86d6c5c8bcab2 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_close_all, 0, 0, IS_VOID, 0) ZEND_END_ARG_INFO() @@ -78,10 +78,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_free_result, 0, 1, _IS_BOOL ZEND_ARG_INFO(0, statement) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_INFO_EX(arginfo_odbc_connect, 0, 0, 3) +ZEND_BEGIN_ARG_INFO_EX(arginfo_odbc_connect, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, dsn, IS_STRING, 0) - ZEND_ARG_TYPE_INFO(0, user, IS_STRING, 0) - ZEND_ARG_TYPE_INFO(0, password, IS_STRING, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, user, IS_STRING, 1, "null") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, password, IS_STRING, 1, "null") ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, cursor_option, IS_LONG, 0, "SQL_CUR_USE_DRIVER") ZEND_END_ARG_INFO() diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index cb86e5b4f03c7..8fa3c1a421cc7 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2098,8 +2098,8 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int size_t db_len = strlen(db); char *db_end = db + db_len; - bool use_uid_arg = !php_memnistr(db, "uid=", strlen("uid="), db_end); - bool use_pwd_arg = !php_memnistr(db, "pwd=", strlen("pwd="), db_end); + bool use_uid_arg = uid != NULL && !php_memnistr(db, "uid=", strlen("uid="), db_end); + bool use_pwd_arg = pwd != NULL && !php_memnistr(db, "pwd=", strlen("pwd="), db_end); /* Force UID and PWD to be set in the DSN */ if (use_uid_arg || use_pwd_arg) { @@ -2190,18 +2190,19 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int /* {{{ odbc_do_connect */ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent) { - char *db, *uid, *pwd; + char *db, *uid=NULL, *pwd=NULL; size_t db_len, uid_len, pwd_len; zend_long pv_opt = SQL_CUR_DEFAULT; odbc_connection *db_conn; int cur_opt; - /* Now an optional 4th parameter specifying the cursor type - * defaulting to the cursors default - */ - if (zend_parse_parameters(ZEND_NUM_ARGS(), "sss|l", &db, &db_len, &uid, &uid_len, &pwd, &pwd_len, &pv_opt) == FAILURE) { - RETURN_THROWS(); - } + ZEND_PARSE_PARAMETERS_START(1, 4) + Z_PARAM_STRING(db, db_len) + Z_PARAM_OPTIONAL + Z_PARAM_STRING_OR_NULL(uid, uid_len) + Z_PARAM_STRING_OR_NULL(pwd, pwd_len) + Z_PARAM_LONG(pv_opt) + ZEND_PARSE_PARAMETERS_END(); cur_opt = pv_opt; diff --git a/ext/odbc/tests/odbc_connect_001.phpt b/ext/odbc/tests/odbc_connect_001.phpt index 7d19cc5d2bd31..7ab80afa81a73 100644 --- a/ext/odbc/tests/odbc_connect_001.phpt +++ b/ext/odbc/tests/odbc_connect_001.phpt @@ -33,6 +33,16 @@ if ($conn) odbc_close($conn); echo "dsn with correct credentials / incorrect user / incorrect password\n"; $conn = odbc_connect("{$dsn};Uid={$user};pwD={$pass}", 'hoge', 'fuga'); +echo $conn ? "Connected.\n\n" : ""; +if ($conn) odbc_close($conn); + +echo "dsn with correct credentials / null user / null password\n"; +$conn = odbc_connect("{$dsn};Uid={$user};pwD={$pass}", null, null); +echo $conn ? "Connected.\n\n" : ""; +if ($conn) odbc_close($conn); + +echo "dsn with correct credentials / not set user / not set password\n"; +$conn = odbc_connect("{$dsn};Uid={$user};pwD={$pass}"); echo $conn ? "Connected.\n" : ""; if ($conn) odbc_close($conn); ?> @@ -48,3 +58,9 @@ Connected. dsn with correct credentials / incorrect user / incorrect password Connected. + +dsn with correct credentials / null user / null password +Connected. + +dsn with correct credentials / not set user / not set password +Connected. From ad06797547a8894d1e9889a4d005a8c6964e02d1 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Fri, 3 Nov 2023 02:43:05 +0900 Subject: [PATCH 5/5] [ci skip] Added comment regarding relation to pdo --- ext/odbc/php_odbc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index 8fa3c1a421cc7..d3dc0b52e8d7e 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2096,6 +2096,7 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int if (strstr((char*)db, "=")) { direct = 1; + /* This should be identical to the code in the PDO driver and vice versa. */ size_t db_len = strlen(db); char *db_end = db + db_len; bool use_uid_arg = uid != NULL && !php_memnistr(db, "uid=", strlen("uid="), db_end);