From 4bfd2121776d0ce7ebd6761b807f083da87cb185 Mon Sep 17 00:00:00 2001 From: Jorgen Loland Date: Mon, 15 Nov 2010 16:18:04 +0100 Subject: [PATCH] Bug#54812: assert in Diagnostics_area::set_ok_status during EXPLAIN Before the patch, send_eof() of some subclasses of select_result (e.g., select_send::send_eof()) could handle being called after an error had occured while others could not. The methods that were not well-behaved would trigger an ASSERT on debug builds. Release builds were not affected. Consider the following query as an example for how the ASSERT could be triggered: A user without execute privilege on f() does SELECT MAX(key1) INTO @dummy FROM t1 WHERE f() < 1; resulting in "ERROR 42000: execute command denied to user..." The server would end the query by calling send_eof(). The fact that the error had occured would make the ASSERT trigger. select_dumpvar::send_eof() was the offending method in the bug report, but the problem also applied to other subclasses of select_result. This patch uniforms send_eof() of all subclasses of select_result to handle being called after an error has occured. --- mysql-test/r/not_embedded_server.result | 29 +++++++++++++ mysql-test/t/not_embedded_server.test | 54 +++++++++++++++++++++++++ sql/sql_class.cc | 12 +++++- sql/sql_insert.cc | 7 +++- sql/sql_prepare.cc | 9 ++++- sql/sql_update.cc | 4 +- 6 files changed, 109 insertions(+), 6 deletions(-) diff --git a/mysql-test/r/not_embedded_server.result b/mysql-test/r/not_embedded_server.result index fac38624695..ce229bf3e2e 100644 --- a/mysql-test/r/not_embedded_server.result +++ b/mysql-test/r/not_embedded_server.result @@ -14,3 +14,32 @@ flush privileges; ERROR HY000: Table 'host' was not locked with LOCK TABLES unlock tables; drop table t1; +# +# Bug#54812: assert in Diagnostics_area::set_ok_status during EXPLAIN +# +CREATE USER nopriv_user@localhost; +connection: default +DROP TABLE IF EXISTS t1,t2,t3; +DROP FUNCTION IF EXISTS f; +CREATE TABLE t1 (key1 INT PRIMARY KEY); +CREATE TABLE t2 (key2 INT); +INSERT INTO t1 VALUES (1),(2); +CREATE FUNCTION f() RETURNS INT RETURN 1; +GRANT FILE ON *.* TO 'nopriv_user'@'localhost'; +FLUSH PRIVILEGES; +connection: con1 +SELECT MAX(key1) FROM t1 WHERE f() < 1 INTO OUTFILE 'mytest'; +ERROR 42000: execute command denied to user 'nopriv_user'@'localhost' for routine 'test.f' +INSERT INTO t2 SELECT MAX(key1) FROM t1 WHERE f() < 1; +ERROR 42000: execute command denied to user 'nopriv_user'@'localhost' for routine 'test.f' +SELECT MAX(key1) INTO @dummy FROM t1 WHERE f() < 1; +ERROR 42000: execute command denied to user 'nopriv_user'@'localhost' for routine 'test.f' +CREATE TABLE t3 (i INT) AS SELECT MAX(key1) FROM t1 WHERE f() < 1; +ERROR 42000: execute command denied to user 'nopriv_user'@'localhost' for routine 'test.f' +connection: default +DROP TABLE t1,t2; +DROP FUNCTION f; +DROP USER nopriv_user@localhost; +# +# End Bug#54812 +# diff --git a/mysql-test/t/not_embedded_server.test b/mysql-test/t/not_embedded_server.test index 917d5871682..3fea1f630e0 100644 --- a/mysql-test/t/not_embedded_server.test +++ b/mysql-test/t/not_embedded_server.test @@ -54,3 +54,57 @@ lock tables t1 read; flush privileges; unlock tables; drop table t1; + +--echo # +--echo # Bug#54812: assert in Diagnostics_area::set_ok_status during EXPLAIN +--echo # + +CREATE USER nopriv_user@localhost; + +connection default; +--echo connection: default + +--disable_warnings +DROP TABLE IF EXISTS t1,t2,t3; +DROP FUNCTION IF EXISTS f; +--enable_warnings + +CREATE TABLE t1 (key1 INT PRIMARY KEY); +CREATE TABLE t2 (key2 INT); +INSERT INTO t1 VALUES (1),(2); + +CREATE FUNCTION f() RETURNS INT RETURN 1; + +GRANT FILE ON *.* TO 'nopriv_user'@'localhost'; + +FLUSH PRIVILEGES; + +connect (con1,localhost,nopriv_user,,); +connection con1; +--echo connection: con1 + +--error ER_PROCACCESS_DENIED_ERROR +SELECT MAX(key1) FROM t1 WHERE f() < 1 INTO OUTFILE 'mytest'; + +--error ER_PROCACCESS_DENIED_ERROR +INSERT INTO t2 SELECT MAX(key1) FROM t1 WHERE f() < 1; + +--error ER_PROCACCESS_DENIED_ERROR +SELECT MAX(key1) INTO @dummy FROM t1 WHERE f() < 1; + +--error ER_PROCACCESS_DENIED_ERROR +CREATE TABLE t3 (i INT) AS SELECT MAX(key1) FROM t1 WHERE f() < 1; + +disconnect con1; +--source include/wait_until_disconnected.inc + +connection default; +--echo connection: default + +DROP TABLE t1,t2; +DROP FUNCTION f; +DROP USER nopriv_user@localhost; + +--echo # +--echo # End Bug#54812 +--echo # diff --git a/sql/sql_class.cc b/sql/sql_class.cc index daf9217a5a1..fc4ab1bd27b 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1842,8 +1842,9 @@ void select_to_file::send_error(uint errcode,const char *err) bool select_to_file::send_eof() { int error= test(end_io_cache(&cache)); - if (mysql_file_close(file, MYF(MY_WME))) - error= 1; + if (mysql_file_close(file, MYF(MY_WME)) || thd->is_error()) + error= true; + if (!error) { ::my_ok(thd,row_count); @@ -2884,6 +2885,13 @@ bool select_dumpvar::send_eof() if (! row_count) push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_SP_FETCH_NO_DATA, ER(ER_SP_FETCH_NO_DATA)); + /* + Don't send EOF if we're in error condition (which implies we've already + sent or are sending an error) + */ + if (thd->is_error()) + return true; + ::my_ok(thd,row_count); return 0; } diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 1f8da3fab5c..a81c9ae15a0 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -3506,6 +3506,9 @@ bool select_insert::send_eof() error= (thd->locked_tables_mode <= LTM_LOCK_TABLES ? table->file->ha_end_bulk_insert() : 0); + if (!error && thd->is_error()) + error= thd->stmt_da->sql_errno(); + table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); @@ -4049,7 +4052,7 @@ bool select_create::send_eof() { bool tmp=select_insert::send_eof(); if (tmp) - abort(); + abort_result_set(); else { /* @@ -4081,7 +4084,7 @@ void select_create::abort_result_set() DBUG_ENTER("select_create::abort_result_set"); /* - In select_insert::abort() we roll back the statement, including + In select_insert::abort_result_set() we roll back the statement, including truncating the transaction cache of the binary log. To do this, we pretend that the statement is transactional, even though it might be the case that it was not. diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 6a84d050d7f..851782c623f 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2898,8 +2898,15 @@ bool Select_fetch_protocol_binary::send_result_set_metadata(List &list, ui bool Select_fetch_protocol_binary::send_eof() { + /* + Don't send EOF if we're in error condition (which implies we've already + sent or are sending an error) + */ + if (thd->is_error()) + return true; + ::my_eof(thd); - return FALSE; + return false; } diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 96b1ac67b49..8cc8d511fd6 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -2064,7 +2064,9 @@ bool multi_update::send_eof() Does updates for the last n - 1 tables, returns 0 if ok; error takes into account killed status gained in do_updates() */ - int local_error = (table_count) ? do_updates() : 0; + int local_error= thd->is_error(); + if (!local_error) + local_error = (table_count) ? do_updates() : 0; /* if local_error is not set ON until after do_updates() then later carried out killing should not affect binlogging.