Hi Sergei, On 07/24/2015 10:55 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Jul 24, Alexander Barkov wrote:
Hi Sergei,
Sorry for delay, I was busy with 10.1 issues.
That's perfectly fine. And even good - there's little sense to spend time on 10.0 bugs when the next release in line is 10.1.
Thanks for review. A new patch is attached. This is a major rewrite since last time. I think the code now looks much easier to understand.
Not really, I couldn't understand what you're trying to do this time :( Besides it's 32K of changes in 10.0-GA, and for what? Corner case CONNECT issue. Could you try to come up with a less intrusive solution, please?
Let me try to explain. The idea is very simple, really. Calling check_access() inside external_lock() is a bad idea: - it's simply not correct in case of VIEW or SP, because it checks privileges for the current INVOKER and ignores the "SQL SECURITY DEFINER" clause. - and by the way, it's slow, and if it's already done in sql_oarse.cc, why call it for the second time? It's nice to reuse the value returned in the first call of check_access(). The correct privilege value that takes into account "SQL SECURITY DEFINER" becomes known in sql_parse.cc, in mysql_execute_command() and its callees, like insert_precheck() etc. So I'm trying to make this correct privilege value be consistently available in external_lock() for all SQLCOM_XXXX commands by assigning it to table->grant.privilege. Before the patch, table->grant.privilege is just set to 0 in many cases and therefore is useless in external_lock(). In order to initialize table->grant.privilege properly in all cases, I added a new parameter "ulong privilege" to these functions: ha_create_table() open_table_uncached() open_table_from_share(). The idea is to make the correct privilege value available at the point where a TABLE instance is initialized and assign this value to table->grant.privilege instead of 0. Does this explanation help? The patch looks quite safe for 10.0 for me: It does not seem to affect the other code, except ConnectSE. It should not be harmful to have the real privilege value instead of 0 in table->grant.privilege. Or, can it be harmful somehow? Thanks.
Regards, Sergei