New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GII generates rules for unique indexes #1624

Merged
merged 3 commits into from Dec 26, 2013

Conversation

Projects
None yet
2 participants
@lucianobaraglia
Contributor

lucianobaraglia commented Dec 26, 2013

Related to #1621

Some discussion here: #1623 (comment)

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

Does it handle auto-incremental PKs? They should not be added to validation rules because their values are not supposed to be entered by end users.

Member

qiangxue commented Dec 26, 2013

Does it handle auto-incremental PKs? They should not be added to validation rules because their values are not supposed to be entered by end users.

@lucianobaraglia

This comment has been minimized.

Show comment
Hide comment
@lucianobaraglia

lucianobaraglia Dec 26, 2013

Contributor

In the case of MySql, the only UNIQUE KEYS taken are from the result of SHOW CREATE TABLE: https://github.com/yiisoft/yii2/blob/master/framework/yii/db/mysql/Schema.php#L267

In Pgsql, the unique indexes are taken where the index is not a primary key: https://github.com/yiisoft/yii2/blob/master/framework/yii/db/pgsql/Schema.php#L252

In Sqlite, only indexes from PRAGMA index_list and PRAGMA index_info are taken into account, and this queries shouldn't return a PK: https://github.com/yiisoft/yii2/blob/master/framework/yii/db/sqlite/Schema.php#L177

Contributor

lucianobaraglia commented Dec 26, 2013

In the case of MySql, the only UNIQUE KEYS taken are from the result of SHOW CREATE TABLE: https://github.com/yiisoft/yii2/blob/master/framework/yii/db/mysql/Schema.php#L267

In Pgsql, the unique indexes are taken where the index is not a primary key: https://github.com/yiisoft/yii2/blob/master/framework/yii/db/pgsql/Schema.php#L252

In Sqlite, only indexes from PRAGMA index_list and PRAGMA index_info are taken into account, and this queries shouldn't return a PK: https://github.com/yiisoft/yii2/blob/master/framework/yii/db/sqlite/Schema.php#L177

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

So it's still possible some DBMS may return PKs as unique keys? I think the generator code needs to check this case to prevent creating unique rules for auto-incremental PKs. Otherwise, we would have security problem.

Member

qiangxue commented Dec 26, 2013

So it's still possible some DBMS may return PKs as unique keys? I think the generator code needs to check this case to prevent creating unique rules for auto-incremental PKs. Otherwise, we would have security problem.

@lucianobaraglia

This comment has been minimized.

Show comment
Hide comment
@lucianobaraglia

lucianobaraglia Dec 26, 2013

Contributor

Sorry, I still dont get your idea.

This check should be done in the method that look for indexes or in GII? In
Schema::findUniqueIndexes() do exist a check.

Anyway, can you give me a little example of the security problem we need to
avoid.

Thanks

Luciano Baraglia

Desde el celular.
El dic 26, 2013 12:08 p.m., "Qiang Xue" notifications@github.com escribió:

So it's still possible some DBMS may return PKs as unique keys? I think
the generator code needs to check this case to prevent creating unique
rules for auto-incremental PKs. Otherwise, we would have security problem.


Reply to this email directly or view it on GitHubhttps://github.com/yiisoft/yii2/pull/1624#issuecomment-31223587
.

Contributor

lucianobaraglia commented Dec 26, 2013

Sorry, I still dont get your idea.

This check should be done in the method that look for indexes or in GII? In
Schema::findUniqueIndexes() do exist a check.

Anyway, can you give me a little example of the security problem we need to
avoid.

Thanks

Luciano Baraglia

Desde el celular.
El dic 26, 2013 12:08 p.m., "Qiang Xue" notifications@github.com escribió:

So it's still possible some DBMS may return PKs as unique keys? I think
the generator code needs to check this case to prevent creating unique
rules for auto-incremental PKs. Otherwise, we would have security problem.


Reply to this email directly or view it on GitHubhttps://github.com/yiisoft/yii2/pull/1624#issuecomment-31223587
.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

It should be done Gii. Just add a check like this:

            if ($column->autoIncrement) {
                continue;
            }

Normally, you shouldn't add an autoincremental attribute to validation rules because it would otherwise mark the attribute to be safe and can be updated by end users.

Member

qiangxue commented Dec 26, 2013

It should be done Gii. Just add a check like this:

            if ($column->autoIncrement) {
                continue;
            }

Normally, you shouldn't add an autoincremental attribute to validation rules because it would otherwise mark the attribute to be safe and can be updated by end users.

@lucianobaraglia

This comment has been minimized.

Show comment
Hide comment
@lucianobaraglia

lucianobaraglia Dec 26, 2013

Contributor

@qiangxue do you want to check now?
If any of the columns in the unique index is auto incrementable, it doesn't generate the rule.

Contributor

lucianobaraglia commented Dec 26, 2013

@qiangxue do you want to check now?
If any of the columns in the unique index is auto incrementable, it doesn't generate the rule.

qiangxue added a commit that referenced this pull request Dec 26, 2013

Merge pull request #1624 from lucianobaraglia/master
GII generates rules for unique indexes

@qiangxue qiangxue merged commit fc19ebd into yiisoft:master Dec 26, 2013

1 check was pending

default The Travis CI build is in progress
Details
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

Looks good to me. Thanks!

Member

qiangxue commented Dec 26, 2013

Looks good to me. Thanks!

@lucianobaraglia

This comment has been minimized.

Show comment
Hide comment
@lucianobaraglia

lucianobaraglia Dec 26, 2013

Contributor

😃

Contributor

lucianobaraglia commented Dec 26, 2013

😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment