From 218497c64257a898d6d9c48da7ef7f00c00357c3 Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Wed, 25 Apr 2018 15:43:56 -0700 Subject: [PATCH 1/5] Made changes to output param handling code to convert doubles to ints if necessary --- source/pdo_sqlsrv/pdo_util.cpp | 4 ++-- source/shared/core_sqlsrv.h | 14 ++++++----- source/shared/core_stmt.cpp | 19 +++++++++++---- source/sqlsrv/util.cpp | 4 ++-- .../pdo_ae_output_param_decimals.phpt | 24 ++++++++----------- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_util.cpp b/source/pdo_sqlsrv/pdo_util.cpp index 6103650e..f6222b43 100644 --- a/source/pdo_sqlsrv/pdo_util.cpp +++ b/source/pdo_sqlsrv/pdo_util.cpp @@ -406,8 +406,8 @@ pdo_error PDO_ERRORS[] = { { IMSSP, (SQLCHAR*) "Stored Procedures do not support text, ntext or image as OUTPUT parameters.", -83, false } }, { - SQLSRV_ERROR_ENCRYPTED_STREAM_FETCH, - { IMSSP, (SQLCHAR*) "Connection with Column Encryption enabled does not support fetching stream. Please fetch the data as a string.", -84, false } + SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED, + { IMSSP, (SQLCHAR*) "Error converting a double value to an integer.", -84, false } }, { UINT_MAX, {} } }; diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index e5cf4565..e56edef4 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -1311,23 +1311,25 @@ struct sqlsrv_output_param { zval* param_z; SQLSRV_ENCODING encoding; - SQLUSMALLINT param_num; // used to index into the ind_or_len of the statement - SQLLEN original_buffer_len; // used to make sure the returned length didn't overflow the buffer + SQLUSMALLINT param_num; // used to index into the ind_or_len of the statement + SQLLEN original_buffer_len; // used to make sure the returned length didn't overflow the buffer + SQLSRV_PHPTYPE php_out_type; // used to convert output param if necessary bool is_bool; // string output param constructor sqlsrv_output_param( _In_ zval* p_z, _In_ SQLSRV_ENCODING enc, _In_ int num, _In_ SQLUINTEGER buffer_len ) : - param_z( p_z ), encoding( enc ), param_num( num ), original_buffer_len( buffer_len ), is_bool( false ) + param_z(p_z), encoding(enc), param_num(num), original_buffer_len(buffer_len), is_bool(false), php_out_type(SQLSRV_PHPTYPE_INVALID) { } // every other type output parameter constructor - sqlsrv_output_param( _In_ zval* p_z, _In_ int num, _In_ bool is_bool ) : + sqlsrv_output_param( _In_ zval* p_z, _In_ int num, _In_ bool is_bool, _In_ SQLSRV_PHPTYPE php_out_type) : param_z( p_z ), encoding( SQLSRV_ENCODING_INVALID ), param_num( num ), original_buffer_len( -1 ), - is_bool( is_bool ) + is_bool( is_bool ), + php_out_type(php_out_type) { } }; @@ -1699,7 +1701,7 @@ enum SQLSRV_ERROR_CODES { SQLSRV_ERROR_BUFFER_LIMIT_EXCEEDED, SQLSRV_ERROR_INVALID_BUFFER_LIMIT, SQLSRV_ERROR_OUTPUT_PARAM_TYPES_NOT_SUPPORTED, - SQLSRV_ERROR_ENCRYPTED_STREAM_FETCH, + SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED, // Driver specific error codes starts from here. SQLSRV_ERROR_DRIVER_SPECIFIC = 1000, diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index ba2deca9..64de3a1b 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -491,7 +491,7 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_ ind_ptr = buffer_len; if( direction != SQL_PARAM_INPUT ){ // save the parameter so that 1) the buffer doesn't go away, and 2) we can set it to NULL if returned - sqlsrv_output_param output_param( param_ref, static_cast( param_num ), zval_was_bool ); + sqlsrv_output_param output_param( param_ref, static_cast( param_num ), zval_was_bool, php_out_type); save_output_param_for_later( stmt, output_param TSRMLS_CC ); } } @@ -503,7 +503,7 @@ void core_sqlsrv_bind_param( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT param_ ind_ptr = buffer_len; if( direction != SQL_PARAM_INPUT ){ // save the parameter so that 1) the buffer doesn't go away, and 2) we can set it to NULL if returned - sqlsrv_output_param output_param( param_ref, static_cast( param_num ), false ); + sqlsrv_output_param output_param( param_ref, static_cast( param_num ), zval_was_bool, php_out_type); save_output_param_for_later( stmt, output_param TSRMLS_CC ); } } @@ -2149,8 +2149,19 @@ void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC ) break; case IS_DOUBLE: // for a long or a float, simply check if NULL was returned and set the parameter to a PHP null if so - if( stmt->param_ind_ptrs[ output_param->param_num ] == SQL_NULL_DATA ) { - ZVAL_NULL( value_z ); + if (stmt->param_ind_ptrs[output_param->param_num] == SQL_NULL_DATA) { + ZVAL_NULL(value_z); + } + else if (output_param->php_out_type == SQLSRV_PHPTYPE_INT) { + if (Z_DVAL_P(value_z) > INT_MAX) { + CHECK_CUSTOM_ERROR(true, stmt, SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED) { + throw core::CoreException(); + } + } + convert_to_long(value_z); + if (output_param->is_bool) { + convert_to_boolean(value_z); + } } break; default: diff --git a/source/sqlsrv/util.cpp b/source/sqlsrv/util.cpp index 0c73b368..74a38944 100644 --- a/source/sqlsrv/util.cpp +++ b/source/sqlsrv/util.cpp @@ -397,8 +397,8 @@ ss_error SS_ERRORS[] = { { IMSSP, (SQLCHAR*) "Stored Procedures do not support text, ntext or image as OUTPUT parameters.", -108, false } }, { - SQLSRV_ERROR_ENCRYPTED_STREAM_FETCH, - { IMSSP, (SQLCHAR*) "Connection with Column Encryption enabled does not support fetching stream. Please fetch the data as a string.", -109, false } + SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED, + { IMSSP, (SQLCHAR*)"Error converting a double value to an integer.", -109, false } }, // terminate the list of errors/warnings diff --git a/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt b/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt index fce2ee4d..91d59e7a 100644 --- a/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt +++ b/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt @@ -50,23 +50,15 @@ function compareFloats($actual, $expected) function compareIntegers($det, $rand, $inputValues, $pdoParamType) { /////////////////////////////////////////////////////////////////////// - // See GitHub issue 707 - Fix this method when the problem is addressed - // // Assume $pdoParamType is PDO::PARAM_BOOL or PDO::PARAM_INT if (is_string($det)) { return (!compareFloats(floatval($det), $inputValues[0]) && !compareFloats(floatval($rand), $inputValues[1])); } else { - // if $pdoParamType is PDO::PARAM_BOOL, - // expect bool(true) or bool(false) depending on the rounded input values - // But with AE enabled (aforementioned GitHub issue), the fetched values - // are floats instead, which should be fixed + // if $pdoParamType is PDO::PARAM_BOOL, expect bool(true) or bool(false) + // depending on the rounded input values $input0 = floor($inputValues[0]); // the positive float $input1 = ceil($inputValues[1]); // the negative float - if (isAEConnected()) { - $det = boolval(floor($det)); - $rand = boolval(ceil($rand)); - } return ($det == boolval($input0) && $rand == boolval($input1)); } @@ -190,10 +182,14 @@ function testOutputDecimals($inout) if ($found === false) { printValues($errMsg, $det, $rand, $inputValues); } - } elseif (!isAEConnected() && $precision >= 16) { - // When not AE enabled, large numbers are expected to - // fail when converting to booleans / integers - $error = "Error converting data type $dataType to int"; + } elseif ($precision >= 16) { + // Large numbers are expected to fail when + // converting to booleans / integers + if (isAEConnected()) { + $error = "Error converting a double value to an integer"; + } else { + $error = "Error converting data type $dataType to int"; + } $found = strpos($message, $error); if ($found === false) { printValues($errMsg, $det, $rand, $inputValues); From 5bb3f1a2ec22ada85094dbfb471d2a6d9b36cb77 Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Thu, 26 Apr 2018 08:48:41 -0700 Subject: [PATCH 2/5] Modified the error message to indicate value out of range --- source/pdo_sqlsrv/pdo_util.cpp | 2 +- source/sqlsrv/util.cpp | 2 +- test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_util.cpp b/source/pdo_sqlsrv/pdo_util.cpp index f6222b43..96f87cbc 100644 --- a/source/pdo_sqlsrv/pdo_util.cpp +++ b/source/pdo_sqlsrv/pdo_util.cpp @@ -407,7 +407,7 @@ pdo_error PDO_ERRORS[] = { }, { SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED, - { IMSSP, (SQLCHAR*) "Error converting a double value to an integer.", -84, false } + { IMSSP, (SQLCHAR*) "Error converting a double (value out of range) to an integer.", -84, false } }, { UINT_MAX, {} } }; diff --git a/source/sqlsrv/util.cpp b/source/sqlsrv/util.cpp index 74a38944..1248ebd9 100644 --- a/source/sqlsrv/util.cpp +++ b/source/sqlsrv/util.cpp @@ -398,7 +398,7 @@ ss_error SS_ERRORS[] = { }, { SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED, - { IMSSP, (SQLCHAR*)"Error converting a double value to an integer.", -109, false } + { IMSSP, (SQLCHAR*)"Error converting a double (value out of range) to an integer.", -109, false } }, // terminate the list of errors/warnings diff --git a/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt b/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt index 91d59e7a..339c6821 100644 --- a/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt +++ b/test/functional/pdo_sqlsrv/pdo_ae_output_param_decimals.phpt @@ -186,7 +186,7 @@ function testOutputDecimals($inout) // Large numbers are expected to fail when // converting to booleans / integers if (isAEConnected()) { - $error = "Error converting a double value to an integer"; + $error = "Error converting a double (value out of range) to an integer"; } else { $error = "Error converting data type $dataType to int"; } From 5f6c3994ca46f729ffd047a1b609dd807d75ff2b Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Thu, 26 Apr 2018 08:57:22 -0700 Subject: [PATCH 3/5] A huge negative number is out of range too --- source/shared/core_stmt.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 64de3a1b..30fe36bb 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -2153,11 +2153,15 @@ void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC ) ZVAL_NULL(value_z); } else if (output_param->php_out_type == SQLSRV_PHPTYPE_INT) { - if (Z_DVAL_P(value_z) > INT_MAX) { + // first check if its value is out of range + double dval = Z_DVAL_P(value_z); + if (dval > INT_MAX || dval < INT_MIN) { CHECK_CUSTOM_ERROR(true, stmt, SQLSRV_ERROR_DOUBLE_CONVERSION_FAILED) { throw core::CoreException(); } } + // if the output param is a boolean, still convert to + // a long integer first to take care of rounding convert_to_long(value_z); if (output_param->is_bool) { convert_to_boolean(value_z); From 2fc3afe4727c3db098d4eeaf21dd1baedcb666af Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Fri, 27 Apr 2018 10:38:21 -0700 Subject: [PATCH 4/5] Added a new test for issue 707 --- .../pdo_707_ae_output_param_decimals.phpt | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt diff --git a/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt b/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt new file mode 100644 index 00000000..28954175 --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt @@ -0,0 +1,163 @@ +--TEST-- +GitHub issue 707 - binding decimals/numerics to integers or booleans with ColumnEncryption +--DESCRIPTION-- +Verifies that the double values will be rounded as integers or returned as booleans +The key of this test is to connect with ColumnEncryption enabled, and the table columns +do not need to be encrypted +--ENV-- +PHPT_EXEC=true +--SKIPIF-- + +--FILE-- +getAttribute(PDO::ATTR_CLIENT_VERSION)["DriverVer"]; + $msodbcsql_maj = explode(".", $msodbcsql_ver)[0]; + if ($msodbcsql_maj < 17) { + return true; + } + + global $daasMode; + if ($daasMode) { + // running against Azure + return false; + } + // otherwise, check server version + $server_ver = $conn->getAttribute(PDO::ATTR_SERVER_VERSION); + if (explode('.', $server_ver)[0] < 13) { + return true; + } + + return false; +} + +function getOutputs($stmt, $outSql, $id, $pdoParamType, $inout = false) +{ + $dec = $num = 0; + + if ($inout) { + $paramType = $pdoParamType | PDO::PARAM_INPUT_OUTPUT; + } else { + $paramType = $pdoParamType; + } + + $stmt->bindParam(1, $id, PDO::PARAM_INT); + $stmt->bindParam(2, $dec, $paramType, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE); + $stmt->bindParam(3, $num, $paramType, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE); + + $stmt->execute(); + + if ($pdoParamType == PDO::PARAM_BOOL) { + if (!$dec || !$num) { + echo "The returned booleans ($dec, $num) were unexpected!\n"; + } + } else { + if ($dec != 100 || $num != 200) { + echo "The returned integers ($dec, $num) were unexpected!\n"; + } + } +} + +function getOutputsWithException($stmt, $outSql, $id, $pdoParamType, $inout = false) +{ + global $error; + + try { + getOutputs($stmt, $outSql, $id, $pdoParamType, $inout); + } catch (PDOException $e) { + $message = $e->getMessage(); + $found = strpos($message, $error); + if ($found === false) { + echo "Exception message unexpected!\n"; + } + } +} + +function getSmallNumbers($conn, $outSql) +{ + $stmt = $conn->prepare($outSql); + getOutputs($stmt, $outSql, 1, PDO::PARAM_BOOL); + getOutputs($stmt, $outSql, 1, PDO::PARAM_INT); + + getOutputs($stmt, $outSql, 1, PDO::PARAM_BOOL, true); + getOutputs($stmt, $outSql, 1, PDO::PARAM_INT, true); + + unset($stmt); +} + +function getHugeNumbers($conn, $outSql) +{ + // Expects an exception for each call + $stmt = $conn->prepare($outSql); + + getOutputsWithException($stmt, $outSql, 2, PDO::PARAM_BOOL); + getOutputsWithException($stmt, $outSql, 2, PDO::PARAM_INT); + + getOutputsWithException($stmt, $outSql, 2, PDO::PARAM_BOOL, true); + getOutputsWithException($stmt, $outSql, 2, PDO::PARAM_INT, true); + + unset($stmt); +} + +try { + // Check eligibility + $conn = new PDO( "sqlsrv:server = $server", $uid, $pwd ); + if (skipTest($conn)) { + echo "Done\n"; + return; + } + unset($conn); + + // Connection with column encryption enabled + $connectionInfo = "ColumnEncryption = Enabled;"; + $conn = new PDO("sqlsrv:server = $server; database=$databaseName; $connectionInfo", $uid, $pwd); + $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $tableName = "test_707_decimals"; + $procName = "sp_test_707_decimals"; + + dropTable($conn, $tableName); + dropProc($conn, $procName); + + // Create a test table + $tsql = "CREATE TABLE $tableName (id int identity(1,1), c1_decimal decimal(19,4), c2_numeric numeric(20, 6))"; + $stmt = $conn->query($tsql); + unset($stmt); + + // Insert two rows + $tsql = "INSERT INTO $tableName (c1_decimal, c2_numeric) VALUES (100.078, 200.034)"; + $stmt = $conn->query($tsql); + unset($stmt); + + $tsql = "INSERT INTO $tableName (c1_decimal, c2_numeric) VALUES (199999999999.0123, 999243876923.09887)"; + $stmt = $conn->query($tsql); + unset($stmt); + + // Create a stored procedure + $procArgs = "@id int, @c_decimal decimal(19,4) OUTPUT, @c_numeric numeric(20, 6) OUTPUT"; + $procCode = "SELECT @c_decimal = c1_decimal, @c_numeric = c2_numeric FROM $tableName WHERE id = @id"; + createProc($conn, $procName, $procArgs, $procCode); + + // Read them back by calling the stored procedure + $outSql = getCallProcSqlPlaceholders($procName, 3); + getSmallNumbers($conn, $outSql); + getHugeNumbers($conn, $outSql); + + dropProc($conn, $procName); + dropTable($conn, $tableName); + + unset($conn); + echo "Done\n"; +} catch( PDOException $e ) { + print_r( $e->getMessage() ); +} + +?> +--EXPECT-- +Done \ No newline at end of file From 06e92979d2b1f9fb0a7e3716e34bb2acb0001f78 Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Mon, 30 Apr 2018 12:51:18 -0700 Subject: [PATCH 5/5] Use helper method isAEQualified instead --- .../pdo_707_ae_output_param_decimals.phpt | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt b/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt index 28954175..6b59c134 100644 --- a/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt +++ b/test/functional/pdo_sqlsrv/pdo_707_ae_output_param_decimals.phpt @@ -15,28 +15,6 @@ require_once("MsCommon_mid-refactor.inc"); $error = "Error converting a double (value out of range) to an integer"; -function skipTest($conn) -{ - $msodbcsql_ver = $conn->getAttribute(PDO::ATTR_CLIENT_VERSION)["DriverVer"]; - $msodbcsql_maj = explode(".", $msodbcsql_ver)[0]; - if ($msodbcsql_maj < 17) { - return true; - } - - global $daasMode; - if ($daasMode) { - // running against Azure - return false; - } - // otherwise, check server version - $server_ver = $conn->getAttribute(PDO::ATTR_SERVER_VERSION); - if (explode('.', $server_ver)[0] < 13) { - return true; - } - - return false; -} - function getOutputs($stmt, $outSql, $id, $pdoParamType, $inout = false) { $dec = $num = 0; @@ -108,7 +86,7 @@ function getHugeNumbers($conn, $outSql) try { // Check eligibility $conn = new PDO( "sqlsrv:server = $server", $uid, $pwd ); - if (skipTest($conn)) { + if (!isAEQualified($conn)) { echo "Done\n"; return; }