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.

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! >>>
 

ext/pdo/pdo_sql_parser.c

@@ -554,40 +554,48 @@ safe:
 					}
 					plc->freeq = 1;
 				} else {
-					zval tmp_param;
+					zend_string *buf = NULL;
-				   	ZVAL_DUP(&tmp_param, parameter);
+
-					switch (Z_TYPE(tmp_param)) {
+					switch (param->param_type) {
-						case IS_NULL:
+						case PDO_PARAM_BOOL:
+							plc->quoted = zend_is_true(parameter) ? "1" : "0";
+							plc->qlen = sizeof("1")-1;
+							plc->freeq = 0;
+							break;
+
+						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;
 
-						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));
-							plc->freeq = 1;
-							break;
-
 						default:
-							convert_to_string(&tmp_param);
+							buf = zval_get_string(parameter);
-							if (!stmt->dbh->methods->quoter(stmt->dbh, Z_STRVAL(tmp_param),
+							if (!stmt->dbh->methods->quoter(stmt->dbh, ZSTR_VAL(buf),
-									Z_STRLEN(tmp_param), &plc->quoted, &plc->qlen,
+									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;

ext/pdo/pdo_sql_parser.re

@@ -240,40 +240,48 @@ safe:
 					}
 					plc->freeq = 1;
 				} else {
-					zval tmp_param;
+					zend_string *buf = NULL;
-				   	ZVAL_DUP(&tmp_param, parameter);
+
-					switch (Z_TYPE(tmp_param)) {
+					switch (param->param_type) {
-						case IS_NULL:
+						case PDO_PARAM_BOOL:
+							plc->quoted = zend_is_true(parameter) ? "1" : "0";
+							plc->qlen = sizeof("1")-1;
+							plc->freeq = 0;
+							break;
+
+						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;
 
-						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));
-							plc->freeq = 1;
-							break;
-
 						default:
-							convert_to_string(&tmp_param);
+							buf = zval_get_string(parameter);
-							if (!stmt->dbh->methods->quoter(stmt->dbh, Z_STRVAL(tmp_param),
+							if (!stmt->dbh->methods->quoter(stmt->dbh, ZSTR_VAL(buf),
-									Z_STRLEN(tmp_param), &plc->quoted, &plc->qlen,
+									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;

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
+		$sql = 'SELECT TOP :limit * FROM test';
-		// this is a bug, to be addressed separately from adding these tests to pdo_dblib
-		$sql = 'SELECT TOP 1 * FROM test';
 		break;
 	case 'firebird':
 		$sql = 'SELECT FIRST :limit * FROM test';

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

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'))
+
-	{
+	$stmt->bindValue(':id', $idx);
-		$val = $obj->serialize();
+	$stmt->bindValue(':classtype', $ctype, $ctype === null ? PDO::PARAM_NULL : PDO::PARAM_INT);
-	}
+	$stmt->bindValue(':val', method_exists($obj, 'serialize') ? $obj->serialize() : '');
-	else
+
-	{
+	$stmt->execute();
-		$val = '';
-	}
-	$stmt->execute();	
 }
 
 unset($stmt);

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 {