fix: handling binary data for prepared statement (#9337)

* fix prepare statement sqlite

* fix prepare statement postgre

* fix prepare statement sqlsrv

* fix prepare statement oci8

* tests

* abstract isBinary() method

* fix prepare statement mysqli

* fix prepare statement oci8

* sqlsrv blob support

* fix tests

* add changelog

* fix rector

* make sqlsrv happy

* make oci8 happy - hopefully

* add a note about options for prepared statement

* ignore PreparedQueryTest.php file

* apply code suggestion for oci8

.php-cs-fixer.tests.php

@@ -26,6 +26,7 @@ $finder = Finder::create()
         '_support/View/Cells/multiplier.php',
         '_support/View/Cells/colors.php',
         '_support/View/Cells/addition.php',
+        'system/Database/Live/PreparedQueryTest.php',
     ])
     ->notName('#Foobar.php$#');
 

system/Database/BasePreparedQuery.php

@@ -259,4 +259,12 @@ abstract class BasePreparedQuery implements PreparedQueryInterface
     {
         return $this->errorString;
     }
+
+    /**
+     * Whether the input contain binary data.
+     */
+    protected function isBinary(string $input): bool
+    {
+        return mb_detect_encoding($input, 'UTF-8', true) === false;
+    }
 }

system/Database/MySQLi/PreparedQuery.php

@@ -66,15 +66,19 @@ class PreparedQuery extends BasePreparedQuery
             throw new BadMethodCallException('You must call prepare before trying to execute a prepared statement.');
         }
 
-        // First off -bind the parameters
+        // First off - bind the parameters
-        $bindTypes = '';
+        $bindTypes  = '';
+        $binaryData = [];
 
         // Determine the type string
-        foreach ($data as $item) {
+        foreach ($data as $key => $item) {
             if (is_int($item)) {
                 $bindTypes .= 'i';
             } elseif (is_numeric($item)) {
                 $bindTypes .= 'd';
+            } elseif (is_string($item) && $this->isBinary($item)) {
+                $bindTypes .= 'b';
+                $binaryData[$key] = $item;
             } else {
                 $bindTypes .= 's';
             }
@@ -83,6 +87,11 @@ class PreparedQuery extends BasePreparedQuery
         // Bind it
         $this->statement->bind_param($bindTypes, ...$data);
 
+        // Stream binary data
+        foreach ($binaryData as $key => $value) {
+            $this->statement->send_long_data($key, $value);
+        }
+
         try {
             return $this->statement->execute();
         } catch (mysqli_sql_exception $e) {

system/Database/OCI8/PreparedQuery.php

@@ -16,6 +16,7 @@ namespace CodeIgniter\Database\OCI8;
 use BadMethodCallException;
 use CodeIgniter\Database\BasePreparedQuery;
 use CodeIgniter\Database\Exceptions\DatabaseException;
+use OCILob;
 
 /**
  * Prepared query for OCI8
@@ -73,12 +74,24 @@ class PreparedQuery extends BasePreparedQuery
             throw new BadMethodCallException('You must call prepare before trying to execute a prepared statement.');
         }
 
+        $binaryData = null;
+
         foreach (array_keys($data) as $key) {
-            oci_bind_by_name($this->statement, ':' . $key, $data[$key]);
+            if (is_string($data[$key]) && $this->isBinary($data[$key])) {
+                $binaryData = oci_new_descriptor($this->db->connID, OCI_D_LOB);
+                $binaryData->writeTemporary($data[$key], OCI_TEMP_BLOB);
+                oci_bind_by_name($this->statement, ':' . $key, $binaryData, -1, OCI_B_BLOB);
+            } else {
+                oci_bind_by_name($this->statement, ':' . $key, $data[$key]);
+            }
         }
 
         $result = oci_execute($this->statement, $this->db->commitMode);
 
+        if ($binaryData instanceof OCILob) {
+            $binaryData->free();
+        }
+
         if ($result && $this->lastInsertTableName !== '') {
             $this->db->lastInsertedTableName = $this->lastInsertTableName;
         }

system/Database/Postgre/Forge.php

@@ -173,6 +173,10 @@ class Forge extends BaseForge
                 $attributes['TYPE'] = 'TIMESTAMP';
                 break;
 
+            case 'BLOB':
+                $attributes['TYPE'] = 'BYTEA';
+                break;
+
             default:
                 break;
         }

system/Database/Postgre/PreparedQuery.php

@@ -87,6 +87,12 @@ class PreparedQuery extends BasePreparedQuery
             throw new BadMethodCallException('You must call prepare before trying to execute a prepared statement.');
         }
 
+        foreach ($data as &$item) {
+            if (is_string($item) && $this->isBinary($item)) {
+                $item = pg_escape_bytea($this->db->connID, $item);
+            }
+        }
+
         $this->result = pg_execute($this->db->connID, $this->name, $data);
 
         return (bool) $this->result;

system/Database/SQLSRV/Connection.php

@@ -368,7 +368,11 @@ class Connection extends BaseConnection
 
             $retVal[$i]->max_length = $query[$i]->CHARACTER_MAXIMUM_LENGTH > 0
                 ? $query[$i]->CHARACTER_MAXIMUM_LENGTH
-                : $query[$i]->NUMERIC_PRECISION;
+                : (
+                    $query[$i]->CHARACTER_MAXIMUM_LENGTH === -1
+                    ? 'max'
+                    : $query[$i]->NUMERIC_PRECISION
+                );
 
             $retVal[$i]->nullable = $query[$i]->IS_NULLABLE !== 'NO';
             $retVal[$i]->default  = $query[$i]->COLUMN_DEFAULT;

system/Database/SQLSRV/Forge.php

@@ -397,6 +397,11 @@ class Forge extends BaseForge
                 $attributes['TYPE'] = 'BIT';
                 break;
 
+            case 'BLOB':
+                $attributes['TYPE'] = 'VARBINARY';
+                $attributes['CONSTRAINT'] ??= 'MAX';
+                break;
+
             default:
                 break;
         }

system/Database/SQLSRV/PreparedQuery.php

@@ -59,7 +59,7 @@ class PreparedQuery extends BasePreparedQuery
         // Prepare parameters for the query
         $queryString = $this->getQueryString();
 
-        $parameters = $this->parameterize($queryString);
+        $parameters = $this->parameterize($queryString, $options);
 
         // Prepare the query
         $this->statement = sqlsrv_prepare($this->db->connID, $sql, $parameters);
@@ -120,8 +120,10 @@ class PreparedQuery extends BasePreparedQuery
 
     /**
      * Handle parameters.
+     *
+     * @param array<int, mixed> $options
      */
-    protected function parameterize(string $queryString): array
+    protected function parameterize(string $queryString, array $options): array
     {
         $numberOfVariables = substr_count($queryString, '?');
 
@@ -129,7 +131,11 @@ class PreparedQuery extends BasePreparedQuery
 
         for ($c = 0; $c < $numberOfVariables; $c++) {
             $this->parameters[$c] = null;
-            $params[]             = &$this->parameters[$c];
+            if (isset($options[$c])) {
+                $params[] = [&$this->parameters[$c], SQLSRV_PARAM_IN, $options[$c]];
+            } else {
+                $params[] = &$this->parameters[$c];
+            }
         }
 
         return $params;

system/Database/SQLite3/PreparedQuery.php

@@ -75,6 +75,8 @@ class PreparedQuery extends BasePreparedQuery
                 $bindType = SQLITE3_INTEGER;
             } elseif (is_float($item)) {
                 $bindType = SQLITE3_FLOAT;
+            } elseif (is_string($item) && $this->isBinary($item)) {
+                $bindType = SQLITE3_BLOB;
             } else {
                 $bindType = SQLITE3_TEXT;
             }

tests/_support/Database/Migrations/20160428212500_Create_test_tables.php

@@ -99,8 +99,7 @@ class Migration_Create_test_tables extends Migration
             unset(
                 $dataTypeFields['type_set'],
                 $dataTypeFields['type_mediumtext'],
-                $dataTypeFields['type_double'],
+                $dataTypeFields['type_double']
-                $dataTypeFields['type_blob']
             );
         }
 

tests/system/Database/Live/AbstractGetFieldDataTestCase.php

@@ -104,8 +104,8 @@ abstract class AbstractGetFieldDataTestCase extends CIUnitTestCase
         $this->forge->dropTable($this->table, true);
 
         // missing types:
-        //   TINYINT,MEDIUMINT,BIT,YEAR,BINARY,VARBINARY,TINYTEXT,LONGTEXT,
+        //   TINYINT,MEDIUMINT,BIT,YEAR,BINARY,VARBINARY (BLOB more or less handles these two),
-        //   JSON,Spatial data types
+        //   TINYTEXT,LONGTEXT,JSON,Spatial data types
         // `id` must be INTEGER else SQLite3 error on not null for autoincrement field.
         $fields = [
             'id'           => ['type' => 'INTEGER', 'constraint' => 20, 'auto_increment' => true],
@@ -138,8 +138,7 @@ abstract class AbstractGetFieldDataTestCase extends CIUnitTestCase
                 $fields['type_enum'],
                 $fields['type_set'],
                 $fields['type_mediumtext'],
-                $fields['type_double'],
+                $fields['type_double']
-                $fields['type_blob']
             );
         }
 
@@ -147,8 +146,7 @@ abstract class AbstractGetFieldDataTestCase extends CIUnitTestCase
             unset(
                 $fields['type_set'],
                 $fields['type_mediumtext'],
-                $fields['type_double'],
+                $fields['type_double']
-                $fields['type_blob']
             );
         }
 

tests/system/Database/Live/Postgre/GetFieldDataTestCase.php

@@ -212,6 +212,13 @@ final class GetFieldDataTestCase extends AbstractGetFieldDataTestCase
                 'default'    => null,
             ],
             15 => (object) [
+                'name'       => 'type_blob',
+                'type'       => 'bytea',
+                'max_length' => null,
+                'nullable'   => true,
+                'default'    => null,
+            ],
+            16 => (object) [
                 'name'       => 'type_boolean',
                 'type'       => 'boolean',
                 'max_length' => null,

tests/system/Database/Live/PreparedQueryTest.php

@@ -269,4 +269,39 @@ final class PreparedQueryTest extends CIUnitTestCase
 
         $this->query->close();
     }
+
+    public function testInsertBinaryData(): void
+    {
+        $params = [];
+        if ($this->db->DBDriver === 'SQLSRV') {
+            $params = [0 => SQLSRV_PHPTYPE_STREAM(SQLSRV_ENC_BINARY)];
+        }
+
+        $this->query = $this->db->prepare(static fn ($db) => $db->table('type_test')->insert([
+            'type_blob' => 'binary',
+        ]), $params);
+
+        $fileContent = file_get_contents(TESTPATH . '_support/Images/EXIFsamples/landscape_0.jpg');
+        $this->assertTrue($this->query->execute($fileContent));
+
+        $id = $this->db->DBDriver === 'SQLSRV'
+            // It seems like INSERT for a prepared statement is run in the
+            // separate execution context even though it's part of the same session
+            ? (int) ($this->db->query('SELECT @@IDENTITY AS insert_id')->getRow()->insert_id ?? 0)
+            : $this->db->insertID();
+        $builder = $this->db->table('type_test');
+
+        if ($this->db->DBDriver === 'Postgre') {
+            $file = $builder->select("ENCODE(type_blob, 'base64') AS type_blob")->where('id', $id)->get()->getRow();
+            $file = base64_decode($file->type_blob, true);
+        } elseif ($this->db->DBDriver === 'OCI8') {
+            $file = $builder->select('type_blob')->where('id', $id)->get()->getRow();
+            $file = $file->type_blob->load();
+        } else {
+            $file = $builder->select('type_blob')->where('id', $id)->get()->getRow();
+            $file = $file->type_blob;
+        }
+
+        $this->assertSame(strlen($fileContent), strlen($file));
+    }
 }

tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php

@@ -219,6 +219,13 @@ final class GetFieldDataTestCase extends AbstractGetFieldDataTestCase
                 'default'    => null,
             ],
             16 => (object) [
+                'name'       => 'type_blob',
+                'type'       => 'varbinary',
+                'max_length' => 'max',
+                'nullable'   => true,
+                'default'    => null,
+            ],
+            17 => (object) [
                 'name'       => 'type_boolean',
                 'type'       => 'bit',
                 'max_length' => null,

user_guide_src/source/changelogs/v4.5.6.rst

@@ -41,7 +41,8 @@ Bugs Fixed
 - **Validation:** Fixed a bug where complex language strings were not properly handled.
 - **CURLRequest:** Added support for handling proxy responses using HTTP versions other than 1.1.
 - **Database:** Fixed a bug that caused ``Postgre\Connection::reconnect()`` method to throw an error when the connection had not yet been established.
-- **Model** Fixed a bug that caused the ``Model::getIdValue()`` method to not correctly recognize the primary key in the ``Entity`` object if a data mapping for the primary key was used.
+- **Model:** Fixed a bug that caused the ``Model::getIdValue()`` method to not correctly recognize the primary key in the ``Entity`` object if a data mapping for the primary key was used.
+- **Database:** Fixed a bug in prepared statement to correctly handle binary data.
 
 See the repo's
 `CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_

user_guide_src/source/database/queries.rst

@@ -246,6 +246,8 @@ array through in the second parameter:
 
 .. literalinclude:: queries/018.php
 
+.. note:: Currently, the only database that actually uses the array of option is SQLSRV.
+
 Executing the Query
 ===================