Fixed a bug in reading varbinary max fields

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,17 +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);
 
-    SQLSRV_ASSERT( ((buffer_length - extra) % (extra * 2)) == 0, "Must be multiple of 2 for binary to system string or "
-                   "multiple of 4 for binary to wide string" );
+    // 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<SQLLEN*>( 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);
@@ -915,22 +917,22 @@ 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<Char*>( buffer );
-        BYTE* b = reinterpret_cast<BYTE*>( field_data );
+        Char* h = reinterpret_cast<Char*>(buffer);
+        BYTE* b = reinterpret_cast<BYTE*>(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 number of hex digits we can copy
-        SQLLEN to_copy_hex = to_copy / (2 * extra);
+        // to get the maximum number of hex digits to copy
+        SQLLEN to_copy_hex = static_cast<SQLLEN>(floor(to_copy / (2 * extra)));
         for( SQLLEN i = 0; i < to_copy_hex; ++i ) {
             *h = hex_chars[(*b & 0xf0) >> 4];
             h++;
             *h = hex_chars[(*b++ & 0x0f)];
             h++;
         }
         read_so_far += to_copy_hex;
-        *h = static_cast<Char>( 0 );
+        *h = static_cast<Char>(0);
     }
     else {
         reinterpret_cast<char*>( buffer )[0] = '\0';

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<size_t>( 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<size_t>(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<SQLLEN>(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

test/functional/pdo_sqlsrv/pdo_fetch_large_stream.phpt

@@ -0,0 +1,153 @@
+--TEST--
+Test fetching varbinary, varchar, nvarchar max fields with client buffer
+--DESCRIPTION--
+Similar to sqlsrv_fetch_large_stream test but fetching varbinary, varchar, nvarchar max fields as strings with or without client buffer
+--SKIPIF--
+<?php require_once('skipif_mid-refactor.inc'); ?>
+--ENV--
+PHPT_EXEC=true
+--FILE--
+<?php
+require_once("MsCommon_mid-refactor.inc");
+
+$tableName = 'pdoFetchLobTest';
+$binaryColumn = 'varbinary_max';
+$strColumn = 'varchar_max';
+$nstrColumn = 'nvarchar_max';
+
+$bin = 'abcdefghijklmnopqrstuvwxyz';
+$binaryValue = str_repeat($bin, 100);
+$hexValue = str_repeat(strtoupper(bin2hex($bin)), 100);
+$strValue = str_repeat("stuvwxyz", 400);
+$nstrValue = str_repeat("ÃÜðßZZýA©", 200);
+
+function checkData($actual, $expected)
+{
+    trace("Actual:\n$actual\n");
+
+    $success = true;
+    $pos = strpos($actual, $expected);
+    if (($pos === false) || ($pos > 1)) {
+        $success = false;
+    }
+
+    return ($success);
+}
+
+function fetchBinary($conn, $buffered)
+{
+    global $tableName, $binaryColumn, $binaryValue, $hexValue;
+
+    try {
+        $query = "SELECT $binaryColumn FROM $tableName";
+        if ($buffered) {
+            $stmt = $conn->prepare($query, array(PDO::ATTR_CURSOR=>PDO::CURSOR_SCROLL, PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE=>PDO::SQLSRV_CURSOR_BUFFERED));
+        } else {
+            $stmt = $conn->prepare($query);
+        }
+        $stmt->bindColumn($binaryColumn, $value, PDO::PARAM_LOB, 0, PDO::SQLSRV_ENCODING_BINARY);
+        $stmt->execute();
+
+        $row = $stmt->fetch(PDO::FETCH_BOUND);
+
+        if (!checkData($value, $binaryValue)) {
+            echo "Fetched binary value unexpected ($buffered): $value\n";
+        }
+
+        $stmt->bindColumn($binaryColumn, $value, PDO::PARAM_LOB, 0, PDO::SQLSRV_ENCODING_SYSTEM);
+        $stmt->execute();
+
+        $row = $stmt->fetch(PDO::FETCH_BOUND);
+
+        if (!checkData($value, $hexValue)) {
+            echo "Fetched binary value a char string ($buffered): $value\n";
+        }
+
+        $stmt->bindColumn($binaryColumn, $value, PDO::PARAM_LOB, 0, PDO::SQLSRV_ENCODING_UTF8);
+        $stmt->execute();
+
+        $row = $stmt->fetch(PDO::FETCH_BOUND);
+
+        if (!checkData($value, $hexValue)) {
+            echo "Fetched binary value as UTF-8 string ($buffered): $value\n";
+        }
+    } catch (PdoException $e) {
+        echo "Caught exception in fetchBinary ($buffered):\n";
+        echo $e->getMessage() . PHP_EOL;
+    }
+}
+
+function fetchAsString($conn, $buffered)
+{
+    global $tableName, $strColumn, $strValue;
+    global $nstrColumn, $nstrValue;
+
+    try {
+        $query = "SELECT $strColumn, $nstrColumn FROM $tableName";
+        if ($buffered) {
+            $stmt = $conn->prepare($query, array(PDO::ATTR_CURSOR=>PDO::CURSOR_SCROLL, PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE=>PDO::SQLSRV_CURSOR_BUFFERED));
+        } else {
+            $stmt = $conn->prepare($query);
+        }
+        $stmt->execute();
+
+        $stmt->bindColumn($strColumn, $value1, PDO::PARAM_STR);
+        $stmt->bindColumn($nstrColumn, $value2, PDO::PARAM_STR);
+        $row = $stmt->fetch(PDO::FETCH_BOUND);
+
+        if (!checkData($value1, $strValue)) {
+            echo "Fetched string value ($buffered): $value1\n";
+        }
+
+        if (!checkData($value2, $nstrValue)) {
+            echo "Fetched string value ($buffered): $value2\n";
+        }
+        $stmt->execute();
+
+        $stmt->bindColumn($strColumn, $value, PDO::PARAM_STR, 0, PDO::SQLSRV_ENCODING_SYSTEM);
+        $row = $stmt->fetch(PDO::FETCH_BOUND);
+
+        if (!checkData($value, $strValue)) {
+            echo "Fetched string value: $value\n";
+        }
+    } catch (PdoException $e) {
+        echo "Caught exception in fetchBinary ($buffered):\n";
+        echo $e->getMessage() . PHP_EOL;
+    }
+}
+
+try {
+    $conn = connect();
+    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+
+    // Create table of one max column
+    $colMeta = array(new ColumnMeta('varbinary(max)', $binaryColumn),
+                     new ColumnMeta('varchar(max)', $strColumn),
+                     new ColumnMeta('nvarchar(max)', $nstrColumn));
+    createTable($conn, $tableName, $colMeta);
+
+    // Insert one row
+    $query = "INSERT INTO $tableName ($binaryColumn, $strColumn, $nstrColumn) VALUES (?, ?, ?)";
+    $stmt = $conn->prepare($query);
+    $stmt->bindParam(1, $binaryValue, PDO::PARAM_LOB, 0, PDO::SQLSRV_ENCODING_BINARY);
+    $stmt->bindParam(2, $strValue, PDO::PARAM_STR, 0, PDO::SQLSRV_ENCODING_SYSTEM);
+    $stmt->bindParam(3, $nstrValue, PDO::PARAM_STR);
+    $stmt->execute();
+    unset($stmt);
+
+    // Starting fetching with or without client buffer
+    fetchBinary($conn, false);
+    fetchBinary($conn, true);
+
+    fetchAsString($conn, false);
+    fetchAsString($conn, true);
+
+    dropTable($conn, $tableName);
+    echo "Done\n";
+    unset($conn);
+} catch (PdoException $e) {
+    echo $e->getMessage() . PHP_EOL;
+}
+?>
+--EXPECT--
+Done