Skip to content

Commit

Permalink
Merge pull request #7759 from kenjis/fix-model-insertBatch-noAutoIncr…
Browse files Browse the repository at this point in the history
…ement

fix: Model::insertBatch() causes error to non auto increment table
  • Loading branch information
kenjis committed Aug 4, 2023
2 parents de129dc + a2c4749 commit dd7a139
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 4 deletions.
24 changes: 20 additions & 4 deletions system/BaseModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ public function insertBatch(?array $set = null, ?bool $escape = null, int $batch

// Must be called first so we don't
// strip out created_at values.
$row = $this->doProtectFields($row);
$row = $this->doProtectFieldsForInsert($row);

// Set created_at and updated_at with same time
$date = $this->setDate();
Expand Down Expand Up @@ -1228,10 +1228,10 @@ public function protect(bool $protect = true)
}

/**
* Ensures that only the fields that are allowed to be updated
* are in the data array.
* Ensures that only the fields that are allowed to be updated are
* in the data array.
*
* Used by insert() and update() to protect against mass assignment
* Used by update() and updateBatch() to protect against mass assignment
* vulnerabilities.
*
* @param array $data Data
Expand All @@ -1257,6 +1257,22 @@ protected function doProtectFields(array $data): array
return $data;
}

/**
* Ensures that only the fields that are allowed to be inserted are in
* the data array.
*
* Used by insert() and insertBatch() to protect against mass assignment
* vulnerabilities.
*
* @param array $data Data
*
* @throws DataException
*/
protected function doProtectFieldsForInsert(array $data): array
{
return $this->doProtectFields($data);
}

/**
* Sets the date or current date if null value is passed.
*
Expand Down
35 changes: 35 additions & 0 deletions system/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,41 @@ public function insert($data = null, bool $returnID = true)
return parent::insert($data, $returnID);
}

/**
* Ensures that only the fields that are allowed to be inserted are in
* the data array.
*
* Used by insert() and insertBatch() to protect against mass assignment
* vulnerabilities.
*
* @param array $data Data
*
* @throws DataException
*/
protected function doProtectFieldsForInsert(array $data): array
{
if (! $this->protectFields) {
return $data;
}

if (empty($this->allowedFields)) {
throw DataException::forInvalidAllowedFields(static::class);
}

foreach (array_keys($data) as $key) {
// Do not remove the non-auto-incrementing primary key data.
if ($this->useAutoIncrement === false && $key === $this->primaryKey) {
continue;
}

if (! in_array($key, $this->allowedFields, true)) {
unset($data[$key]);
}
}

return $data;
}

/**
* Updates a single record in the database. If an object is provided,
* it will attempt to convert it into an array.
Expand Down
20 changes: 20 additions & 0 deletions tests/system/Models/InsertModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,26 @@ public function testInsertBatchSuccess(): void
$this->seeInDatabase('job', ['name' => 'Cab Driver']);
}

public function testInsertBatchUseAutoIncrementSetToFalse(): void
{
$insertData = [
[
'key' => 'key1',
'value' => 'value1',
],
[
'key' => 'key2',
'value' => 'value2',
],
];

$this->createModel(WithoutAutoIncrementModel::class);
$this->model->insertBatch($insertData);

$this->seeInDatabase('without_auto_increment', ['key' => 'key1']);
$this->seeInDatabase('without_auto_increment', ['key' => 'key2']);
}

public function testInsertBatchValidationFail(): void
{
$jobData = [
Expand Down

0 comments on commit dd7a139

Please sign in to comment.