Skip to content

Commit

Permalink
Check exception when finalizing output parameters (#1143)
Browse files Browse the repository at this point in the history
  • Loading branch information
yitam authored Jun 22, 2020
1 parent f45f257 commit d4a29fe
Show file tree
Hide file tree
Showing 5 changed files with 294 additions and 5 deletions.
2 changes: 1 addition & 1 deletion source/shared/core_sqlsrv.h
Original file line number Diff line number Diff line change
Expand Up @@ -1913,7 +1913,7 @@ enum error_handling_flags {
// 3/message) driver specific error message
// The fetch type determines if the indices are numeric, associative, or both.
bool core_sqlsrv_get_odbc_error( _Inout_ sqlsrv_context& ctx, _In_ int record_number, _Inout_ sqlsrv_error_auto_ptr& error,
_In_ logging_severity severity, _In_ bool check_warning = false );
_In_ logging_severity severity, _In_opt_ bool check_warning = false );

// format and return a driver specfic error
void core_sqlsrv_format_driver_error( _In_ sqlsrv_context& ctx, _In_ sqlsrv_error_const const* custom_error,
Expand Down
14 changes: 11 additions & 3 deletions source/shared/core_stmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void col_cache_dtor( _Inout_ zval* data_z );
void field_cache_dtor( _Inout_ zval* data_z );
int round_up_decimal_numbers(_Inout_ char* buffer, _In_ short decimal_pos, _In_ short decimals_places, _In_ short offset, _In_ short lastpos);
void format_decimal_numbers(_In_ SQLSMALLINT decimals_places, _In_ SQLSMALLINT field_scale, _Inout_updates_bytes_(*field_len) char*& field_value, _Inout_ SQLLEN* field_len);
void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt );
void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt, _In_opt_ bool exception_thrown = false );
void get_field_as_string( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT field_index, _Inout_ sqlsrv_phptype sqlsrv_php_type,
_Inout_updates_bytes_(*field_len) void*& field_value, _Inout_ SQLLEN* field_len );
stmt_option const* get_stmt_option( sqlsrv_conn const* conn, _In_ zend_ulong key, _In_ const stmt_option stmt_opts[] );
Expand Down Expand Up @@ -820,7 +820,7 @@ SQLRETURN core_sqlsrv_execute( _Inout_ sqlsrv_stmt* stmt, _In_reads_bytes_(sql_l
// if the statement executed but failed in a subsequent operation before returning,
// we need to cancel the statement and deref the output and stream parameters
if ( stmt->send_streams_at_exec ) {
finalize_output_parameters( stmt );
finalize_output_parameters( stmt, true );
zend_hash_clean( Z_ARRVAL( stmt->param_streams ));
}
if( stmt->executed ) {
Expand Down Expand Up @@ -2364,10 +2364,18 @@ void format_decimal_numbers(_In_ SQLSMALLINT decimals_places, _In_ SQLSMALLINT f
// parameters passed to SQLBindParameter. It also converts output strings from UTF-16 to UTF-8 if necessary.
// For integer or float parameters, it sets those to NULL if a NULL was returned by SQL Server

void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt )
void finalize_output_parameters( _Inout_ sqlsrv_stmt* stmt, _In_opt_ bool exception_thrown /*= false*/ )
{
if (Z_ISUNDEF(stmt->output_params))
return;

// If an error occurs or an exception is thrown during an execution, the values of any output
// parameters or columns are undefined. Therefore, do not depend on them having any specific
// values, because the ODBC driver may or may not have modified them.
if (exception_thrown) {
zend_hash_clean(Z_ARRVAL(stmt->output_params));
return;
}

HashTable* params_ht = Z_ARRVAL(stmt->output_params);
zend_ulong index = -1;
Expand Down
2 changes: 1 addition & 1 deletion source/shared/core_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ void convert_datetime_string_to_zval(_Inout_ sqlsrv_stmt* stmt, _In_opt_ char* i
// 3/message) driver specific error message
// The fetch type determines if the indices are numeric, associative, or both.

bool core_sqlsrv_get_odbc_error( _Inout_ sqlsrv_context& ctx, _In_ int record_number, _Inout_ sqlsrv_error_auto_ptr& error, _In_ logging_severity severity, _In_ bool check_warning /* = false */)
bool core_sqlsrv_get_odbc_error( _Inout_ sqlsrv_context& ctx, _In_ int record_number, _Inout_ sqlsrv_error_auto_ptr& error, _In_ logging_severity severity, _In_opt_ bool check_warning /* = false */)
{
SQLHANDLE h = ctx.handle();
SQLSMALLINT h_type = ctx.handle_type();
Expand Down
138 changes: 138 additions & 0 deletions test/functional/pdo_sqlsrv/pdo_ae_output_param_errors.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
--TEST--
Test to incorrectly bind input parameters as output parameters of various types
--DESCRIPTION--
Test to incorrectly bind input parameters as output parameters of various types.
The key is to enable ColumnEncryption and check for memory leaks.
--SKIPIF--
<?php require('skipif_mid-refactor.inc'); ?>
--FILE--
<?php

function checkODBCVersion()
{
$conn = connect();
$msodbcsql_ver = $conn->getAttribute(PDO::ATTR_CLIENT_VERSION)["DriverVer"];
$vers = explode(".", $msodbcsql_ver);

unset($conn);
if ($vers[0] >= 17 && $vers[1] > 0){
return true;
} else {
return false;
}
}

require_once("MsCommon_mid-refactor.inc");

try {
// Check if the ODBC driver supports connecting with ColumnEncryption
// If not simply return
if (!checkODBCVersion()) {
echo "Done\n";
return;
}

$conn = connect("ColumnEncryption=Enabled;");

// Create a dummy table with various data types
$tbname = 'pdo_output_param_errors';
$colMetaArr = array("c1_int" => "int",
"c2_smallint" => "smallint",
"c3_tinyint" => "tinyint",
"c4_bit" => "bit",
"c5_bigint" => "bigint",
"c6_decimal" => "decimal(18,5)",
"c7_numeric" => "numeric(10,5)",
"c8_float" => "float",
"c9_real" => "real",
"c10_date" => "date",
"c11_datetime" => "datetime",
"c12_datetime2" => "datetime2",
"c13_datetimeoffset" => "datetimeoffset",
"c14_time" => "time",
"c15_char" => "char(5)",
"c16_varchar" => "varchar(max)",
"c17_nchar" => "nchar(5)",
"c18_nvarchar" => "nvarchar(max)");
createTable($conn, $tbname, $colMetaArr);

// Create a dummy select statement
$tsql = "SELECT * FROM $tbname WHERE c1_int = ? OR c2_smallint = ? OR c3_tinyint = ? ";
$tsql .= "OR c4_bit = ? OR c5_bigint = ? OR c6_decimal = ? OR c7_numeric = ? OR c8_float = ? ";
$tsql .= "OR c9_real = ? OR c10_date = ? OR c11_datetime = ? OR c12_datetime2 = ? ";
$tsql .= "OR c13_datetimeoffset = ? OR c14_time = ? OR c15_char = ? ";
$tsql .= "OR c16_varchar = ? OR c17_nchar = ? OR c18_nvarchar = ?";

// Initialize all inputs, set bigint, decimal and numeric as empty strings
$intOut = 0;
$smallintOut = 0;
$tinyintOut = 0;
$bitOut = 0;
$bigintOut = '';
$decimalOut = '';
$numericOut = '';
$floatOut = 0.0;
$realOut = 0.0;
$dateOut = '0001-01-01';
$datetimeOut = '1753-01-01 00:00:00';
$datetime2Out = '0001-01-01 00:00:00';
$datetimeoffsetOut = '1900-01-01 00:00:00 +01:00';
$timeOut = '00:00:00';
$charOut = '';
$varcharOut = '';
$ncharOut = '';
$nvarcharOut = '';

$usage1 = 0;
$rounds = 30;
for ($i = 0; $i < $rounds; $i++) {
$stmt = $conn->prepare($tsql);

$stmt->bindParam(1, $intOut, PDO::PARAM_INT, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(2, $smallintOut, PDO::PARAM_INT, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(3, $tinyintOut, PDO::PARAM_INT, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(4, $bitOut, PDO::PARAM_INT, PDO::SQLSRV_PARAM_OUT_DEFAULT_SIZE);
$stmt->bindParam(5, $bigintOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(6, $decimalOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(7, $numericOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(8, $floatOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(9, $realOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(10, $dateOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(11, $datetimeOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(12, $datetime2Out, PDO::PARAM_STR, 2048);
$stmt->bindParam(13, $datetimeoffsetOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(14, $timeOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(15, $charOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(16, $varcharOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(17, $ncharOut, PDO::PARAM_STR, 2048);
$stmt->bindParam(18, $nvarcharOut, PDO::PARAM_STR, 2048, PDO::SQLSRV_ENCODING_UTF8);

// Expect the following to fail so just ignore the exception caught
try {
$stmt->execute();
} catch (PDOException $e) {
;
}
unset($stmt);

// Compare the current memory usage to the previous usage
if ($i == 0) {
$usage1 = memory_get_usage();
} else {
$usage2 = memory_get_usage();
if ($usage2 > $usage1) {
echo "Memory leaks ($i)! Expected $usage1 but now $usage2\n";
}
}
}

dropTable($conn, $tbname);
unset($conn);
} catch (PDOException $e) {
var_dump($e);
}

echo "Done\n";
?>
--EXPECT--
Done
143 changes: 143 additions & 0 deletions test/functional/sqlsrv/sqlsrv_ae_output_param_errors.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
--TEST--
Test to incorrectly bind input parameters as output parameters of various types
--DESCRIPTION--
Similar to pdo_ae_output_param_errors.phpt - test to incorrectly bind input parameters
as output parameters of various types. The key is to enable ColumnEncryption and
check for memory leaks.
--SKIPIF--
<?php require('skipif_versions_old.inc'); ?>
--FILE--
<?php
function checkODBCVersion($conn)
{
$msodbcsql_ver = sqlsrv_client_info($conn)['DriverVer'];
$vers = explode(".", $msodbcsql_ver);

if ($vers[0] >= 17 && $vers[1] > 0){
return true;
} else {
return false;
}
}

// Check if the ODBC driver supports connecting with ColumnEncryption
// If not simply return
require_once('MsHelper.inc');
require_once('MsSetup.inc');

$conn = sqlsrv_connect($server, $connectionOptions);
if ($conn === false) {
fatalError("Failed to connect to $server.");
}
if (!checkODBCVersion($conn)) {
echo "Done\n";
sqlsrv_close($conn);
return;
}

// Create a dummy table with various data types
$tbname = 'srv_output_param_errors';
$colMetaArr = array( new AE\ColumnMeta("int", "c1_int"),
new AE\ColumnMeta("smallint", "c2_smallint"),
new AE\ColumnMeta("tinyint", "c3_tinyint"),
new AE\ColumnMeta("bit", "c4_bit"),
new AE\ColumnMeta("bigint", "c5_bigint"),
new AE\ColumnMeta("decimal(18,5)", "c6_decimal"),
new AE\ColumnMeta("numeric(10,5)", "c7_numeric"),
new AE\ColumnMeta("float", "c8_float"),
new AE\ColumnMeta("real", "c9_real"),
new AE\ColumnMeta("date", "c10_date"),
new AE\ColumnMeta("datetime", "c11_datetime"),
new AE\ColumnMeta("datetime2", "c12_datetime2"),
new AE\ColumnMeta("datetimeoffset", "c13_datetimeoffset"),
new AE\ColumnMeta("time", "c14_time"),
new AE\ColumnMeta("char(5)", "c15_char"),
new AE\ColumnMeta("varchar(max)", "c16_varchar"),
new AE\ColumnMeta("nchar(5)", "c17_nchar"),
new AE\ColumnMeta("nvarchar(max)", "c18_nvarchar"));
AE\createTable($conn, $tbname, $colMetaArr);

// Create a dummy select statement
$sql = "SELECT * FROM $tbname WHERE c1_int = ? OR c2_smallint = ? OR c3_tinyint = ? ";
$sql .= "OR c4_bit = ? OR c5_bigint = ? OR c6_decimal = ? OR c7_numeric = ? OR c8_float = ? ";
$sql .= "OR c9_real = ? OR c10_date = ? OR c11_datetime = ? OR c12_datetime2 = ? ";
$sql .= "OR c13_datetimeoffset = ? OR c14_time = ? OR c15_char = ? ";
$sql .= "OR c16_varchar = ? OR c17_nchar = ? OR c18_nvarchar = ?";

$options = array_merge($connectionOptions,
array('ColumnEncryption' => 'Enabled',
'CharacterSet' => 'UTF-8'));

// Initialize all inputs, set bigint, decimal and numeric as empty strings
$intOut = 0;
$smallintOut = 0;
$tinyintOut = 0;
$bitOut = 0;
$bigintOut = '';
$decimalOut = '';
$numericOut = '';
$floatOut = 0.0;
$realOut = 0.0;
$dateOut = '';
$datetimeOut = '';
$datetime2Out = '';
$datetimeoffsetOut = '';
$timeOut = '';
$charOut = '';
$varcharOut = '';
$ncharOut = '';
$nvarcharOut = '';

$usage1 = 0;
$rounds = 30;
for ($i = 0; $i < $rounds; $i++) {
// Connect with ColumnEncryption enabled
$conn2 = sqlsrv_connect($server, $options);
if ($conn2 === false) {
fatalError("Failed to connect to $server.");
}

$stmt = sqlsrv_prepare($conn2, $sql, array( array( &$intOut, SQLSRV_PARAM_OUT ),
array( &$smallintOut, SQLSRV_PARAM_OUT ),
array( &$tinyintOut, SQLSRV_PARAM_OUT ),
array( &$bitOut, SQLSRV_PARAM_OUT ),
array( &$bigintOut, SQLSRV_PARAM_OUT ),
array( &$decimalOut, SQLSRV_PARAM_OUT ),
array( &$numericOut, SQLSRV_PARAM_OUT ),
array( &$floatOut, SQLSRV_PARAM_OUT ),
array( &$realOut, SQLSRV_PARAM_OUT ),
array( &$dateOut, SQLSRV_PARAM_OUT ),
array( &$datetimeOut, SQLSRV_PARAM_OUT ),
array( &$datetime2Out, SQLSRV_PARAM_OUT ),
array( &$datetimeoffsetOut, SQLSRV_PARAM_OUT ),
array( &$timeOut, SQLSRV_PARAM_OUT ),
array( &$charOut, SQLSRV_PARAM_OUT ),
array( &$varcharOut, SQLSRV_PARAM_OUT ),
array( &$ncharOut, SQLSRV_PARAM_OUT ),
array( &$nvarcharOut, SQLSRV_PARAM_OUT )));

// Expect the following to fail so just ignore the errors
sqlsrv_execute($stmt);

// Compare the current memory usage to the previous usage
if ($i == 0) {
$usage1 = memory_get_usage();
} else {
$usage2 = memory_get_usage();
if ($usage2 > $usage1) {
echo "Memory leaks ($i)! Expected $usage1 but now $usage2\n";
}
}

// Free the resources to trigger the destruction of any zvals with refcount of 0
sqlsrv_free_stmt($stmt);
sqlsrv_close($conn2);
}

sqlsrv_query($conn, "DROP TABLE $tbname");
sqlsrv_close($conn);

echo "Done\n";
?>
--EXPECT--
Done

0 comments on commit d4a29fe

Please sign in to comment.