Skip to content

Commit fe1e944

Browse files
feat(model): primary key validation (codeigniter4#9840)
* feat: primary key validation in model * phpstan baseline * primary key validation in insertBatch * add changelog * cs fix * update user guide * fix tests for postgre * allow RawSql for the primary key * update phpstan * Update system/BaseModel.php Co-authored-by: John Paul E. Balandan, CPA <[email protected]> * cs fix --------- Co-authored-by: John Paul E. Balandan, CPA <[email protected]>
1 parent cbe2ed0 commit fe1e944

File tree

10 files changed

+494
-99
lines changed

10 files changed

+494
-99
lines changed

system/BaseModel.php

Lines changed: 76 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use CodeIgniter\Database\Exceptions\DatabaseException;
2020
use CodeIgniter\Database\Exceptions\DataException;
2121
use CodeIgniter\Database\Query;
22+
use CodeIgniter\Database\RawSql;
2223
use CodeIgniter\DataCaster\Cast\CastInterface;
2324
use CodeIgniter\DataConverter\DataConverter;
2425
use CodeIgniter\Entity\Cast\CastInterface as EntityCastInterface;
@@ -780,6 +781,67 @@ public function getInsertID()
780781
return is_numeric($this->insertID) ? (int) $this->insertID : $this->insertID;
781782
}
782783

784+
/**
785+
* Validates that the primary key values are valid for update/delete/insert operations.
786+
* Throws exception if invalid.
787+
*
788+
* @param bool $allowArray Whether to allow array of IDs (true for update/delete, false for insert)
789+
*
790+
* @phpstan-assert non-zero-int|non-empty-list<int|string>|RawSql|non-falsy-string $id
791+
* @throws InvalidArgumentException
792+
*/
793+
protected function validateID(mixed $id, bool $allowArray = true): void
794+
{
795+
if (is_array($id)) {
796+
// Check if arrays are allowed
797+
if (! $allowArray) {
798+
throw new InvalidArgumentException(
799+
'Invalid primary key: only a single value is allowed, not an array.',
800+
);
801+
}
802+
803+
// Check for empty array
804+
if ($id === []) {
805+
throw new InvalidArgumentException('Invalid primary key: cannot be an empty array.');
806+
}
807+
808+
// Validate each ID in the array recursively
809+
foreach ($id as $key => $valueId) {
810+
if (is_array($valueId)) {
811+
throw new InvalidArgumentException(
812+
sprintf('Invalid primary key at index %s: nested arrays are not allowed.', $key),
813+
);
814+
}
815+
816+
// Recursive call for each value (single values only in recursion)
817+
$this->validateID($valueId, false);
818+
}
819+
820+
return;
821+
}
822+
823+
// Allow RawSql objects for complex scenarios
824+
if ($id instanceof RawSql) {
825+
return;
826+
}
827+
828+
// Check for invalid single values
829+
if (in_array($id, [null, 0, '0', '', true, false], true)) {
830+
$type = is_bool($id) ? 'boolean ' . var_export($id, true) : var_export($id, true);
831+
832+
throw new InvalidArgumentException(
833+
sprintf('Invalid primary key: %s is not allowed.', $type),
834+
);
835+
}
836+
837+
// Only allow int and string at this point
838+
if (! is_int($id) && ! is_string($id)) {
839+
throw new InvalidArgumentException(
840+
sprintf('Invalid primary key: must be int or string, %s given.', get_debug_type($id)),
841+
);
842+
}
843+
}
844+
783845
/**
784846
* Inserts data into the database. If an object is provided,
785847
* it will attempt to convert it to an array.
@@ -962,19 +1024,19 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch
9621024
* Updates a single record in the database. If an object is provided,
9631025
* it will attempt to convert it into an array.
9641026
*
965-
* @param int|list<int|string>|string|null $id
966-
* @param object|row_array|null $row
1027+
* @param int|list<int|string>|RawSql|string|null $id
1028+
* @param object|row_array|null $row
9671029
*
9681030
* @throws ReflectionException
9691031
*/
9701032
public function update($id = null, $row = null): bool
9711033
{
972-
if (is_bool($id)) {
973-
throw new InvalidArgumentException('update(): argument #1 ($id) should not be boolean.');
974-
}
1034+
if ($id !== null) {
1035+
if (! is_array($id)) {
1036+
$id = [$id];
1037+
}
9751038

976-
if (is_numeric($id) || is_string($id)) {
977-
$id = [$id];
1039+
$this->validateID($id);
9781040
}
9791041

9801042
$row = $this->transformDataToArray($row, 'update');
@@ -1091,21 +1153,21 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc
10911153
/**
10921154
* Deletes a single record from the database where $id matches.
10931155
*
1094-
* @param int|list<int|string>|string|null $id The rows primary key(s).
1095-
* @param bool $purge Allows overriding the soft deletes setting.
1156+
* @param int|list<int|string>|RawSql|string|null $id The rows primary key(s).
1157+
* @param bool $purge Allows overriding the soft deletes setting.
10961158
*
10971159
* @return bool|string Returns a SQL string if in test mode.
10981160
*
10991161
* @throws DatabaseException
11001162
*/
11011163
public function delete($id = null, bool $purge = false)
11021164
{
1103-
if (is_bool($id)) {
1104-
throw new InvalidArgumentException('delete(): argument #1 ($id) should not be boolean.');
1105-
}
1165+
if ($id !== null) {
1166+
if (! is_array($id)) {
1167+
$id = [$id];
1168+
}
11061169

1107-
if (! in_array($id, [null, 0, '0'], true) && (is_numeric($id) || is_string($id))) {
1108-
$id = [$id];
1170+
$this->validateID($id);
11091171
}
11101172

11111173
$eventData = [

system/Model.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,13 @@ protected function doInsert(array $row)
311311

312312
// Require non-empty primaryKey when
313313
// not using auto-increment feature
314-
if (! $this->useAutoIncrement && ! isset($row[$this->primaryKey])) {
315-
throw DataException::forEmptyPrimaryKey('insert');
314+
if (! $this->useAutoIncrement) {
315+
if (! isset($row[$this->primaryKey])) {
316+
throw DataException::forEmptyPrimaryKey('insert');
317+
}
318+
319+
// Validate the primary key value (arrays not allowed for insert)
320+
$this->validateID($row[$this->primaryKey], false);
316321
}
317322

318323
$builder = $this->builder();
@@ -368,6 +373,9 @@ protected function doInsertBatch(?array $set = null, ?bool $escape = null, int $
368373
if (! isset($row[$this->primaryKey])) {
369374
throw DataException::forEmptyPrimaryKey('insertBatch');
370375
}
376+
377+
// Validate the primary key value
378+
$this->validateID($row[$this->primaryKey], false);
371379
}
372380
}
373381

@@ -381,7 +389,7 @@ protected function doUpdate($id = null, $row = null): bool
381389

382390
$builder = $this->builder();
383391

384-
if (! in_array($id, [null, '', 0, '0', []], true)) {
392+
if (is_array($id) && $id !== []) {
385393
$builder = $builder->whereIn($this->table . '.' . $this->primaryKey, $id);
386394
}
387395

@@ -409,7 +417,7 @@ protected function doDelete($id = null, bool $purge = false)
409417
$set = [];
410418
$builder = $this->builder();
411419

412-
if (! in_array($id, [null, '', 0, '0', []], true)) {
420+
if (is_array($id) && $id !== []) {
413421
$builder = $builder->whereIn($this->primaryKey, $id);
414422
}
415423

0 commit comments

Comments
 (0)