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 fba9c1c4b2fee91b5f28005e8923a88ecbe96b59..df57efc45561a78e3d55eecef30ddaf4bbb7feef 100644 (file)
--- 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 573d5865a0dfa0a3a1cb25ddcf362de0e23fc1ee..1156d9fe03d92dbe189a820778b8c3225374ad19 100644 (file)
--- 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 20b9976850c44f26dbeb1f792d78e85e7ff77344..7528830dae4121be4ed59c946bc6448af186ceaf 100644 (file)
--- 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 c636db52043f78dbd92358aee7cc90e54f9cef18..76a4e8db89e430a57e001189f67a02b88681bc2f 100644 (file)
--- 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 (file)
index 0000000..43b484e
--- /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--
+<?php
+if (!extension_loaded('pdo')) die('skip');
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.dirname(__FILE__) . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+
+$db = PDOTest::factory();
+$db->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 80e34532872f21f8fc4ef1bc1138137456fd2205..9bb0d1c7a1521fa0c32f09c24e6367287514bb66 100644 (file)
--- 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 571ea732868d83dc57cffad813a4e082bc83e2bf..a70e924d49a8efced6293e2e93cebe8ddc275c98 100644 (file)
--- 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 {