Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handling binary data for prepared statement #9337

Merged
merged 17 commits into from
Dec 27, 2024
1 change: 1 addition & 0 deletions .php-cs-fixer.tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -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$#');

Expand Down
8 changes: 8 additions & 0 deletions system/Database/BasePreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
15 changes: 12 additions & 3 deletions system/Database/MySQLi/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
Expand All @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion system/Database/OCI8/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,21 @@ public function _execute(array $data): bool
}

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 (isset($binaryData)) {
samsonasik marked this conversation as resolved.
Show resolved Hide resolved
$binaryData->free();
}

if ($result && $this->lastInsertTableName !== '') {
$this->db->lastInsertedTableName = $this->lastInsertTableName;
}
Expand Down
4 changes: 4 additions & 0 deletions system/Database/Postgre/Forge.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ protected function _attributeType(array &$attributes)
$attributes['TYPE'] = 'TIMESTAMP';
break;

case 'BLOB':
$attributes['TYPE'] = 'BYTEA';
break;

default:
break;
}
Expand Down
6 changes: 6 additions & 0 deletions system/Database/Postgre/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion system/Database/SQLSRV/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions system/Database/SQLSRV/Forge.php
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,11 @@ protected function _attributeType(array &$attributes)
$attributes['TYPE'] = 'BIT';
break;

case 'BLOB':
$attributes['TYPE'] = 'VARBINARY';
$attributes['CONSTRAINT'] ??= 'MAX';
break;

default:
break;
}
Expand Down
12 changes: 9 additions & 3 deletions system/Database/SQLSRV/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -120,16 +120,22 @@ protected function _close(): bool

/**
* 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, '?');

$params = [];

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;
Expand Down
2 changes: 2 additions & 0 deletions system/Database/SQLite3/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ public function up(): void
unset(
$dataTypeFields['type_set'],
$dataTypeFields['type_mediumtext'],
$dataTypeFields['type_double'],
$dataTypeFields['type_blob']
$dataTypeFields['type_double']
);
}

Expand Down
10 changes: 4 additions & 6 deletions tests/system/Database/Live/AbstractGetFieldDataTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -138,17 +138,15 @@ protected function createTableForType(): void
$fields['type_enum'],
$fields['type_set'],
$fields['type_mediumtext'],
$fields['type_double'],
$fields['type_blob']
$fields['type_double']
);
}

if ($this->db->DBDriver === 'SQLSRV') {
unset(
$fields['type_set'],
$fields['type_mediumtext'],
$fields['type_double'],
$fields['type_blob']
$fields['type_double']
);
}

Expand Down
7 changes: 7 additions & 0 deletions tests/system/Database/Live/Postgre/GetFieldDataTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 35 additions & 0 deletions tests/system/Database/Live/PreparedQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
7 changes: 7 additions & 0 deletions tests/system/Database/Live/SQLSRV/GetFieldDataTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion user_guide_src/source/changelogs/v4.5.6.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`_
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/database/queries.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
===================

Expand Down
Loading