Skip to content

Commit bbe1222

Browse files
SakiTakamachiSakiTakamachi
authored
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
1 parent f288d9c commit bbe1222

File tree

6 files changed

+265
-63
lines changed

6 files changed

+265
-63
lines changed

‎ext/odbc/odbc.stub.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,12 @@ function odbc_free_result($statement): bool {}
382382
/**
383383
* @return resource|false
384384
*/
385-
function odbc_connect(string $dsn, string $user, #[\SensitiveParameter] string $password, int $cursor_option = SQL_CUR_USE_DRIVER) {}
385+
function odbc_connect(string $dsn, ?string $user = null, #[\SensitiveParameter] ?string $password = null, int $cursor_option = SQL_CUR_USE_DRIVER) {}
386386

387387
/**
388388
* @return resource|false
389389
*/
390-
function odbc_pconnect(string $dsn, string $user, #[\SensitiveParameter] string $password, int $cursor_option = SQL_CUR_USE_DRIVER) {}
390+
function odbc_pconnect(string $dsn, ?string $user = null, #[\SensitiveParameter] ?string $password = null, int $cursor_option = SQL_CUR_USE_DRIVER) {}
391391

392392
/** @param resource $odbc */
393393
function odbc_close($odbc): void {}

‎ext/odbc/odbc_arginfo.h‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/odbc/php_odbc.c‎

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,32 +2095,56 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int
20952095
/* a connection string may have = but not ; - i.e. "DSN=PHP" */
20962096
if (strstr((char*)db, "=")) {
20972097
direct = 1;
2098+
2099+
/* This should be identical to the code in the PDO driver and vice versa. */
2100+
size_t db_len = strlen(db);
2101+
char *db_end = db + db_len;
2102+
bool use_uid_arg = uid != NULL && !php_memnistr(db, "uid=", strlen("uid="), db_end);
2103+
bool use_pwd_arg = pwd != NULL && !php_memnistr(db, "pwd=", strlen("pwd="), db_end);
2104+
20982105
/* Force UID and PWD to be set in the DSN */
2099-
bool is_uid_set = uid && *uid
2100-
&& !strstr(db, "uid=")
2101-
&& !strstr(db, "UID=");
2102-
bool is_pwd_set = pwd && *pwd
2103-
&& !strstr(db, "pwd=")
2104-
&& !strstr(db, "PWD=");
2105-
if (is_uid_set && is_pwd_set) {
2106+
if (use_uid_arg || use_pwd_arg) {
2107+
db_end--;
2108+
if ((unsigned char)*(db_end) == ';') {
2109+
*db_end = '\0';
2110+
}
2111+
21062112
char *uid_quoted = NULL, *pwd_quoted = NULL;
2107-
bool should_quote_uid = !php_odbc_connstr_is_quoted(uid) && php_odbc_connstr_should_quote(uid);
2108-
bool should_quote_pwd = !php_odbc_connstr_is_quoted(pwd) && php_odbc_connstr_should_quote(pwd);
2109-
if (should_quote_uid) {
2110-
size_t estimated_length = php_odbc_connstr_estimate_quote_length(uid);
2111-
uid_quoted = emalloc(estimated_length);
2112-
php_odbc_connstr_quote(uid_quoted, uid, estimated_length);
2113-
} else {
2114-
uid_quoted = uid;
2113+
bool should_quote_uid, should_quote_pwd;
2114+
if (use_uid_arg) {
2115+
should_quote_uid = !php_odbc_connstr_is_quoted(uid) && php_odbc_connstr_should_quote(uid);
2116+
if (should_quote_uid) {
2117+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(uid);
2118+
uid_quoted = emalloc(estimated_length);
2119+
php_odbc_connstr_quote(uid_quoted, uid, estimated_length);
2120+
} else {
2121+
uid_quoted = uid;
2122+
}
2123+
2124+
if (!use_pwd_arg) {
2125+
spprintf(&ldb, 0, "%s;UID=%s;", db, uid_quoted);
2126+
}
21152127
}
2116-
if (should_quote_pwd) {
2117-
size_t estimated_length = php_odbc_connstr_estimate_quote_length(pwd);
2118-
pwd_quoted = emalloc(estimated_length);
2119-
php_odbc_connstr_quote(pwd_quoted, pwd, estimated_length);
2120-
} else {
2121-
pwd_quoted = pwd;
2128+
2129+
if (use_pwd_arg) {
2130+
should_quote_pwd = !php_odbc_connstr_is_quoted(pwd) && php_odbc_connstr_should_quote(pwd);
2131+
if (should_quote_pwd) {
2132+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(pwd);
2133+
pwd_quoted = emalloc(estimated_length);
2134+
php_odbc_connstr_quote(pwd_quoted, pwd, estimated_length);
2135+
} else {
2136+
pwd_quoted = pwd;
2137+
}
2138+
2139+
if (!use_uid_arg) {
2140+
spprintf(&ldb, 0, "%s;PWD=%s;", db, pwd_quoted);
2141+
}
2142+
}
2143+
2144+
if (use_uid_arg && use_pwd_arg) {
2145+
spprintf(&ldb, 0, "%s;UID=%s;PWD=%s;", db, uid_quoted, pwd_quoted);
21222146
}
2123-
spprintf(&ldb, 0, "%s;UID=%s;PWD=%s", db, uid_quoted, pwd_quoted);
2147+
21242148
if (uid_quoted && should_quote_uid) {
21252149
efree(uid_quoted);
21262150
}
@@ -2167,18 +2191,19 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int
21672191
/* {{{ odbc_do_connect */
21682192
void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
21692193
{
2170-
char *db, *uid, *pwd;
2194+
char *db, *uid=NULL, *pwd=NULL;
21712195
size_t db_len, uid_len, pwd_len;
21722196
zend_long pv_opt = SQL_CUR_DEFAULT;
21732197
odbc_connection *db_conn;
21742198
int cur_opt;
21752199

2176-
/* Now an optional 4th parameter specifying the cursor type
2177-
* defaulting to the cursors default
2178-
*/
2179-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sss|l", &db, &db_len, &uid, &uid_len, &pwd, &pwd_len, &pv_opt) == FAILURE) {
2180-
RETURN_THROWS();
2181-
}
2200+
ZEND_PARSE_PARAMETERS_START(1, 4)
2201+
Z_PARAM_STRING(db, db_len)
2202+
Z_PARAM_OPTIONAL
2203+
Z_PARAM_STRING_OR_NULL(uid, uid_len)
2204+
Z_PARAM_STRING_OR_NULL(pwd, pwd_len)
2205+
Z_PARAM_LONG(pv_opt)
2206+
ZEND_PARSE_PARAMETERS_END();
21822207

21832208
cur_opt = pv_opt;
21842209

‎ext/odbc/tests/odbc_connect_001.phpt‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--TEST--
2+
odbc_connect(): Basic test for odbc_connect(). (When not using a DSN alias)
3+
--EXTENSIONS--
4+
odbc
5+
--SKIPIF--
6+
<?php
7+
include 'config.inc';
8+
if (strpos($dsn, '=') === false) {
9+
die('skip uses a DSN alias');
10+
}
11+
include 'skipif.inc';
12+
?>
13+
--FILE--
14+
<?php
15+
16+
include 'config.inc';
17+
$dsn = str_replace(";uid={$user};pwd={$pass}", '', $dsn);
18+
19+
echo "dsn without credentials / correct user / correct password\n";
20+
$conn = odbc_connect($dsn, $user, $pass);
21+
echo $conn ? "Connected.\n\n" : "";
22+
if ($conn) odbc_close($conn);
23+
24+
echo "dsn with correct user / incorrect user / correct password\n";
25+
$conn = odbc_connect("{$dsn};uid={$user}", 'hoge', $pass);
26+
echo $conn ? "Connected.\n\n" : "";
27+
if ($conn) odbc_close($conn);
28+
29+
echo "dsn with correct password / correct user / incorrect password\n";
30+
$conn = odbc_connect("{$dsn};PWD={$pass}", $user, 'fuga');
31+
echo $conn ? "Connected.\n\n" : "";
32+
if ($conn) odbc_close($conn);
33+
34+
echo "dsn with correct credentials / incorrect user / incorrect password\n";
35+
$conn = odbc_connect("{$dsn};Uid={$user};pwD={$pass}", 'hoge', 'fuga');
36+
echo $conn ? "Connected.\n\n" : "";
37+
if ($conn) odbc_close($conn);
38+
39+
echo "dsn with correct credentials / null user / null password\n";
40+
$conn = odbc_connect("{$dsn};Uid={$user};pwD={$pass}", null, null);
41+
echo $conn ? "Connected.\n\n" : "";
42+
if ($conn) odbc_close($conn);
43+
44+
echo "dsn with correct credentials / not set user / not set password\n";
45+
$conn = odbc_connect("{$dsn};Uid={$user};pwD={$pass}");
46+
echo $conn ? "Connected.\n" : "";
47+
if ($conn) odbc_close($conn);
48+
?>
49+
--EXPECT--
50+
dsn without credentials / correct user / correct password
51+
Connected.
52+
53+
dsn with correct user / incorrect user / correct password
54+
Connected.
55+
56+
dsn with correct password / correct user / incorrect password
57+
Connected.
58+
59+
dsn with correct credentials / incorrect user / incorrect password
60+
Connected.
61+
62+
dsn with correct credentials / null user / null password
63+
Connected.
64+
65+
dsn with correct credentials / not set user / not set password
66+
Connected.

‎ext/pdo_odbc/odbc_driver.c‎

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -500,36 +500,65 @@ static int pdo_odbc_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{
500500

501501
use_direct = 1;
502502

503-
/* Force UID and PWD to be set in the DSN */
504-
bool is_uid_set = dbh->username && *dbh->username
505-
&& !strstr(dbh->data_source, "uid=")
506-
&& !strstr(dbh->data_source, "UID=");
507-
bool is_pwd_set = dbh->password && *dbh->password
508-
&& !strstr(dbh->data_source, "pwd=")
509-
&& !strstr(dbh->data_source, "PWD=");
510-
if (is_uid_set && is_pwd_set) {
511-
char *uid = NULL, *pwd = NULL;
512-
bool should_quote_uid = !php_odbc_connstr_is_quoted(dbh->username) && php_odbc_connstr_should_quote(dbh->username);
513-
bool should_quote_pwd = !php_odbc_connstr_is_quoted(dbh->password) && php_odbc_connstr_should_quote(dbh->password);
514-
if (should_quote_uid) {
515-
size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->username);
516-
uid = emalloc(estimated_length);
517-
php_odbc_connstr_quote(uid, dbh->username, estimated_length);
518-
} else {
519-
uid = dbh->username;
503+
size_t db_len = strlen(dbh->data_source);
504+
bool use_uid_arg = dbh->username != NULL && !php_memnistr(dbh->data_source, "uid=", strlen("uid="), dbh->data_source + db_len);
505+
bool use_pwd_arg = dbh->password != NULL && !php_memnistr(dbh->data_source, "pwd=", strlen("pwd="), dbh->data_source + db_len);
506+
507+
if (use_uid_arg || use_pwd_arg) {
508+
char *db = (char*) emalloc(db_len + 1);
509+
strcpy(db, dbh->data_source);
510+
char *db_end = db + db_len;
511+
db_end--;
512+
if ((unsigned char)*(db_end) == ';') {
513+
*db_end = '\0';
520514
}
521-
if (should_quote_pwd) {
522-
size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->password);
523-
pwd = emalloc(estimated_length);
524-
php_odbc_connstr_quote(pwd, dbh->password, estimated_length);
525-
} else {
526-
pwd = dbh->password;
515+
516+
char *uid = NULL, *pwd = NULL, *dsn;
517+
bool should_quote_uid, should_quote_pwd;
518+
size_t new_dsn_size;
519+
520+
if (use_uid_arg) {
521+
should_quote_uid = !php_odbc_connstr_is_quoted(dbh->username) && php_odbc_connstr_should_quote(dbh->username);
522+
if (should_quote_uid) {
523+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->username);
524+
uid = emalloc(estimated_length);
525+
php_odbc_connstr_quote(uid, dbh->username, estimated_length);
526+
} else {
527+
uid = dbh->username;
528+
}
529+
530+
if (!use_pwd_arg) {
531+
new_dsn_size = strlen(db) + strlen(uid) + strlen(";UID=;") + 1;
532+
dsn = pemalloc(new_dsn_size, dbh->is_persistent);
533+
snprintf(dsn, new_dsn_size, "%s;UID=%s;", db, uid);
534+
}
535+
}
536+
537+
if (use_pwd_arg) {
538+
should_quote_pwd = !php_odbc_connstr_is_quoted(dbh->password) && php_odbc_connstr_should_quote(dbh->password);
539+
if (should_quote_pwd) {
540+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->password);
541+
pwd = emalloc(estimated_length);
542+
php_odbc_connstr_quote(pwd, dbh->password, estimated_length);
543+
} else {
544+
pwd = dbh->password;
545+
}
546+
547+
if (!use_uid_arg) {
548+
new_dsn_size = strlen(db) + strlen(pwd) + strlen(";PWD=;") + 1;
549+
dsn = pemalloc(new_dsn_size, dbh->is_persistent);
550+
snprintf(dsn, new_dsn_size, "%s;PWD=%s;", db, pwd);
551+
}
527552
}
528-
size_t new_dsn_size = strlen(dbh->data_source)
529-
+ strlen(uid) + strlen(pwd)
530-
+ strlen(";UID=;PWD=") + 1;
531-
char *dsn = pemalloc(new_dsn_size, dbh->is_persistent);
532-
snprintf(dsn, new_dsn_size, "%s;UID=%s;PWD=%s", dbh->data_source, uid, pwd);
553+
554+
if (use_uid_arg && use_pwd_arg) {
555+
new_dsn_size = strlen(db)
556+
+ strlen(uid) + strlen(pwd)
557+
+ strlen(";UID=;PWD=;") + 1;
558+
dsn = pemalloc(new_dsn_size, dbh->is_persistent);
559+
snprintf(dsn, new_dsn_size, "%s;UID=%s;PWD=%s;", db, uid, pwd);
560+
}
561+
533562
pefree((char*)dbh->data_source, dbh->is_persistent);
534563
dbh->data_source = dsn;
535564
if (uid && should_quote_uid) {
@@ -538,6 +567,7 @@ static int pdo_odbc_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{
538567
if (pwd && should_quote_pwd) {
539568
efree(pwd);
540569
}
570+
efree(db);
541571
}
542572

543573
rc = SQLDriverConnect(H->dbc, NULL, (SQLCHAR *) dbh->data_source, strlen(dbh->data_source),

‎ext/pdo_odbc/tests/basic_connection.phpt‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
--TEST--
2+
Basic test for connection. (When not using a DSN alias)
3+
--EXTENSIONS--
4+
pdo_odbc
5+
--SKIPIF--
6+
<?php
7+
require 'ext/pdo/tests/pdo_test.inc';
8+
PDOTest::skip();
9+
$dsn = getenv('PDO_ODBC_TEST_DSN');
10+
if (!$dsn || strpos($dsn, '=') === false) {
11+
die('skip');
12+
}
13+
?>
14+
--FILE--
15+
<?php
16+
$dsnWithCredentials = getenv('PDO_ODBC_TEST_DSN');
17+
$user = getenv('PDO_ODBC_TEST_USER');
18+
$password = getenv('PDO_ODBC_TEST_PASS');
19+
20+
$dsn = str_replace(";uid={$user};pwd={$password}", '', $dsnWithCredentials);
21+
22+
echo "dsn without credentials / correct user / correct password\n";
23+
try {
24+
$db = new PDO($dsn, $user, $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
25+
echo "Connected.\n\n";
26+
$db = null;
27+
} catch (PDOException $e) {
28+
echo $e->getMessage()."\n";
29+
}
30+
31+
echo "dsn with credentials / no user / no password\n";
32+
try {
33+
$db = new PDO("{$dsn};uid={$user};pwd={$password}", null, null, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
34+
echo "Connected.\n\n";
35+
$db = null;
36+
} catch (PDOException $e) {
37+
echo $e->getMessage()."\n";
38+
}
39+
40+
echo "dsn with correct user / incorrect user / correct password\n";
41+
try {
42+
$db = new PDO("{$dsn};UID={$user}", 'hoge', $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
43+
echo "Connected.\n\n";
44+
$db = null;
45+
} catch (PDOException $e) {
46+
echo $e->getMessage()."\n";
47+
}
48+
49+
echo "dsn with correct password / correct user / incorrect password\n";
50+
try {
51+
$db = new PDO("{$dsn};PWD={$password}", $user, 'fuga', [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
52+
echo "Connected.\n\n";
53+
$db = null;
54+
} catch (PDOException $e) {
55+
echo $e->getMessage()."\n";
56+
}
57+
58+
echo "dsn with correct credentials / incorrect user / incorrect password\n";
59+
try {
60+
$db = new PDO("{$dsn};Uid={$user};Pwd={$password}", 'hoge', 'fuga', [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
61+
echo "Connected.\n";
62+
$db = null;
63+
} catch (PDOException $e) {
64+
echo $e->getMessage()."\n";
65+
}
66+
?>
67+
--EXPECT--
68+
dsn without credentials / correct user / correct password
69+
Connected.
70+
71+
dsn with credentials / no user / no password
72+
Connected.
73+
74+
dsn with correct user / incorrect user / correct password
75+
Connected.
76+
77+
dsn with correct password / correct user / incorrect password
78+
Connected.
79+
80+
dsn with correct credentials / incorrect user / incorrect password
81+
Connected.

0 commit comments

Comments
 (0)