From 6cbbf601b047f1c60de3f9f7d4003b2aa46f3fa3 Mon Sep 17 00:00:00 2001 From: Michal Sniatala Date: Fri, 27 Dec 2024 08:14:06 +0100 Subject: [PATCH] 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 | 1 + system/Database/BasePreparedQuery.php | 8 +++++ system/Database/MySQLi/PreparedQuery.php | 15 ++++++-- system/Database/OCI8/PreparedQuery.php | 15 +++++++- system/Database/Postgre/Forge.php | 4 +++ system/Database/Postgre/PreparedQuery.php | 6 ++++ system/Database/SQLSRV/Connection.php | 6 +++- system/Database/SQLSRV/Forge.php | 5 +++ system/Database/SQLSRV/PreparedQuery.php | 12 +++++-- system/Database/SQLite3/PreparedQuery.php | 2 ++ .../20160428212500_Create_test_tables.php | 3 +- .../Live/AbstractGetFieldDataTestCase.php | 10 +++--- .../Live/Postgre/GetFieldDataTestCase.php | 7 ++++ .../Database/Live/PreparedQueryTest.php | 35 +++++++++++++++++++ .../Live/SQLSRV/GetFieldDataTestCase.php | 7 ++++ user_guide_src/source/changelogs/v4.5.6.rst | 3 +- user_guide_src/source/database/queries.rst | 2 ++ 17 files changed, 124 insertions(+), 17 deletions(-) diff --git a/.php-cs-fixer.tests.php b/.php-cs-fixer.tests.php index 28a7124909e5..bf37256da58a 100644 --- a/.php-cs-fixer.tests.php +++ b/.php-cs-fixer.tests.php @@ -26,6 +26,7 @@ '_support/View/Cells/multiplier.php', '_support/View/Cells/colors.php', '_support/View/Cells/addition.php', + 'system/Database/Live/PreparedQueryTest.php', ]) ->notName('#Foobar.php$#'); diff --git a/system/Database/BasePreparedQuery.php b/system/Database/BasePreparedQuery.php index 8c4f252cfb7e..6ba2f6eb5a43 100644 --- a/system/Database/BasePreparedQuery.php +++ b/system/Database/BasePreparedQuery.php @@ -259,4 +259,12 @@ public function getErrorMessage(): string { return $this->errorString; } + + /** + * Whether the input contain binary data. + */ + protected function isBinary(string $input): bool + { + return mb_detect_encoding($input, 'UTF-8', true) === false; + } } diff --git a/system/Database/MySQLi/PreparedQuery.php b/system/Database/MySQLi/PreparedQuery.php index e9a6c6d10e30..34eedc25ff15 100644 --- a/system/Database/MySQLi/PreparedQuery.php +++ b/system/Database/MySQLi/PreparedQuery.php @@ -66,15 +66,19 @@ public function _execute(array $data): bool throw new BadMethodCallException('You must call prepare before trying to execute a prepared statement.'); } - // First off -bind the parameters - $bindTypes = ''; + // First off - bind the parameters + $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 @@ public function _execute(array $data): bool // 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) { diff --git a/system/Database/OCI8/PreparedQuery.php b/system/Database/OCI8/PreparedQuery.php index feffc3be50b5..e1577642c3a9 100644 --- a/system/Database/OCI8/PreparedQuery.php +++ b/system/Database/OCI8/PreparedQuery.php @@ -16,6 +16,7 @@ use BadMethodCallException; use CodeIgniter\Database\BasePreparedQuery; use CodeIgniter\Database\Exceptions\DatabaseException; +use OCILob; /** * Prepared query for OCI8 @@ -73,12 +74,24 @@ public function _execute(array $data): bool 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; } diff --git a/system/Database/Postgre/Forge.php b/system/Database/Postgre/Forge.php index b516172737ae..704f3e576047 100644 --- a/system/Database/Postgre/Forge.php +++ b/system/Database/Postgre/Forge.php @@ -173,6 +173,10 @@ protected function _attributeType(array &$attributes) $attributes['TYPE'] = 'TIMESTAMP'; break; + case 'BLOB': + $attributes['TYPE'] = 'BYTEA'; + break; + default: break; } diff --git a/system/Database/Postgre/PreparedQuery.php b/system/Database/Postgre/PreparedQuery.php index 33a6c8044c7d..c55d5d8c5402 100644 --- a/system/Database/Postgre/PreparedQuery.php +++ b/system/Database/Postgre/PreparedQuery.php @@ -87,6 +87,12 @@ public function _execute(array $data): bool 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; diff --git a/system/Database/SQLSRV/Connection.php b/system/Database/SQLSRV/Connection.php index 7d60ad1cabd1..8ded4cd02098 100644 --- a/system/Database/SQLSRV/Connection.php +++ b/system/Database/SQLSRV/Connection.php @@ -368,7 +368,11 @@ protected function _fieldData(string $table): array $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; diff --git a/system/Database/SQLSRV/Forge.php b/system/Database/SQLSRV/Forge.php index d121560c9d47..df3d55c779c4 100644 --- a/system/Database/SQLSRV/Forge.php +++ b/system/Database/SQLSRV/Forge.php @@ -397,6 +397,11 @@ protected function _attributeType(array &$attributes) $attributes['TYPE'] = 'BIT'; break; + case 'BLOB': + $attributes['TYPE'] = 'VARBINARY'; + $attributes['CONSTRAINT'] ??= 'MAX'; + break; + default: break; } diff --git a/system/Database/SQLSRV/PreparedQuery.php b/system/Database/SQLSRV/PreparedQuery.php index f3a4a14c2bc6..425555927c60 100644 --- a/system/Database/SQLSRV/PreparedQuery.php +++ b/system/Database/SQLSRV/PreparedQuery.php @@ -59,7 +59,7 @@ public function _prepare(string $sql, array $options = []): PreparedQuery // 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 @@ protected function _close(): bool /** * Handle parameters. + * + * @param array $options */ - protected function parameterize(string $queryString): array + protected function parameterize(string $queryString, array $options): array { $numberOfVariables = substr_count($queryString, '?'); @@ -129,7 +131,11 @@ protected function parameterize(string $queryString): array 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; diff --git a/system/Database/SQLite3/PreparedQuery.php b/system/Database/SQLite3/PreparedQuery.php index 21dc4c2fdeff..0e15b5d61f3b 100644 --- a/system/Database/SQLite3/PreparedQuery.php +++ b/system/Database/SQLite3/PreparedQuery.php @@ -75,6 +75,8 @@ public function _execute(array $data): bool $bindType = SQLITE3_INTEGER; } elseif (is_float($item)) { $bindType = SQLITE3_FLOAT; + } elseif (is_string($item) && $this->isBinary($item)) { + $bindType = SQLITE3_BLOB; } else { $bindType = SQLITE3_TEXT; } diff --git a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php index cf567ace96cf..434644bef280 100644 --- a/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php +++ b/tests/_support/Database/Migrations/20160428212500_Create_test_tables.php @@ -99,8 +99,7 @@ public function up(): void unset( $dataTypeFields['type_set'], $dataTypeFields['type_mediumtext'], - $dataTypeFields['type_double'], - $dataTypeFields['type_blob'] + $dataTypeFields['type_double'] ); } diff --git a/tests/system/Database/Live/AbstractGetFieldDataTestCase.php b/tests/system/Database/Live/AbstractGetFieldDataTestCase.php index 60413acc784f..011564992922 100644 --- a/tests/system/Database/Live/AbstractGetFieldDataTestCase.php +++ b/tests/system/Database/Live/AbstractGetFieldDataTestCase.php @@ -104,8 +104,8 @@ protected function createTableForType(): void $this->forge->dropTable($this->table, true); // missing types: - // TINYINT,MEDIUMINT,BIT,YEAR,BINARY,VARBINARY,TINYTEXT,LONGTEXT, - // JSON,Spatial data types + // TINYINT,MEDIUMINT,BIT,YEAR,BINARY,VARBINARY (BLOB more or less handles these two), + // 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 @@ protected function createTableForType(): void $fields['type_enum'], $fields['type_set'], $fields['type_mediumtext'], - $fields['type_double'], - $fields['type_blob'] + $fields['type_double'] ); } @@ -147,8 +146,7 @@ protected function createTableForType(): void unset( $fields['type_set'], $fields['type_mediumtext'], - $fields['type_double'], - $fields['type_blob'] + $fields['type_double'] ); } diff --git a/tests/system/Database/Live/Postgre/GetFieldDataTestCase.php b/tests/system/Database/Live/Postgre/GetFieldDataTestCase.php index 42e823397926..0adfc036c4c3 100644 --- a/tests/system/Database/Live/Postgre/GetFieldDataTestCase.php +++ b/tests/system/Database/Live/Postgre/GetFieldDataTestCase.php @@ -212,6 +212,13 @@ public function testGetFieldDataType(): void '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, diff --git a/tests/system/Database/Live/PreparedQueryTest.php b/tests/system/Database/Live/PreparedQueryTest.php index fd3b6cedb403..80e0addbca82 100644 --- a/tests/system/Database/Live/PreparedQueryTest.php +++ b/tests/system/Database/Live/PreparedQueryTest.php @@ -269,4 +269,39 @@ public function testDeallocatePreparedQueryThenTryToClose(): void $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)); + } } diff --git a/tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php b/tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php index bf00175c4fab..47aa8d108972 100644 --- a/tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php +++ b/tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php @@ -219,6 +219,13 @@ public function testGetFieldDataType(): void '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, diff --git a/user_guide_src/source/changelogs/v4.5.6.rst b/user_guide_src/source/changelogs/v4.5.6.rst index 569c5a542ecd..bf751a686135 100644 --- a/user_guide_src/source/changelogs/v4.5.6.rst +++ b/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 `_ diff --git a/user_guide_src/source/database/queries.rst b/user_guide_src/source/database/queries.rst index b4cc0f978ff9..4a07b4ba8295 100644 --- a/user_guide_src/source/database/queries.rst +++ b/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 ===================