Bug: Model inserts the casted $primaryKey value when using an entity and casts.

MaulanaMalikS commented 1 year ago

PHP Version

8.2

CodeIgniter4 Version

4.3.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

cli-server (PHP built-in webserver)

Database

No response

What happened?

If I understand correctly, based on the documentation, casting only occurs when a value is read, not when a value is inserted, and only raw values are inserted.

However, when using an entity and casting the primary key, the casted value is inserted instead of the raw value.

For example, I am using the Entity constructor to generate a binary representation of UUID for the primary key:

public function __construct(?array $data = null)
{
    parent::__construct($data);

    $this->attributes['id'] = Services::uuid()->generateBinary();
}

When using toRawArray(), all values are correct and ready to be inserted into the database. The $primaryKey value is a binary string. When using toArray(), all values are casted correctly, including the $primaryKey being casted to a string representation.

However, when I insert the entity into the model, the inserted $primaryKey value is casted. When I check the database, string representations are inserted instead of the raw binary representation.

Steps to Reproduce

Create an Entity and use casting on the primary key. Here is my example:
```
<?php
```

namespace App\Entities\Application;

use CodeIgniter\Entity\Entity; use App\Entities\Cast\Uuid as CastUuid; use Config\Services;

/**

@property string $id
@property string $name
@property string $logo */ class Application extends Entity { protected $dates = ['deleted_at']; protected $casts = [ 'id' => 'uuid', ];

protected $castHandlers = [ 'uuid' => CastUuid::class ];

public function construct(?array $data = null) { parent::construct($data);
```
$this->attributes['id'] = Services::uuid()->generateBinary();
```
} }

Create a Model and set $returnType to the Entity. Here is my example:
```
<?php
```

namespace App\Models\Application;

use CodeIgniter\Model; use App\Entities\Application\Application as ApplicationEntitiy;

class Application extends Model { protected $table = 'm_application'; protected $primaryKey = 'id'; protected $useAutoIncrement = false; protected $returnType = ApplicationEntitiy::class; protected $useSoftDeletes = true; protected $allowedFields = ['name', 'logo'];

// Dates
protected $useTimestamps = true;
protected $dateFormat    = 'datetime';
protected $createdField  = null;
protected $updatedField  = null;
protected $deletedField  = 'deleted_at';

public function getEntity(array $data = null): ApplicationEntitiy
{
    return new ApplicationEntitiy($data);
}

}


3. Insert the data. In this case, I am using a Seeder:
```php
<?php

namespace App\Database\Seeds;

use CodeIgniter\Database\Seeder;
use App\Models\Application\Application;

class ApplicationSeeder extends Seeder
{
    public function run()
    {
        $model = model(Application::class);
        $entity = $model->getEntity();

        $data = [
            'name' => 'User Management'
        ];

        $entity->fill($data);
        var_dump($model->insert($entity));
    }
}

Expected Output

The primary key should be inserted as a raw value.

Anything else?

My guess is that this code here: https://github.com/codeigniter4/CodeIgniter4/blob/c640145b5ffa2b81e7a52a684fd4232b6ccb6dec/system/Model.php#L801 gets the property directly, instead of using cast(false) or toRawArray().

neznaika0 commented 1 year ago

Which library is used for UUID?

kenjis commented 1 year ago

Your sample code does not work.

kenjis commented 1 year ago

@MaulanaMalikS https://github.com/codeigniter4/CodeIgniter4/blob/c640145b5ffa2b81e7a52a684fd4232b6ccb6dec/system/Model.php#L801 Yes, you are probably right. Check the value:

--- a/system/Model.php
+++ b/system/Model.php
@@ -787,6 +787,7 @@ class Model extends BaseModel
     protected function objectToRawArray($data, bool $onlyChanged = true, bool $recursive = false): ?array
     {
         $properties = parent::objectToRawArray($data, $onlyChanged);
+        d($properties);

         // Always grab the primary key otherwise updates will fail.
         if (
@@ -800,6 +801,7 @@ class Model extends BaseModel
         ) {
             $properties[$this->primaryKey] = $data->{$this->primaryKey};
         }
+        dd($properties);

         return $properties;
     }

MaulanaMalikS commented 1 year ago

Which library is used for UUID?

i am using ramsey/uuid and keiko/uuid-shortener

Your sample code does not work.

Might be I am not including this:

<?php

namespace App\Entities\Cast;

use CodeIgniter\Entity\Cast\BaseCast;
use Config\Services;

class Uuid extends BaseCast
{
    public static function get($value, array $params = []): string
    {
        return Services::uuid()->shortFromBinary($value);
    }

    public static function set($value, array $params = [])
    {
        return Services::uuid()->binaryFromShort($value);
    }
}

--- a/system/Model.php
+++ b/system/Model.php
@@ -787,6 +787,7 @@ class Model extends BaseModel
     protected function objectToRawArray($data, bool $onlyChanged = true, bool $recursive = false): ?array
     {
         $properties = parent::objectToRawArray($data, $onlyChanged);
+        d($properties);

         // Always grab the primary key otherwise updates will fail.
         if (
@@ -800,6 +801,7 @@ class Model extends BaseModel
         ) {
             $properties[$this->primaryKey] = $data->{$this->primaryKey};
         }
+        dd($properties);

         return $properties;
     }

resulting this:

┌──────────────────────────────────────────────────────────────────────────────┐
│ $properties                                                                  │
└──────────────────────────────────────────────────────────────────────────────┘
array (2) [
    'id' => binary string (16) "=sMS  -|"
    'name' => string (15) "User Management"
]
════════════════════════════════════════════════════════════════════════════════
Called from .../Model.php:773 [d()]

┌──────────────────────────────────────────────────────────────────────────────┐
│ $properties                                                                  │
└──────────────────────────────────────────────────────────────────────────────┘
array (2) [
    'id' => string (21) "RbVzytm63VBCXKHQrELcH"
    'name' => string (15) "User Management"
]
════════════════════════════════════════════════════════════════════════════════
Called from .../Model.php:787 [dd()]

kenjis commented 1 year ago

@MaulanaMalikS Thank you. The bug is in it.

If I understand correctly, based on the documentation, casting only occurs when a value is read, not when a value is inserted, and only raw values are inserted.

The documentation is wrong. Casting works both read (get) and write (set). Otherwise, the following sample does not work:

$widget->colors = ['red', 'yellow', 'green'];

https://www.codeigniter.com/user_guide/models/entities.html#csv-casting

kenjis commented 1 year ago

Try:

--- a/system/Model.php
+++ b/system/Model.php
@@ -798,7 +798,16 @@ class Model extends BaseModel
                 && ! empty($data->{$this->primaryKey})
             )
         ) {
+            if (method_exists($data, 'cast')) {
+                $cast = $data->cast(false);
+                $data->cast(false);
+            }
+
             $properties[$this->primaryKey] = $data->{$this->primaryKey};
+
+            if (method_exists($data, 'cast')) {
+                $data->cast($cast);
+            }
         }

         return $properties;

MaulanaMalikS commented 1 year ago

Try:

--- a/system/Model.php
+++ b/system/Model.php
@@ -798,7 +798,16 @@ class Model extends BaseModel
                 && ! empty($data->{$this->primaryKey})
             )
         ) {
+            if (method_exists($data, 'cast')) {
+                $cast = $data->cast(false);
+                $data->cast(false);
+            }
+
             $properties[$this->primaryKey] = $data->{$this->primaryKey};
+
+            if (method_exists($data, 'cast')) {
+                $data->cast($cast);
+            }
         }

         return $properties;

return error :

[TypeError]

CodeIgniter\Entity\Entity::cast(): Argument #1 ($cast) must be of type ?bool, App\Entities\Application\Application given, called in D:\Apache24\htdocs\hse-app-usman\vendor\codeigniter4\framework\system\Model.php on line 792

at SYSTEMPATH\Entity\Entity.php:410

Backtrace:
  1    SYSTEMPATH\Model.php:792
       CodeIgniter\Entity\Entity()->cast(Object(App\Entities\Application\Application))

  2    SYSTEMPATH\BaseModel.php:1613
       CodeIgniter\Model()->objectToRawArray(Object(App\Entities\Application\Application), false, true)

  3    SYSTEMPATH\BaseModel.php:1691
       CodeIgniter\BaseModel()->objectToArray(Object(App\Entities\Application\Application), false, true)

  4    SYSTEMPATH\BaseModel.php:742
       CodeIgniter\BaseModel()->transformDataToArray(Object(App\Entities\Application\Application), 'insert')

  5    SYSTEMPATH\Model.php:730
       CodeIgniter\BaseModel()->insert(Object(App\Entities\Application\Application), true)

  6    APPPATH\Database\Seeds\ApplicationSeeder.php:20
       CodeIgniter\Model()->insert(Object(App\Entities\Application\Application))

  7    SYSTEMPATH\Database\Seeder.php:146
       App\Database\Seeds\ApplicationSeeder()->run()

  8    SYSTEMPATH\Commands\Database\Seed.php:77
       CodeIgniter\Database\Seeder()->call('App\\Database\\Seeds\\ApplicationSeeder')

  9    SYSTEMPATH\CLI\Commands.php:65
       CodeIgniter\Commands\Database\Seed()->run([])

 10    SYSTEMPATH\CLI\Console.php:37
       CodeIgniter\CLI\Commands()->run('db:seed', [...])

 11    ROOTPATH\spark:97
       CodeIgniter\CLI\Console()->run()

However, when I use the following:

            if (method_exists($data, 'cast')) {
                $data->cast(false);
            }

            $properties[$this->primaryKey] = $data->{$this->primaryKey};

it still inserts casted values.

Casting might occur in this code: https://github.com/codeigniter4/CodeIgniter4/blob/8a74161a2db80a27ea1f43dd69bd6f4940ca86ff/system/Model.php#L723

Because i am using v4.3.3, that line has been removed in the develop branch.

kenjis commented 1 year ago

Sorry one line was not correct.

--- a/system/Model.php
+++ b/system/Model.php
@@ -798,7 +798,16 @@ class Model extends BaseModel
                 && ! empty($data->{$this->primaryKey})
             )
         ) {
+            if (method_exists($data, 'cast')) {
+                $cast = $data->cast();
+                $data->cast(false);
+            }
+
             $properties[$this->primaryKey] = $data->{$this->primaryKey};
+
+            if (method_exists($data, 'cast')) {
+                $data->cast($cast);
+            }
         }

         return $properties;

kenjis commented 1 year ago

$data->{$this->primaryKey} returns cast value by default. Try wrapping it by the if statements above.

MaulanaMalikS commented 1 year ago

$data->{$this->primaryKey} returns cast value by default. Try wrapping it by the if statements above.

It works! Maybe I can override the objectToRawArray() method, but I don't know about the insert() method because the $tempPrimaryKeyValue is private.

For now, I think I'll either insert without using an Entity, or just insert the array using the toRawArray() method of the Entity.

kenjis commented 1 year ago

4.3.3 has a known vulnerability. See https://github.com/codeigniter4/CodeIgniter4/security/advisories I recommend you upgrade to 4.3.7.

kenjis commented 1 year ago

I sent PRs to fix: #7806 and #7807

kenjis commented 9 months ago

I thought I had fixed this bug. However, when I wrote the test code, it failed. See #8282 If there is something wrong with the new PR code, let me know.

And it seems using binary column works only on MySQL. Other databases need special escaping for binary data, so it seems not easy to use binary column.

codeigniter4 / CodeIgniter4