From bbe122295672320abe121998fc80fc76d383284b Mon Sep 17 00:00:00 2001 From: Saki Takamachi <34942839+SakiTakamachi@users.noreply.github.com> Date: Sat, 4 Nov 2023 22:46:17 +0900 Subject: [PATCH] Fix GH-12296: [odbc] [pdo_odbc] Optimized odbc connection string creating (#12306) 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. Add condition when authentication information is null --- ext/odbc/odbc.stub.php | 4 +- ext/odbc/odbc_arginfo.h | 8 +-- ext/odbc/php_odbc.c | 83 +++++++++++++++-------- ext/odbc/tests/odbc_connect_001.phpt | 66 ++++++++++++++++++ ext/pdo_odbc/odbc_driver.c | 86 ++++++++++++++++-------- ext/pdo_odbc/tests/basic_connection.phpt | 81 ++++++++++++++++++++++ 6 files changed, 265 insertions(+), 63 deletions(-) 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/odbc.stub.php b/ext/odbc/odbc.stub.php index 9606a38633f..732b9d01fd8 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 574d829daad..7a431780ce8 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 443232894b2..d3dc0b52e8d 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -2095,32 +2095,56 @@ 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; + + /* 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); + bool use_pwd_arg = pwd != NULL && !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); + } } - spprintf(&ldb, 0, "%s;UID=%s;PWD=%s", db, uid_quoted, pwd_quoted); + + if (use_uid_arg && use_pwd_arg) { + spprintf(&ldb, 0, "%s;UID=%s;PWD=%s;", db, uid_quoted, pwd_quoted); + } + if (uid_quoted && should_quote_uid) { efree(uid_quoted); } @@ -2167,18 +2191,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 new file mode 100644 index 00000000000..7ab80afa81a --- /dev/null +++ b/ext/odbc/tests/odbc_connect_001.phpt @@ -0,0 +1,66 @@ +--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. + +dsn with correct credentials / null user / null password +Connected. + +dsn with correct credentials / not set user / not set password +Connected. diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index a9d5befdac8..69ce9dde532 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); + } } - 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_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); + } + } + + 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), diff --git a/ext/pdo_odbc/tests/basic_connection.phpt b/ext/pdo_odbc/tests/basic_connection.phpt new file mode 100644 index 00000000000..255eb03516f --- /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.