28 June 2018, 14:35

Recently, we upgraded to Mahara 17.10 series and noticed following error flooding our error logs even between upgrade tasks as for some reason it failed to drop ‘testtable’ in ensure_upgrade_sanity >  mysql_has_trigger_privilege whenever it was called.

[DBG] 53 (lib/dml.php:157) mysqli error: [1050: Table 'testtable' already exists] in EXECUTE("CREATE TABLE "testtable" (testcolumn INT);")Command was: CREATE TABLE "testtable" (testcolumn INT);
[WAR] 53 (lib/errors.php:859) Could not execute command: CREATE TABLE "testtable" (testcolumn INT);
Call stack (most recent first):
* log_message(string(size 69), integer, true, true) at /www/htdocs/lib/errors.php:95
* log_warn(string(size 69)) at /www/htdocs/lib/errors.php:859
* SQLException->__construct(string(size 69)) at /www/htdocs/lib/dml.php:158
* execute_sql(string(size 42)) at /www/htdocs/lib/dml.php:1836
* mysql_has_trigger_privilege() at /www/htdocs/lib/mahara.php:194
* ensure_upgrade_sanity() at /www/htdocs/init.php:337
* require("/www/htdocs/init.php") at /www/htdocs/index.php:16

[WAR] 53 (lib/dml.php:158) Could not execute command: CREATE TABLE "testtable" (testcolumn INT);
Call stack (most recent first):
* execute_sql(string(size 42)) at /www/htdocs/lib/dml.php:1836
* mysql_has_trigger_privilege() at /www/htdocs/lib/mahara.php:194
* ensure_upgrade_sanity() at /www/htdocs/init.php:337
* require("/www/htdocs/init.php") at /www/htdocs/index.php:16

This routine is being called all the time through init.php when it detects upgrade, either someone hit homepage or every minute as part of cron.php (require(dirname(dirname(__FILE__)).'/init.php');).

If this routine is called whenever the upgrade script (htdocs/admin/upgrade.php) is run then why this needs to be called through init.php even in between upgrade tasks, I think this is unnecessary unless it is serving another purpose that I don’t know. 

28 June 2018, 16:41

Hi Yaju,

Yeah, it does seem like a lot of overkill to be checking this on every page load.

It would be better to check it on install only and then set a flag in the database config table to say the trigger privs are ok.

And then drop that config table flag if we ever need to check the database again if some new trigger type privs need to be checked.

Because I imagine that once the database is set up for the first time it would be highly unlikely a sysadmin would come along and drop the trigger priv for the database owner.

I've added a bug report for this: https://bugs.launchpad.net/mahara/+bug/1779049

Cheers

Robert

29 June 2018, 12:10

Hi Robert,

If binary logging is enabled,  this requires either SUPER privileges or enabling the global log_bin_trust_function_creators system variable for creating a trigger and this applies to both MySQL 5.7 & latest MySQL 8.0.

Having the SUPER privileges all the time is not a good practice, so enable this during an upgrade and revoke it as soon it finishes would be ideal practice or enable global variable log_bin_trust_function_creators permanently. Both of these steps are manual at this stage which requires a DBA for each upgrade being a larger organization unless Mahara states log_bin_trust_function_creators as a mandatory requirement when binary logging is enabled - wink ;).

For this scenario where binary logging is enabled and want to avoid SUPER privileges/log_bin_trust_function_creators permanently, the check should happen each time whenever an upgrade is invoked (htdocs/admin/upgrade.php - the function ensure_upgrade_sanity is already doing this). To minimize the impact this routine should be removed from init.php.

Kind regards,

Yaju Mahida