Skip to content

Commit 978034a

Browse files
michalsnmichalsn
committed
feat: primary key validation in model
1 parent c9d3dcd commit 978034a

File tree

5 files changed

+320
-39
lines changed

5 files changed

+320
-39
lines changed

‎system/BaseModel.php‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,61 @@ public function getInsertID()
780780
return is_numeric($this->insertID) ? (int) $this->insertID : $this->insertID;
781781
}
782782

783+
/**
784+
* Validates that the primary key values are valid for update/delete/insert operations.
785+
* Throws exception if invalid.
786+
*
787+
* @param int|list<int|string>|string $id
788+
* @param bool $allowArray Whether to allow array of IDs (true for update/delete, false for insert)
789+
*
790+
* @throws InvalidArgumentException
791+
*/
792+
protected function validateID($id, bool $allowArray = true): void
793+
{
794+
if (is_array($id)) {
795+
// Check if arrays are allowed
796+
if (! $allowArray) {
797+
throw new InvalidArgumentException(
798+
'Invalid primary key: only a single value is allowed, not an array.'
799+
);
800+
}
801+
802+
// Check for empty array
803+
if ($id === []) {
804+
throw new InvalidArgumentException('Invalid primary key: cannot be an empty array.');
805+
}
806+
807+
// Validate each ID in the array recursively
808+
foreach ($id as $key => $valueId) {
809+
if (is_array($valueId)) {
810+
throw new InvalidArgumentException(
811+
sprintf('Invalid primary key at index %s: nested arrays are not allowed.', $key)
812+
);
813+
}
814+
815+
// Recursive call for each value (single values only in recursion)
816+
$this->validateID($valueId, false);
817+
}
818+
819+
return;
820+
}
821+
822+
// Check for invalid single values
823+
if (in_array($id, [null, 0, '0', '', true, false], true)) {
824+
$type = is_bool($id) ? 'boolean ' . var_export($id, true) : var_export($id, true);
825+
throw new InvalidArgumentException(
826+
sprintf('Invalid primary key: %s is not allowed.', $type)
827+
);
828+
}
829+
830+
// Only allow int and string at this point
831+
if (! is_int($id) && ! is_string($id)) {
832+
throw new InvalidArgumentException(
833+
sprintf('Invalid primary key: must be int or string, %s given.', get_debug_type($id))
834+
);
835+
}
836+
}
837+
783838
/**
784839
* Inserts data into the database. If an object is provided,
785840
* it will attempt to convert it to an array.
@@ -969,12 +1024,12 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch
9691024
*/
9701025
public function update($id = null, $row = null): bool
9711026
{
972-
if (is_bool($id)) {
973-
throw new InvalidArgumentException('update(): argument #1 ($id) should not be boolean.');
974-
}
1027+
if ($id !== null) {
1028+
if (! is_array($id)) {
1029+
$id = [$id];
1030+
}
9751031

976-
if (is_numeric($id) || is_string($id)) {
977-
$id = [$id];
1032+
$this->validateID($id);
9781033
}
9791034

9801035
$row = $this->transformDataToArray($row, 'update');
@@ -1100,12 +1155,12 @@ public function updateBatch(?array $set = null, ?string $index = null, int $batc
11001155
*/
11011156
public function delete($id = null, bool $purge = false)
11021157
{
1103-
if (is_bool($id)) {
1104-
throw new InvalidArgumentException('delete(): argument #1 ($id) should not be boolean.');
1105-
}
1158+
if ($id !== null) {
1159+
if (! is_array($id)) {
1160+
$id = [$id];
1161+
}
11061162

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

11111166
$eventData = [

‎system/Model.php‎

Lines changed: 9 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();
@@ -381,7 +386,7 @@ protected function doUpdate($id = null, $row = null): bool
381386

382387
$builder = $this->builder();
383388

384-
if (! in_array($id, [null, '', 0, '0', []], true)) {
389+
if (is_array($id) && $id !== []) {
385390
$builder = $builder->whereIn($this->table . '.' . $this->primaryKey, $id);
386391
}
387392

@@ -409,7 +414,7 @@ protected function doDelete($id = null, bool $purge = false)
409414
$set = [];
410415
$builder = $this->builder();
411416

412-
if (! in_array($id, [null, '', 0, '0', []], true)) {
417+
if (is_array($id) && $id !== []) {
413418
$builder = $builder->whereIn($this->primaryKey, $id);
414419
}
415420

‎tests/system/Models/DeleteModelTest.php‎

Lines changed: 107 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace CodeIgniter\Models;
1515

1616
use CodeIgniter\Database\Exceptions\DatabaseException;
17+
use CodeIgniter\Exceptions\InvalidArgumentException;
1718
use CodeIgniter\Exceptions\ModelException;
1819
use PHPUnit\Framework\Attributes\DataProvider;
1920
use PHPUnit\Framework\Attributes\Group;
@@ -152,29 +153,37 @@ public function testOnlyDeleted(): void
152153

153154
/**
154155
* Given an explicit empty value in the WHERE condition
155-
* When executing a soft delete
156+
* When executing a soft delete with where() clause
156157
* Then an exception should not be thrown
157158
*
159+
* This test uses where() so values go into WHERE clause, not through validateID().
160+
*
158161
* @param int|string|null $emptyValue
159162
*/
160-
#[DataProvider('emptyPkValues')]
163+
#[DataProvider('emptyPkValuesWithWhereClause')]
161164
public function testDontThrowExceptionWhenSoftDeleteConditionIsSetWithEmptyValue($emptyValue): void
162165
{
163166
$this->createModel(UserModel::class);
164167
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
165168

166169
$this->model->where('id', $emptyValue)->delete();
167-
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
170+
// Special case: true converted to 1
171+
if ($emptyValue === true) {
172+
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NOT NULL' => null]);
173+
} else {
174+
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
175+
}
168176
}
169177

170178
/**
171179
* @param int|string|null $emptyValue
180+
* @param class-string $exception
172181
*/
173182
#[DataProvider('emptyPkValues')]
174-
public function testThrowExceptionWhenSoftDeleteParamIsEmptyValue($emptyValue): void
183+
public function testThrowExceptionWhenSoftDeleteParamIsEmptyValue($emptyValue, string $exception, string $exceptionMessage): void
175184
{
176-
$this->expectException(DatabaseException::class);
177-
$this->expectExceptionMessage('Deletes are not allowed unless they contain a "where" or "like" clause.');
185+
$this->expectException($exception);
186+
$this->expectExceptionMessage($exceptionMessage);
178187

179188
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
180189

@@ -183,16 +192,17 @@ public function testThrowExceptionWhenSoftDeleteParamIsEmptyValue($emptyValue):
183192

184193
/**
185194
* @param int|string|null $emptyValue
195+
* @param class-string $exception
186196
*/
187197
#[DataProvider('emptyPkValues')]
188-
public function testDontDeleteRowsWhenSoftDeleteParamIsEmpty($emptyValue): void
198+
public function testDontDeleteRowsWhenSoftDeleteParamIsEmpty($emptyValue, string $exception, string $exceptionMessage): void
189199
{
190200
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
191201

192202
try {
193203
$this->createModel(UserModel::class)->delete($emptyValue);
194-
} catch (DatabaseException) {
195-
// Do nothing.
204+
} catch (DatabaseException | InvalidArgumentException) {
205+
// Do nothing - both exceptions are expected for different values.
196206
}
197207

198208
$this->seeInDatabase('user', ['name' => 'Derek Jones', 'deleted_at IS NULL' => null]);
@@ -233,15 +243,13 @@ public function testPurgeDeletedWithSoftDeleteFalse(): void
233243

234244
/**
235245
* @param int|string|null $id
246+
* @param class-string $exception
236247
*/
237248
#[DataProvider('emptyPkValues')]
238-
public function testDeleteThrowDatabaseExceptionWithoutWhereClause($id): void
249+
public function testDeleteThrowDatabaseExceptionWithoutWhereClause($id, string $exception, string $exceptionMessage): void
239250
{
240-
// BaseBuilder throws Exception.
241-
$this->expectException(DatabaseException::class);
242-
$this->expectExceptionMessage(
243-
'Deletes are not allowed unless they contain a "where" or "like" clause.',
244-
);
251+
$this->expectException($exception);
252+
$this->expectExceptionMessage($exceptionMessage);
245253

246254
// $useSoftDeletes = false
247255
$this->createModel(JobModel::class);
@@ -251,15 +259,13 @@ public function testDeleteThrowDatabaseExceptionWithoutWhereClause($id): void
251259

252260
/**
253261
* @param int|string|null $id
262+
* @param class-string $exception
254263
*/
255264
#[DataProvider('emptyPkValues')]
256-
public function testDeleteWithSoftDeleteThrowDatabaseExceptionWithoutWhereClause($id): void
265+
public function testDeleteWithSoftDeleteThrowDatabaseExceptionWithoutWhereClause($id, string $exception, string $exceptionMessage): void
257266
{
258-
// Model throws Exception.
259-
$this->expectException(DatabaseException::class);
260-
$this->expectExceptionMessage(
261-
'Deletes are not allowed unless they contain a "where" or "like" clause.',
262-
);
267+
$this->expectException($exception);
268+
$this->expectExceptionMessage($exceptionMessage);
263269

264270
// $useSoftDeletes = true
265271
$this->createModel(UserModel::class);
@@ -268,11 +274,91 @@ public function testDeleteWithSoftDeleteThrowDatabaseExceptionWithoutWhereClause
268274
}
269275

270276
public static function emptyPkValues(): iterable
277+
{
278+
return [
279+
'null' => [
280+
null,
281+
DatabaseException::class,
282+
'Deletes are not allowed unless they contain a "where" or "like" clause.',
283+
],
284+
'false' => [
285+
false,
286+
InvalidArgumentException::class,
287+
'Invalid primary key: boolean false is not allowed.',
288+
],
289+
'0 integer' => [
290+
0,
291+
InvalidArgumentException::class,
292+
'Invalid primary key: 0 is not allowed.',
293+
],
294+
"'0' string" => [
295+
'0',
296+
InvalidArgumentException::class,
297+
"Invalid primary key: '0' is not allowed.",
298+
],
299+
'empty string' => [
300+
'',
301+
InvalidArgumentException::class,
302+
"Invalid primary key: '' is not allowed.",
303+
],
304+
'true' => [
305+
true,
306+
InvalidArgumentException::class,
307+
'Invalid primary key: boolean true is not allowed.',
308+
],
309+
'empty array' => [
310+
[],
311+
InvalidArgumentException::class,
312+
'Invalid primary key: cannot be an empty array.',
313+
],
314+
'nested array' => [
315+
[[1, 2]],
316+
InvalidArgumentException::class,
317+
'Invalid primary key at index 0: nested arrays are not allowed.',
318+
],
319+
'array with null' => [
320+
[1, null, 3],
321+
InvalidArgumentException::class,
322+
'Invalid primary key: NULL is not allowed.',
323+
],
324+
'array with 0' => [
325+
[1, 0, 3],
326+
InvalidArgumentException::class,
327+
'Invalid primary key: 0 is not allowed.',
328+
],
329+
"array with '0'" => [
330+
[1, '0', 3],
331+
InvalidArgumentException::class,
332+
"Invalid primary key: '0' is not allowed.",
333+
],
334+
'array with empty string' => [
335+
[1, '', 3],
336+
InvalidArgumentException::class,
337+
"Invalid primary key: '' is not allowed.",
338+
],
339+
'array with boolean' => [
340+
[1, false, 3],
341+
InvalidArgumentException::class,
342+
'Invalid primary key: boolean false is not allowed.',
343+
],
344+
];
345+
}
346+
347+
/**
348+
* Data provider for tests using where() clause.
349+
* These values go into WHERE clause, not through validateID().
350+
*
351+
* @return iterable<array{bool|int|string|null}>
352+
*/
353+
public static function emptyPkValuesWithWhereClause(): iterable
271354
{
272355
return [
273356
[0],
274357
[null],
275358
['0'],
359+
[''],
360+
[true],
361+
[false],
276362
];
277363
}
278364
}

0 commit comments

Comments
 (0)