Description:
In 5.7 and older applications would always get full detail about failed operation due to constraint violations, but in 8.0 an ACL check was introduced, and now the error can have two different error codes: 1217 and 1451:
ERROR 1217 (23000) at line 1: Cannot delete or update a parent row: a foreign key constraint fails
ERROR 1451 (23000) at line 1: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`joinit`, CONSTRAINT `joinit_ibfk_1` FOREIGN KEY (`g`) REFERENCES `joinit_fk` (`i`) ON DELETE RESTRICT ON UPDATE CASCADE)
Now this brings two problems:
1) Applications that handled error 1451 have to be modified to handle additional error codes (so it breaks backwards compatibility)
2) For applications that require to see the 1451 version of the error, the following privileges are required:
SELECT
DELETE
UPDATE
TRIGGER
REFERENCES
INDEX
INSERT
CREATE VIEW
DROP
SHOW VIEW
CREATE
ALTER
I can, of course, agree with many of those being required in the interest of security...but DROP, CREATE and ALTER to show me an error for a DELETE? That seems pretty excessive. I understand all those could lead to a FK error, but it looks like the error should be handled more granularly, as having to grant all this stuff to an application user makes many people justifiably nervous :-D
Hope it makes sense!
How to repeat:
Look at code
/**
An Internal_error_handler that converts errors related to foreign key
constraint checks 'ER_NO_REFERENCED_ROW_2' and 'ER_ROW_IS_REFERENCED_2'
to ER_NO_REFERENCED_ROW and ER_ROW_IS_REFERENCED based on privilege checks.
This prevents from revealing parent and child tables information respectively
when the foreign key constraint check fails and user does not have privileges
to access those tables.
*/
And looking at the code in sql/error_handler.cc we see:
bool Foreign_key_error_handler::handle_condition(
...
} else if (sql_errno == ER_ROW_IS_REFERENCED_2) {
for (TABLE_SHARE_FOREIGN_KEY_PARENT_INFO *fk_p = share->foreign_key_parent;
fk_p < share->foreign_key_parent + share->foreign_key_parents;
++fk_p) {
Table_ref table(fk_p->referencing_table_db.str,
fk_p->referencing_table_db.length,
fk_p->referencing_table_name.str,
fk_p->referencing_table_name.length, TL_READ);
if (check_table_access(m_thd, TABLE_OP_ACLS, &table, true, 1, true)) {
my_error(ER_ROW_IS_REFERENCED, MYF(0));
return true;
}
}
}
return false;
}
You can see the highlighted check_table_access call is checking for TABLE_OP_ACLS. This is a catch-all group, so that any action on the table that would break foreign key will throw the error; And we can see that TABLE_OP_ACLS is indeed composed of all these:
#define TABLE_OP_ACLS \
(SELECT_ACL | INSERT_ACL | UPDATE_ACL | DELETE_ACL | CREATE_ACL | DROP_ACL | \
REFERENCES_ACL | INDEX_ACL | ALTER_ACL | CREATE_VIEW_ACL | SHOW_VIEW_ACL | \
TRIGGER_ACL)
Suggested fix:
Break down the check into more discrete ACLs groups
Thanks!!! :-)
Description: In 5.7 and older applications would always get full detail about failed operation due to constraint violations, but in 8.0 an ACL check was introduced, and now the error can have two different error codes: 1217 and 1451: ERROR 1217 (23000) at line 1: Cannot delete or update a parent row: a foreign key constraint fails ERROR 1451 (23000) at line 1: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`joinit`, CONSTRAINT `joinit_ibfk_1` FOREIGN KEY (`g`) REFERENCES `joinit_fk` (`i`) ON DELETE RESTRICT ON UPDATE CASCADE) Now this brings two problems: 1) Applications that handled error 1451 have to be modified to handle additional error codes (so it breaks backwards compatibility) 2) For applications that require to see the 1451 version of the error, the following privileges are required: SELECT DELETE UPDATE TRIGGER REFERENCES INDEX INSERT CREATE VIEW DROP SHOW VIEW CREATE ALTER I can, of course, agree with many of those being required in the interest of security...but DROP, CREATE and ALTER to show me an error for a DELETE? That seems pretty excessive. I understand all those could lead to a FK error, but it looks like the error should be handled more granularly, as having to grant all this stuff to an application user makes many people justifiably nervous :-D Hope it makes sense! How to repeat: Look at code /** An Internal_error_handler that converts errors related to foreign key constraint checks 'ER_NO_REFERENCED_ROW_2' and 'ER_ROW_IS_REFERENCED_2' to ER_NO_REFERENCED_ROW and ER_ROW_IS_REFERENCED based on privilege checks. This prevents from revealing parent and child tables information respectively when the foreign key constraint check fails and user does not have privileges to access those tables. */ And looking at the code in sql/error_handler.cc we see: bool Foreign_key_error_handler::handle_condition( ... } else if (sql_errno == ER_ROW_IS_REFERENCED_2) { for (TABLE_SHARE_FOREIGN_KEY_PARENT_INFO *fk_p = share->foreign_key_parent; fk_p < share->foreign_key_parent + share->foreign_key_parents; ++fk_p) { Table_ref table(fk_p->referencing_table_db.str, fk_p->referencing_table_db.length, fk_p->referencing_table_name.str, fk_p->referencing_table_name.length, TL_READ); if (check_table_access(m_thd, TABLE_OP_ACLS, &table, true, 1, true)) { my_error(ER_ROW_IS_REFERENCED, MYF(0)); return true; } } } return false; } You can see the highlighted check_table_access call is checking for TABLE_OP_ACLS. This is a catch-all group, so that any action on the table that would break foreign key will throw the error; And we can see that TABLE_OP_ACLS is indeed composed of all these: #define TABLE_OP_ACLS \ (SELECT_ACL | INSERT_ACL | UPDATE_ACL | DELETE_ACL | CREATE_ACL | DROP_ACL | \ REFERENCES_ACL | INDEX_ACL | ALTER_ACL | CREATE_VIEW_ACL | SHOW_VIEW_ACL | \ TRIGGER_ACL) Suggested fix: Break down the check into more discrete ACLs groups Thanks!!! :-)