Deleting group with permissions gives SQL syntax errors

[PeterTillema]

When you're trying to delete a group which already has some permissions, it gives me a SQL syntax error:

Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ') WHERE `id` = '1'' at line 1 with query: "UPDATE `users_group_permissions` SET `group_id` = null, `actions` = () WHERE `id` = '1'"

I've created some test code which replicates the bug:

$group = \Auth\Model\Auth_Group::forge([
    'name' => 'Test',
    'user_id' => 0
]);
$perm = \Auth\Model\Auth_Permission::forge([
    'area' => 'test',
    'permission' => 'test',
    'description' => 'test',
    'action' => '',
    'user_id' => 0
]);

try {
    $perm->save();
    $group->permissions[] = $perm;
    $group->save();

    $group->delete();
} catch (\Exception $ex) {
    \Debug::dump($ex->getMessage());
}

After some debugging I'm still not sure what would cause this, but it turns that removing the grouppermission has_many relationship "fixes" it. Dunno if that's wanted, but just sayin'.

[WanWizard]

It might be caused by https://github.com/fuel/auth/blob/1.9/develop/classes/model/auth/group.php#L96. I see User and Role has the same config.

I wonder if that is right. given the fact this is a ManyMany, and deleting a group or deleting a permission should also delete the relation record.

[PeterTillema]

I think I found the problem. The default for action in Auth_Grouppermission is an empty array, which FuelPHP stringifies as something like '(' . implode(', ', <array>) . ')'. However, it doesn't do this as a string, so it becomes () rather than '()'. Also, it doesn't even call the data_type, so that probably should be changed @WanWizard.

[WanWizard]

I think the default is wrong, it should be either NULL or "", an array is not a valid value as the database expects a string. I'll have to check the code to see if only changing the default doesn't have side-effects.

[WanWizard]

You may have another problem. In Auth_Grouppermission, actions is defined with data_type serialize, which means that the ORM should serialize() it on write, and unserialize() it on read.

If that doesn't happen, the Observer_Typing, which is defined in the Model, doesn't run, and the DB query indeed gets an array passed to it.

[PeterTillema]

Found the culprit: https://github.com/fuel/orm/blob/1.9/develop/classes/observer/typing.php#L173-L191. The observer is fired, but since the value is null and the event before, it entirely skips the typing at all. Even setting a default value doesn't work, as line 181-188 is skipped as well. Maybe the serialize typing is the exception, and should be added to this if-clause? Or rather, anything which replaces null with something, so serialize, encrypt, json and maybe a few more. How do you feel about this?

To say even more: it uses the default value, defined in the properties of the field, which is... an empty array, which of course translates to (). Either the default value should be changed, or the typing should also apply on the default value, no idea which one is better, but I'm in favor of typing the default value. If I manually change the actions default to '', it works properly.

[PeterTillema]

Well, apparently this is already fixed, be it in a development branch. Will close this, since it doesn't require more action.

https://github.com/fuel/orm/commit/f6831aabb2ee7521363210cd21d76006fbd1046a