From 6b1923cde238dfe729bb927bc5187ac1591a28b4 Mon Sep 17 00:00:00 2001 From: Jenny Tam Date: Fri, 23 Oct 2020 13:13:29 -0700 Subject: [PATCH] Fixed buffer length when fetching binary fields as streams --- source/shared/core_results.cpp | 23 ++++----- source/shared/core_stream.cpp | 49 +++++++++++++------ .../sqlsrv_fetch_large_stream_binary.phpt | 41 +++++++++++----- 3 files changed, 76 insertions(+), 37 deletions(-) diff --git a/source/shared/core_results.cpp b/source/shared/core_results.cpp index ca47644b..bdc9f6fb 100644 --- a/source/shared/core_results.cpp +++ b/source/shared/core_results.cpp @@ -882,7 +882,7 @@ SQLRETURN binary_to_string( _Inout_ SQLCHAR* field_data, _Inout_ SQLLEN& read_so _In_ SQLLEN buffer_length, _Inout_ SQLLEN* out_buffer_length, _Inout_ sqlsrv_error_auto_ptr& out_error ) { - // hex characters for the conversion loop below + // The hex characters for the conversion loop below static char hex_chars[] = "0123456789ABCDEF"; SQLSRV_ASSERT( out_error == 0, "Pending error for sqlsrv_buffered_results_set::binary_to_string" ); @@ -892,18 +892,19 @@ SQLRETURN binary_to_string( _Inout_ SQLCHAR* field_data, _Inout_ SQLLEN& read_so // Set the amount of space necessary for null characters at the end of the data. SQLSMALLINT extra = sizeof(Char); - // TO convert a binary to a system string or a binary to a wide string, the buffer size minur extra - // is expected to be multiples of 2 or 4 (depending on Char), but calculating to_copy_hex below + // TO convert a binary to a system string or a binary to a wide string, the buffer size minus + // 'extra' is ideally multiples of 2 or 4 (depending on Char), but calculating to_copy_hex below // takes care of this. - // all fields will be treated as ODBC returns varchar(max) fields: + // All fields will be treated as ODBC returns varchar(max) fields: // the entire length of the string is returned the first // call in out_buffer_len. Successive calls return how much is // left minus how much has already been read by previous reads - // *2 is for each byte to hex conversion and * extra is for either system or wide string allocation + // *2 is for each byte to hex conversion and * extra is for either system + // or wide string allocation *out_buffer_length = (*reinterpret_cast( field_data - sizeof( SQLULEN )) - read_so_far) * 2 * extra; - // copy as much as we can into the buffer + // Will copy as much as we can into the buffer SQLLEN to_copy; if( buffer_length < *out_buffer_length + extra ) { to_copy = (buffer_length - extra); @@ -916,13 +917,13 @@ SQLRETURN binary_to_string( _Inout_ SQLCHAR* field_data, _Inout_ SQLLEN& read_so to_copy = *out_buffer_length; } - // if there are bytes to copy as hex + // If there are bytes to copy as hex if( to_copy > 0 ) { // quick hex conversion routine - Char* h = reinterpret_cast( buffer ); - BYTE* b = reinterpret_cast( field_data + read_so_far ); + Char* h = reinterpret_cast(buffer); + BYTE* b = reinterpret_cast(field_data + read_so_far); // to_copy contains the number of bytes to copy, so we divide the number in half (or quarter) - // to get the maximum number of hex digits we can copy + // to get the maximum number of hex digits to copy SQLLEN to_copy_hex = static_cast(floor(to_copy / (2 * extra))); for( SQLLEN i = 0; i < to_copy_hex; ++i ) { *h = hex_chars[(*b & 0xf0) >> 4]; @@ -931,7 +932,7 @@ SQLRETURN binary_to_string( _Inout_ SQLCHAR* field_data, _Inout_ SQLLEN& read_so h++; } read_so_far += to_copy_hex; - *h = static_cast( 0 ); + *h = static_cast(0); } else { reinterpret_cast( buffer )[0] = '\0'; diff --git a/source/shared/core_stream.cpp b/source/shared/core_stream.cpp index 05f4224d..a2288aea 100644 --- a/source/shared/core_stream.cpp +++ b/source/shared/core_stream.cpp @@ -101,13 +101,18 @@ size_t sqlsrv_stream_read(_Inout_ php_stream* stream, _Out_writes_bytes_(count) throw core::CoreException(); } - // if the stream returns either no data, NULL data, or returns data < than the count requested then - // we are at the "end of the stream" so we mark it - if( r == SQL_NO_DATA || read == SQL_NULL_DATA || ( static_cast( read ) <= count && read != SQL_NO_TOTAL )) { + // If the stream returns no data or NULL data, mark the "end of the stream" and return + if( r == SQL_NO_DATA || read == SQL_NULL_DATA) { + stream->eof = 1; + return 0; + } + + // If the stream returns data less than the count requested then we are at the "end of the stream" but continue processing + if (static_cast(read) <= count && read != SQL_NO_TOTAL) { stream->eof = 1; } - // if ODBC returns the 01004 (truncated string) warning, then we return the count minus the null terminator + // If ODBC returns the 01004 (truncated string) warning, then we return the count minus the null terminator // if it's not a binary encoded field if( r == SQL_SUCCESS_WITH_INFO ) { @@ -120,26 +125,42 @@ size_t sqlsrv_stream_read(_Inout_ php_stream* stream, _Out_writes_bytes_(count) SQLSRV_ASSERT( is_truncated_warning( state ), "sqlsrv_stream_read: truncation warning was expected but it " "did not occur." ); } - - // with unixODBC connection pooling enabled the truncated state may not be returned so check the actual length read - // with buffer length. + + // As per SQLGetData documentation, if the length of character data exceeds the BufferLength, + // SQLGetData truncates the data to BufferLength less the length of null-termination character. + // But when fetching binary fields as chars (wide chars), each byte is represented as 2 hex characters, + // each takes the size of a char (wide char). Note that BufferLength may not be multiples of 2 or 4. + bool is_binary = (ss->sql_type == SQL_BINARY || ss->sql_type == SQL_VARBINARY || ss->sql_type == SQL_LONGVARBINARY); + + // With unixODBC connection pooling enabled the truncated state may not be returned so check the actual length read + // with buffer length. #ifndef _WIN32 if( is_truncated_warning( state ) || count < read) { #else if( is_truncated_warning( state ) ) { #endif // !_WIN32 + size_t char_size = sizeof(SQLCHAR); + switch( c_type ) { - - // As per SQLGetData documentation, if the length of character data exceeds the BufferLength, - // SQLGetData truncates the data to BufferLength less the length of null-termination character. case SQL_C_BINARY: read = count; break; case SQL_C_WCHAR: - read = ( count % 2 == 0 ? count - 2 : count - 3 ); + char_size = sizeof(SQLWCHAR); + if (is_binary) { + // Each binary byte read will be 2 hex wide chars in the buffer + SQLLEN num_bytes_read = static_cast(floor((count - char_size) / (2 * char_size))); + read = num_bytes_read * char_size * 2 ; + } else { + read = (count % 2 == 0 ? count - 2 : count - 3); + } break; case SQL_C_CHAR: - read = count - 1; + if (is_binary) { + read = ((count - char_size) % 2 == 0 ? count - char_size : count - char_size - 1); + } else { + read = count - 1; + } break; default: DIE( "sqlsrv_stream_read: should have never reached in this switch case."); @@ -151,10 +172,10 @@ size_t sqlsrv_stream_read(_Inout_ php_stream* stream, _Out_writes_bytes_(count) } } - // if the encoding is UTF-8 + // If the encoding is UTF-8 if( c_type == SQL_C_WCHAR ) { count *= 2; - // undo the shift to use the full buffer + // Undo the shift to use the full buffer // flags set to 0 by default, which means that any invalid characters are dropped rather than causing // an error. This happens only on XP. // convert to UTF-8 diff --git a/test/functional/sqlsrv/sqlsrv_fetch_large_stream_binary.phpt b/test/functional/sqlsrv/sqlsrv_fetch_large_stream_binary.phpt index ee3fece5..576079ff 100644 --- a/test/functional/sqlsrv/sqlsrv_fetch_large_stream_binary.phpt +++ b/test/functional/sqlsrv/sqlsrv_fetch_large_stream_binary.phpt @@ -18,10 +18,23 @@ function fetchNull($stmt, $sqltype, $message) } } +function fetchNullStream($stmt, $sqltype, $message) +{ + $stream = sqlsrv_get_field($stmt, 1, $sqltype); + if ($stream !== false) { + $value = fread($stream, 8192); + fclose($stream); + + if (!empty($value)) { + echo("$message: expected an empty value\n"); + } + } +} + function fetchStream($stmt, $test) { global $binaryValue, $hexValue; - + if (!sqlsrv_execute($stmt)) { fatalError("fetchStream: failed to execute select"); } @@ -32,17 +45,17 @@ function fetchStream($stmt, $test) switch ($test) { case 1: $sqltype = SQLSRV_PHPTYPE_STREAM(SQLSRV_ENC_CHAR); - trace("fetchStream (char string):\n"); + $type = 'char string'; $expected = $hexValue; break; case 2: $sqltype = SQLSRV_PHPTYPE_STREAM('UTF-8'); - trace("fetchStream (char string):\n"); + $type = 'UTF-8 string'; $expected = $hexValue; break; case 3: $sqltype = SQLSRV_PHPTYPE_STREAM(SQLSRV_ENC_BINARY); - trace("fetchStream (binary string):\n"); + $type = 'binary string'; $expected = $binaryValue; break; default: @@ -50,6 +63,7 @@ function fetchStream($stmt, $test) break; } + trace("fetchStream ($type):\n"); $stream = sqlsrv_get_field($stmt, 0, $sqltype); if ($stream !== false) { $value = ''; @@ -58,11 +72,13 @@ function fetchStream($stmt, $test) } fclose($stream); if (!checkData($value, $expected)) { - echo("Expected:\n$expected\nActual:\n$value\n"); + echo("fetchStream ($type)\nExpected:\n$expected\nActual:\n$value\n"); } } else { - fatalError("fetchStream ($test) failed"); + fatalError("fetchStream ($type) failed"); } + + fetchNullStream($stmt, $sqltype, "fetchStream ($type)\n"); } function fetchData($stmt, $test) @@ -79,17 +95,17 @@ function fetchData($stmt, $test) switch ($test) { case 1: $sqltype = SQLSRV_PHPTYPE_STRING(SQLSRV_ENC_CHAR); - trace("fetchData (char string):\n"); + $type = 'char string'; $expected = $hexValue; break; case 2: $sqltype = SQLSRV_PHPTYPE_STRING('UTF-8'); - trace("fetchData (char string):\n"); + $type = 'UTF-8 string'; $expected = $hexValue; break; case 3: $sqltype = SQLSRV_PHPTYPE_STRING(SQLSRV_ENC_BINARY); - trace("fetchData (binary string):\n"); + $type = 'binary string'; $expected = $binaryValue; break; default: @@ -97,12 +113,13 @@ function fetchData($stmt, $test) break; } + trace("fetchData ($type):\n"); $value = sqlsrv_get_field($stmt, 0, $sqltype); if (!checkData($value, $expected)) { - echo("Expected:\n$expected\nActual:\n$value\n"); + echo("fetchData ($type)\nExpected:\n$expected\nActual:\n$value\n"); } - fetchNull($stmt, $sqltype, "fetchData ($test)\n"); + fetchNull($stmt, $sqltype, "fetchData ($type)\n"); } function runTest($conn, $buffered) @@ -154,7 +171,7 @@ $columns = array(new AE\ColumnMeta("varbinary(max)", "varbinary_max_col"), AE\createTable($conn, $tableName, $columns); $bin = 'abcdefghijk'; -$binaryValue = str_repeat($bin, 40); +$binaryValue = str_repeat($bin, 400); $hexValue = strtoupper(bin2hex($binaryValue)); $insertSql = "INSERT INTO $tableName (varbinary_max_col, varbinary_null_col) VALUES (?, ?)";