Commit ac10cb5b authored by Jorgen Loland's avatar Jorgen Loland

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:
  Added test for BUG#54812
mysql-test/t/not_embedded_server.test:
  Added test for BUG#54812
sql/sql_class.cc:
  send_eof() of all subclasses of select_result can now handle being
  called after an error has occured.
sql/sql_insert.cc:
  send_eof() of all subclasses of select_result can now handle being
  called after an error has occured.
  Also fix call to abort() in select_create::send_eof(), which was supposed to abort the result set, not terminate the server. This call to abort() should have been changed when the function was renamed from abort_result_set() but was forgotten. New test case added by BUG#54812 covered this line and terminated server.
sql/sql_prepare.cc:
  send_eof() of all subclasses of select_result can now handle being
  called after an error has occured.
sql/sql_update.cc:
  send_eof() of all subclasses of select_result can now handle being
  called after an error has occured.
parent 6574f4ba
...@@ -14,3 +14,32 @@ flush privileges; ...@@ -14,3 +14,32 @@ flush privileges;
ERROR HY000: Table 'host' was not locked with LOCK TABLES ERROR HY000: Table 'host' was not locked with LOCK TABLES
unlock tables; unlock tables;
drop table t1; 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
#
...@@ -54,3 +54,57 @@ lock tables t1 read; ...@@ -54,3 +54,57 @@ lock tables t1 read;
flush privileges; flush privileges;
unlock tables; unlock tables;
drop table t1; 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 #
...@@ -1842,8 +1842,9 @@ void select_to_file::send_error(uint errcode,const char *err) ...@@ -1842,8 +1842,9 @@ void select_to_file::send_error(uint errcode,const char *err)
bool select_to_file::send_eof() bool select_to_file::send_eof()
{ {
int error= test(end_io_cache(&cache)); int error= test(end_io_cache(&cache));
if (mysql_file_close(file, MYF(MY_WME))) if (mysql_file_close(file, MYF(MY_WME)) || thd->is_error())
error= 1; error= true;
if (!error) if (!error)
{ {
::my_ok(thd,row_count); ::my_ok(thd,row_count);
...@@ -2884,6 +2885,13 @@ bool select_dumpvar::send_eof() ...@@ -2884,6 +2885,13 @@ bool select_dumpvar::send_eof()
if (! row_count) if (! row_count)
push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
ER_SP_FETCH_NO_DATA, ER(ER_SP_FETCH_NO_DATA)); 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); ::my_ok(thd,row_count);
return 0; return 0;
} }
......
...@@ -3506,6 +3506,9 @@ bool select_insert::send_eof() ...@@ -3506,6 +3506,9 @@ bool select_insert::send_eof()
error= (thd->locked_tables_mode <= LTM_LOCK_TABLES ? error= (thd->locked_tables_mode <= LTM_LOCK_TABLES ?
table->file->ha_end_bulk_insert() : 0); 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_NO_IGNORE_DUP_KEY);
table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
...@@ -4049,7 +4052,7 @@ bool select_create::send_eof() ...@@ -4049,7 +4052,7 @@ bool select_create::send_eof()
{ {
bool tmp=select_insert::send_eof(); bool tmp=select_insert::send_eof();
if (tmp) if (tmp)
abort(); abort_result_set();
else else
{ {
/* /*
...@@ -4081,7 +4084,7 @@ void select_create::abort_result_set() ...@@ -4081,7 +4084,7 @@ void select_create::abort_result_set()
DBUG_ENTER("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 truncating the transaction cache of the binary log. To do this, we
pretend that the statement is transactional, even though it might pretend that the statement is transactional, even though it might
be the case that it was not. be the case that it was not.
......
...@@ -2898,8 +2898,15 @@ bool Select_fetch_protocol_binary::send_result_set_metadata(List<Item> &list, ui ...@@ -2898,8 +2898,15 @@ bool Select_fetch_protocol_binary::send_result_set_metadata(List<Item> &list, ui
bool Select_fetch_protocol_binary::send_eof() 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); ::my_eof(thd);
return FALSE; return false;
} }
......
...@@ -2064,7 +2064,9 @@ bool multi_update::send_eof() ...@@ -2064,7 +2064,9 @@ bool multi_update::send_eof()
Does updates for the last n - 1 tables, returns 0 if ok; Does updates for the last n - 1 tables, returns 0 if ok;
error takes into account killed status gained in do_updates() 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 if local_error is not set ON until after do_updates() then
later carried out killing should not affect binlogging. later carried out killing should not affect binlogging.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment