From: Adam Baratz Date: Mon, 10 Oct 2016 22:10:37 +0000 (-0400) Subject: Fix #73234: Emulated statements let value dictate parameter type X-Git-Tag: php-7.2.0alpha1~1174 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=32b6154a61fae820386527f3019f8c5937fc5d27;p=php Fix #73234: Emulated statements let value dictate parameter type The prepared statement emulator (pdo_sql_parser.*) figures out how to quote each query parameter. The intended type is specified by the PDO::PARAM_* consts, but this direction wasn't always followed. In practice, queries could work as expected, but subtle errors could result. For example, a numeric string bound as PDO::PARAM_INT would be sent to a driver's quote function. While these functions are told which type is expected, they generally assume values are being quoted as strings. This can result in implicit casts, which are bad for performance. This commit includes the following changes: - Cast values marked as bool/int/null to the appropriate type and bypass the driver's quote function. - Save some memory by dropping the temporary zval used for casting. - Avoid a memory leak if the driver's quote function produces an error. - Appropriate test suite updates. --- diff --git a/NEWS b/NEWS index fba9c1c4b2..df57efc455 100644 --- a/NEWS +++ b/NEWS @@ -33,5 +33,9 @@ PHP NEWS . Added array input support to mb_convert_encoding(). (Yasuo) . Added array input support to mb_check_encoding(). (Yasuo) +- PDO_DBlib: + . Fixed bug #73234 (Emulated statements let value dictate parameter type). + (Adam Baratz) + <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>> diff --git a/ext/pdo/pdo_sql_parser.c b/ext/pdo/pdo_sql_parser.c index 573d5865a0..1156d9fe03 100644 --- a/ext/pdo/pdo_sql_parser.c +++ b/ext/pdo/pdo_sql_parser.c @@ -554,40 +554,48 @@ safe: } plc->freeq = 1; } else { - zval tmp_param; - ZVAL_DUP(&tmp_param, parameter); - switch (Z_TYPE(tmp_param)) { - case IS_NULL: - plc->quoted = "NULL"; - plc->qlen = sizeof("NULL")-1; + zend_string *buf = NULL; + + switch (param->param_type) { + case PDO_PARAM_BOOL: + plc->quoted = zend_is_true(parameter) ? "1" : "0"; + plc->qlen = sizeof("1")-1; plc->freeq = 0; break; - case IS_FALSE: - case IS_TRUE: - convert_to_long(&tmp_param); - /* fall through */ - case IS_LONG: - case IS_DOUBLE: - convert_to_string(&tmp_param); - plc->qlen = Z_STRLEN(tmp_param); - plc->quoted = estrdup(Z_STRVAL(tmp_param)); + case PDO_PARAM_INT: + buf = zend_long_to_str(zval_get_long(parameter)); + + plc->qlen = ZSTR_LEN(buf); + plc->quoted = estrdup(ZSTR_VAL(buf)); plc->freeq = 1; break; + case PDO_PARAM_NULL: + plc->quoted = "NULL"; + plc->qlen = sizeof("NULL")-1; + plc->freeq = 0; + break; + default: - convert_to_string(&tmp_param); - if (!stmt->dbh->methods->quoter(stmt->dbh, Z_STRVAL(tmp_param), - Z_STRLEN(tmp_param), &plc->quoted, &plc->qlen, + buf = zval_get_string(parameter); + if (!stmt->dbh->methods->quoter(stmt->dbh, ZSTR_VAL(buf), + ZSTR_LEN(buf), &plc->quoted, &plc->qlen, param->param_type)) { /* bork */ ret = -1; strncpy(stmt->error_code, stmt->dbh->error_code, 6); + if (buf) { + zend_string_release(buf); + } goto clean_up; } plc->freeq = 1; } - zval_dtor(&tmp_param); + + if (buf) { + zend_string_release(buf); + } } } else { zval *parameter; diff --git a/ext/pdo/pdo_sql_parser.re b/ext/pdo/pdo_sql_parser.re index 20b9976850..7528830dae 100644 --- a/ext/pdo/pdo_sql_parser.re +++ b/ext/pdo/pdo_sql_parser.re @@ -240,40 +240,48 @@ safe: } plc->freeq = 1; } else { - zval tmp_param; - ZVAL_DUP(&tmp_param, parameter); - switch (Z_TYPE(tmp_param)) { - case IS_NULL: - plc->quoted = "NULL"; - plc->qlen = sizeof("NULL")-1; + zend_string *buf = NULL; + + switch (param->param_type) { + case PDO_PARAM_BOOL: + plc->quoted = zend_is_true(parameter) ? "1" : "0"; + plc->qlen = sizeof("1")-1; plc->freeq = 0; break; - case IS_FALSE: - case IS_TRUE: - convert_to_long(&tmp_param); - /* fall through */ - case IS_LONG: - case IS_DOUBLE: - convert_to_string(&tmp_param); - plc->qlen = Z_STRLEN(tmp_param); - plc->quoted = estrdup(Z_STRVAL(tmp_param)); + case PDO_PARAM_INT: + buf = zend_long_to_str(zval_get_long(parameter)); + + plc->qlen = ZSTR_LEN(buf); + plc->quoted = estrdup(ZSTR_VAL(buf)); plc->freeq = 1; break; + case PDO_PARAM_NULL: + plc->quoted = "NULL"; + plc->qlen = sizeof("NULL")-1; + plc->freeq = 0; + break; + default: - convert_to_string(&tmp_param); - if (!stmt->dbh->methods->quoter(stmt->dbh, Z_STRVAL(tmp_param), - Z_STRLEN(tmp_param), &plc->quoted, &plc->qlen, + buf = zval_get_string(parameter); + if (!stmt->dbh->methods->quoter(stmt->dbh, ZSTR_VAL(buf), + ZSTR_LEN(buf), &plc->quoted, &plc->qlen, param->param_type)) { /* bork */ ret = -1; strncpy(stmt->error_code, stmt->dbh->error_code, 6); + if (buf) { + zend_string_release(buf); + } goto clean_up; } plc->freeq = 1; } - zval_dtor(&tmp_param); + + if (buf) { + zend_string_release(buf); + } } } else { zval *parameter; diff --git a/ext/pdo/tests/bug_65946.phpt b/ext/pdo/tests/bug_65946.phpt index c636db5204..76a4e8db89 100644 --- a/ext/pdo/tests/bug_65946.phpt +++ b/ext/pdo/tests/bug_65946.phpt @@ -18,9 +18,7 @@ $db->exec('CREATE TABLE test(id int)'); $db->exec('INSERT INTO test VALUES(1)'); switch ($db->getAttribute(PDO::ATTR_DRIVER_NAME)) { case 'dblib': - // if :limit is used, the value will be quoted as '1', which is invalid syntax - // this is a bug, to be addressed separately from adding these tests to pdo_dblib - $sql = 'SELECT TOP 1 * FROM test'; + $sql = 'SELECT TOP :limit * FROM test'; break; case 'firebird': $sql = 'SELECT FIRST :limit * FROM test'; diff --git a/ext/pdo/tests/bug_73234.phpt b/ext/pdo/tests/bug_73234.phpt new file mode 100644 index 0000000000..43b484e9b2 --- /dev/null +++ b/ext/pdo/tests/bug_73234.phpt @@ -0,0 +1,43 @@ +--TEST-- +PDO Common: Bug #73234 (Emulated statements let value dictate parameter type) +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_EMULATE_PREPARES, true); +$db->exec('CREATE TABLE test(id INT NULL)'); + +$stmt = $db->prepare('INSERT INTO test VALUES(:value)'); + +$stmt->bindValue(':value', 0, PDO::PARAM_NULL); +$stmt->execute(); + +$stmt->bindValue(':value', null, PDO::PARAM_NULL); +$stmt->execute(); + +$stmt = $db->query('SELECT * FROM test'); +var_dump($stmt->fetchAll(PDO::FETCH_ASSOC)); +?> +--EXPECT-- +array(2) { + [0]=> + array(1) { + ["id"]=> + NULL + } + [1]=> + array(1) { + ["id"]=> + NULL + } +} diff --git a/ext/pdo/tests/pdo_018.phpt b/ext/pdo/tests/pdo_018.phpt index 80e3453287..9bb0d1c7a1 100644 --- a/ext/pdo/tests/pdo_018.phpt +++ b/ext/pdo/tests/pdo_018.phpt @@ -106,22 +106,16 @@ unset($stmt); echo "===INSERT===\n"; $stmt = $db->prepare('INSERT INTO test VALUES(:id, :classtype, :val)'); -$stmt->bindParam(':id', $idx); -$stmt->bindParam(':classtype', $ctype); -$stmt->bindParam(':val', $val); foreach($objs as $idx => $obj) { $ctype = $ctypes[get_class($obj)]; - if (method_exists($obj, 'serialize')) - { - $val = $obj->serialize(); - } - else - { - $val = ''; - } - $stmt->execute(); + + $stmt->bindValue(':id', $idx); + $stmt->bindValue(':classtype', $ctype, $ctype === null ? PDO::PARAM_NULL : PDO::PARAM_INT); + $stmt->bindValue(':val', method_exists($obj, 'serialize') ? $obj->serialize() : ''); + + $stmt->execute(); } unset($stmt); diff --git a/ext/pdo/tests/pdo_024.phpt b/ext/pdo/tests/pdo_024.phpt index 571ea73286..a70e924d49 100644 --- a/ext/pdo/tests/pdo_024.phpt +++ b/ext/pdo/tests/pdo_024.phpt @@ -19,7 +19,7 @@ $db->exec('create table test (id int, name varchar(10) null)'); $stmt = $db->prepare('insert into test (id, name) values(0, :name)'); $name = NULL; $before_bind = $name; -$stmt->bindParam(':name', $name); +$stmt->bindParam(':name', $name, PDO::PARAM_NULL); if ($name !== $before_bind) { echo "bind: fail\n"; } else {