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

Author: SakiTakamachi

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 {}

ext/odbc/odbc_arginfo.h

@@ -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);
+					}
+				}
+
+				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);
 				}
@@ -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;
 

ext/odbc/php_odbc.c

@@ -0,0 +1,66 @@
+--TEST--
+odbc_connect(): Basic test for odbc_connect(). (When not using a DSN alias)
+--EXTENSIONS--
+odbc
+--SKIPIF--
+<?php
+include 'config.inc';
+if (strpos($dsn, '=') === false) {
+    die('skip uses a DSN alias');
+}
+include 'skipif.inc';
+?>
+--FILE--
+<?php
+
+include 'config.inc';
+$dsn = str_replace(";uid={$user};pwd={$pass}", '', $dsn);
+
+echo "dsn without credentials / correct user / correct password\n";
+$conn = odbc_connect($dsn, $user, $pass);
+echo $conn ? "Connected.\n\n" : "";
+if ($conn) odbc_close($conn);
+
+echo "dsn with correct user / incorrect user / correct password\n";
+$conn = odbc_connect("{$dsn};uid={$user}", 'hoge', $pass);
+echo $conn ? "Connected.\n\n" : "";
+if ($conn) odbc_close($conn);
+
+echo "dsn with correct password / correct user / incorrect password\n";
+$conn = odbc_connect("{$dsn};PWD={$pass}", $user, 'fuga');
+echo $conn ? "Connected.\n\n" : "";
+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);
+?>
+--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.

ext/odbc/tests/odbc_connect_001.phpt

@@ -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),

ext/pdo_odbc/odbc_driver.c

@@ -0,0 +1,81 @@
+--TEST--
+Basic test for connection. (When not using a DSN alias)
+--EXTENSIONS--
+pdo_odbc
+--SKIPIF--
+<?php
+require 'ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+$dsn = getenv('PDO_ODBC_TEST_DSN');
+if (!$dsn || strpos($dsn, '=') === false) {
+    die('skip');
+}
+?>
+--FILE--
+<?php
+$dsnWithCredentials = getenv('PDO_ODBC_TEST_DSN');
+$user = getenv('PDO_ODBC_TEST_USER');
+$password = getenv('PDO_ODBC_TEST_PASS');
+
+$dsn = str_replace(";uid={$user};pwd={$password}", '', $dsnWithCredentials);
+
+echo "dsn without credentials / correct user / correct password\n";
+try {
+    $db = new PDO($dsn, $user, $password, [PDO::ATTR_ERRMODE => 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.