[Commits] 658ccc3f54b: MDEV-12666: CURRENT_ROLE() and DATABASE() does not work in a view

vicentiu vicentiu at mariadb.org
Wed Jun 7 14:17:48 EEST 2017


revision-id: 658ccc3f54b0cd6d48c191c4da11867ed98ba137 (mariadb-10.0.31-2-g658ccc3f54b)
parent(s): 54cd5bc703da08be45894495d7f3e2c8117617cd
author: Vicențiu Ciorbaru
committer: Vicențiu Ciorbaru
timestamp: 2017-06-07 14:17:20 +0300
message:

MDEV-12666: CURRENT_ROLE() and DATABASE() does not work in a view

The problem lies in how CURRENT_ROLE is defined. The
Item_func_current_role inherits from Item_func_sysconst, which defines
a safe_charset_converter to be a const_charset_converter.

During view creation, if there is no role previously set, the current_role()
function returns NULL.

This is captured on item instantiation and the
const_charset_converter call subsequently returns an Item_null.
In turn, the function is replaced with Item_null and the view is
then created with an Item_null instead of Item_func_current_role.

Without this patch, the first SHOW CREATE VIEW from the testcase would
have a where clause of WHERE role_name = NULL, while the second SHOW
CREATE VIEW would show a correctly created view.

The same applies for the DATABASE function, as it can change as well.

There is an additional problem with CURRENT_ROLE() when used in a
prepared statement. During prepared statement creation we used to set
the string_value of the function to the current role. Afterwards, we
would keep using that value, even if the CURRENT_ROLE() could change.

Item_func_current_user however can never be NULL so it did not show this
problem in a view before. At the same time, the CURRENT_USER() can not
be changed between prepared statement execution and creation so the
implementation where the value is stored during fix_fields is
sufficient.

Note also that DATABASE() function behaves differently during prepared
statements. See bug 25843 for details or commit
7e0ad09edff587dadc3e9855fc81e1b7de8f2199

---
 mysql-test/r/view.result                           |  62 +++++++++++++
 .../suite/roles/current_role_view-12666.result     | 103 +++++++++++++++++++++
 .../suite/roles/current_role_view-12666.test       | 102 ++++++++++++++++++++
 mysql-test/t/view.test                             |  49 ++++++++++
 sql/item.cc                                        |   1 +
 sql/item.h                                         |   2 +-
 sql/item_strfunc.cc                                |  50 ++++++++--
 sql/item_strfunc.h                                 |  12 +--
 8 files changed, 362 insertions(+), 19 deletions(-)

diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result
index d6da2a03b46..30c3aaba36e 100644
--- a/mysql-test/r/view.result
+++ b/mysql-test/r/view.result
@@ -5944,6 +5944,68 @@ use_case_id	InitialDeadline
 10	2015-12-18
 drop view v1;
 drop table t1;
+#
+# MDEV-12666: CURRENT_ROLE() and DATABASE() does not work in a view
+#
+# DATABASE() fails only when the initial view creation features a NULL
+# default database.
+#
+# CREATE, USE and DROP database so that we have no "default" database.
+#
+CREATE DATABASE temporary;
+USE temporary;
+DROP DATABASE temporary;
+SELECT DATABASE();
+DATABASE()
+NULL
+CREATE VIEW test.v_no_db AS SELECT DATABASE() = 'temporary_two';
+SHOW CREATE VIEW test.v_no_db;
+View	Create View	character_set_client	collation_connection
+v_no_db	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `test`.`v_no_db` AS select (database() = 'temporary_two') AS `DATABASE() = 'temporary_two'`	latin1	latin1_swedish_ci
+PREPARE prepared_no_database FROM "SELECT DATABASE() = 'temporary_two'";
+#
+# All statements should return NULL
+#
+EXECUTE prepared_no_database;
+DATABASE() = 'temporary_two'
+NULL
+SELECT DATABASE() = 'temporary_two';
+DATABASE() = 'temporary_two'
+NULL
+SELECT * FROM test.v_no_db;
+DATABASE() = 'temporary_two'
+NULL
+CREATE DATABASE temporary_two;
+USE temporary_two;
+CREATE VIEW test.v_with_db AS SELECT DATABASE() = 'temporary_two';
+PREPARE prepared_with_database FROM "SELECT DATABASE() = 'temporary_two'";
+#
+# All statements should return 1;
+#
+SELECT DATABASE() = 'temporary_two';
+DATABASE() = 'temporary_two'
+1
+SELECT * FROM test.v_no_db;
+DATABASE() = 'temporary_two'
+1
+SELECT * FROM test.v_with_db;
+DATABASE() = 'temporary_two'
+1
+EXECUTE prepared_with_database;
+DATABASE() = 'temporary_two'
+1
+#
+# Prepared statements maintain default database to be the same
+# during on creation so this should return NULL still.
+# See MySQL bug #25843
+#
+EXECUTE prepared_no_database;
+DATABASE() = 'temporary_two'
+NULL
+DROP DATABASE temporary_two;
+DROP VIEW test.v_no_db;
+DROP VIEW test.v_with_db;
+USE test;
 # -----------------------------------------------------------------
 # -- End of 10.0 tests.
 # -----------------------------------------------------------------
diff --git a/mysql-test/suite/roles/current_role_view-12666.result b/mysql-test/suite/roles/current_role_view-12666.result
new file mode 100644
index 00000000000..1d14593be4b
--- /dev/null
+++ b/mysql-test/suite/roles/current_role_view-12666.result
@@ -0,0 +1,103 @@
+CREATE USER has_role@'localhost';
+GRANT ALL PRIVILEGES ON *.* TO has_role@'localhost';
+CREATE ROLE test_role;
+GRANT test_role TO has_role@'localhost';
+CREATE USER no_role@'localhost';
+GRANT ALL PRIVILEGES ON *.* TO no_role@'localhost';
+CREATE TABLE view_role_test (
+id int primary key,
+role_name varchar(50)
+);
+INSERT INTO view_role_test VALUES (1, 'test_role');
+#
+# Use the same logic for stored procedures.
+#
+PREPARE prepared_no_current_role FROM "SELECT * from view_role_test WHERE role_name = CURRENT_ROLE()";
+#
+# Creating a view with no CURRENT_ROLE() set and one with CURRENT_ROLE()
+# set. Both should produce the same SHOW CREATE VIEW output.
+#
+CREATE
+DEFINER = no_role at localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test_no_current_role
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+SHOW CREATE VIEW v_view_role_test_no_current_role;
+View	Create View	character_set_client	collation_connection
+v_view_role_test_no_current_role	CREATE ALGORITHM=UNDEFINED DEFINER=`no_role`@`localhost` SQL SECURITY INVOKER VIEW `v_view_role_test_no_current_role` AS select `view_role_test`.`id` AS `id`,`view_role_test`.`role_name` AS `role_name` from `view_role_test` where (`view_role_test`.`role_name` = current_role())	latin1	latin1_swedish_ci
+#
+# No values should be returned
+#
+EXECUTE prepared_no_current_role;
+id	role_name
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+id	role_name
+SELECT * FROM v_view_role_test_no_current_role;
+id	role_name
+#
+# Now let's set the role. Create identical views as before. See if
+# their behaviour is different. It should not be.
+#
+SET ROLE test_role;
+SELECT CURRENT_USER();
+CURRENT_USER()
+root at localhost
+SELECT CURRENT_ROLE();
+CURRENT_ROLE()
+test_role
+#
+# Create the VIEW and prepared Statement with a CURRENT_ROLE() set.
+#
+CREATE
+DEFINER = no_role at localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test_with_current_role
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+PREPARE prepared_with_current_role FROM "SELECT * from view_role_test WHERE role_name = CURRENT_ROLE()";
+SHOW CREATE VIEW v_view_role_test_with_current_role;
+View	Create View	character_set_client	collation_connection
+v_view_role_test_with_current_role	CREATE ALGORITHM=UNDEFINED DEFINER=`no_role`@`localhost` SQL SECURITY INVOKER VIEW `v_view_role_test_with_current_role` AS select `view_role_test`.`id` AS `id`,`view_role_test`.`role_name` AS `role_name` from `view_role_test` where (`view_role_test`.`role_name` = current_role())	latin1	latin1_swedish_ci
+#
+# Values should be returned for all select statements as we do have
+# a CURRENT_ROLE() active;
+#
+EXECUTE prepared_no_current_role;
+id	role_name
+1	test_role
+EXECUTE prepared_with_current_role;
+id	role_name
+1	test_role
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+id	role_name
+1	test_role
+SELECT * FROM v_view_role_test_no_current_role;
+id	role_name
+1	test_role
+SELECT * FROM v_view_role_test_with_current_role;
+id	role_name
+1	test_role
+SET ROLE NONE;
+#
+# No values should be returned for all select statements as we do not have
+# a CURRENT_ROLE() active;
+#
+EXECUTE prepared_no_current_role;
+id	role_name
+EXECUTE prepared_with_current_role;
+id	role_name
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+id	role_name
+SELECT * FROM v_view_role_test_no_current_role;
+id	role_name
+SELECT * FROM v_view_role_test_with_current_role;
+id	role_name
+DROP USER has_role@'localhost';
+DROP USER no_role@'localhost';
+DROP ROLE test_role;
+DROP table view_role_test;
+DROP VIEW v_view_role_test_no_current_role;
+DROP VIEW v_view_role_test_with_current_role;
+DROP PREPARE prepared_no_current_role;
+DROP PREPARE prepared_with_current_role;
diff --git a/mysql-test/suite/roles/current_role_view-12666.test b/mysql-test/suite/roles/current_role_view-12666.test
new file mode 100644
index 00000000000..32039ffef07
--- /dev/null
+++ b/mysql-test/suite/roles/current_role_view-12666.test
@@ -0,0 +1,102 @@
+#
+# MDEV-12666 CURRENT_ROLE() does not work in a view
+#
+--source include/not_embedded.inc
+
+CREATE USER has_role@'localhost';
+GRANT ALL PRIVILEGES ON *.* TO has_role@'localhost';
+
+CREATE ROLE test_role;
+GRANT test_role TO has_role@'localhost';
+
+CREATE USER no_role@'localhost';
+GRANT ALL PRIVILEGES ON *.* TO no_role@'localhost';
+
+CREATE TABLE view_role_test (
+        id int primary key,
+        role_name varchar(50)
+        );
+
+INSERT INTO view_role_test VALUES (1, 'test_role');
+
+--echo #
+--echo # Use the same logic for stored procedures.
+--echo #
+PREPARE prepared_no_current_role FROM "SELECT * from view_role_test WHERE role_name = CURRENT_ROLE()";
+
+--echo #
+--echo # Creating a view with no CURRENT_ROLE() set and one with CURRENT_ROLE()
+--echo # set. Both should produce the same SHOW CREATE VIEW output.
+--echo #
+CREATE
+DEFINER = no_role at localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test_no_current_role
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+
+SHOW CREATE VIEW v_view_role_test_no_current_role;
+
+
+--echo #
+--echo # No values should be returned
+--echo #
+EXECUTE prepared_no_current_role;
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+SELECT * FROM v_view_role_test_no_current_role;
+
+--echo #
+--echo # Now let's set the role. Create identical views as before. See if
+--echo # their behaviour is different. It should not be.
+--echo #
+SET ROLE test_role;
+
+SELECT CURRENT_USER();
+SELECT CURRENT_ROLE();
+
+--echo #
+--echo # Create the VIEW and prepared Statement with a CURRENT_ROLE() set.
+--echo #
+CREATE
+DEFINER = no_role at localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test_with_current_role
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+
+PREPARE prepared_with_current_role FROM "SELECT * from view_role_test WHERE role_name = CURRENT_ROLE()";
+
+SHOW CREATE VIEW v_view_role_test_with_current_role;
+
+
+--echo #
+--echo # Values should be returned for all select statements as we do have
+--echo # a CURRENT_ROLE() active;
+--echo #
+EXECUTE prepared_no_current_role;
+EXECUTE prepared_with_current_role;
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+SELECT * FROM v_view_role_test_no_current_role;
+SELECT * FROM v_view_role_test_with_current_role;
+
+SET ROLE NONE;
+--echo #
+--echo # No values should be returned for all select statements as we do not have
+--echo # a CURRENT_ROLE() active;
+--echo #
+EXECUTE prepared_no_current_role;
+EXECUTE prepared_with_current_role;
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+SELECT * FROM v_view_role_test_no_current_role;
+SELECT * FROM v_view_role_test_with_current_role;
+
+
+DROP USER has_role@'localhost';
+DROP USER no_role@'localhost';
+DROP ROLE test_role;
+
+DROP table view_role_test;
+DROP VIEW v_view_role_test_no_current_role;
+DROP VIEW v_view_role_test_with_current_role;
+DROP PREPARE prepared_no_current_role;
+DROP PREPARE prepared_with_current_role;
diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test
index 89a8e0c9ffc..db48e1a7661 100644
--- a/mysql-test/t/view.test
+++ b/mysql-test/t/view.test
@@ -5814,6 +5814,55 @@ SELECT * FROM v1 where use_case_id = 10;
 drop view v1;
 drop table t1;
 
+--echo #
+--echo # MDEV-12666: CURRENT_ROLE() and DATABASE() does not work in a view
+--echo #
+--echo # DATABASE() fails only when the initial view creation features a NULL
+--echo # default database.
+--echo #
+--echo # CREATE, USE and DROP database so that we have no "default" database.
+--echo #
+CREATE DATABASE temporary;
+USE temporary;
+DROP DATABASE temporary;
+SELECT DATABASE();
+
+CREATE VIEW test.v_no_db AS SELECT DATABASE() = 'temporary_two';
+SHOW CREATE VIEW test.v_no_db;
+PREPARE prepared_no_database FROM "SELECT DATABASE() = 'temporary_two'";
+
+--echo #
+--echo # All statements should return NULL
+--echo #
+EXECUTE prepared_no_database;
+SELECT DATABASE() = 'temporary_two';
+SELECT * FROM test.v_no_db;
+
+CREATE DATABASE temporary_two;
+USE temporary_two;
+CREATE VIEW test.v_with_db AS SELECT DATABASE() = 'temporary_two';
+PREPARE prepared_with_database FROM "SELECT DATABASE() = 'temporary_two'";
+
+--echo #
+--echo # All statements should return 1;
+--echo #
+SELECT DATABASE() = 'temporary_two';
+SELECT * FROM test.v_no_db;
+SELECT * FROM test.v_with_db;
+EXECUTE prepared_with_database;
+
+--echo #
+--echo # Prepared statements maintain default database to be the same
+--echo # during on creation so this should return NULL still.
+--echo # See MySQL bug #25843
+--echo #
+EXECUTE prepared_no_database;
+
+DROP DATABASE temporary_two;
+DROP VIEW test.v_no_db;
+DROP VIEW test.v_with_db;
+USE test;
+
 --echo # -----------------------------------------------------------------
 --echo # -- End of 10.0 tests.
 --echo # -----------------------------------------------------------------
diff --git a/sql/item.cc b/sql/item.cc
index 4ce8396f71e..77b6ff5b82d 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1263,6 +1263,7 @@ Item *Item::const_charset_converter(CHARSET_INFO *tocs,
   DBUG_ASSERT(fixed);
   StringBuffer<64>tmp;
   String *s= val_str(&tmp);
+
   if (!s)
     return new Item_null((char *) func_name, tocs);
 
diff --git a/sql/item.h b/sql/item.h
index 4d33a0eb6c1..28a6649b9b1 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -1476,7 +1476,7 @@ class Item {
   virtual Item *expr_cache_insert_transformer(uchar *thd_arg) { return this; }
   virtual bool expr_cache_is_needed(THD *) { return FALSE; }
   virtual Item *safe_charset_converter(CHARSET_INFO *tocs);
-  bool needs_charset_converter(uint32 length, CHARSET_INFO *tocs)
+  bool needs_charset_converter(uint32 length, CHARSET_INFO *tocs) const
   {
     /*
       This will return "true" if conversion happens:
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 7cd712cc5e1..9ef9b136c24 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -2344,6 +2344,7 @@ String *Item_func_database::val_str(String *str)
   }
   else
     str->copy(thd->db, thd->db_length, system_charset_info);
+  null_value= 0;
   return str;
 }
 
@@ -2378,6 +2379,30 @@ bool Item_func_user::init(const char *user, const char *host)
 }
 
 
+Item *Item_func_sysconst::safe_charset_converter(CHARSET_INFO *tocs)
+{
+  /*
+   During view or prepared statement creation, the item should not
+   make use of const_charset_converter as it would imply substitution
+   with constant items which is not correct. Functions can have different
+   values during view creation and view execution based on context.
+
+   Return the identical item during view creation and prepare.
+  */
+  if (current_thd->lex->context_analysis_only &
+      (CONTEXT_ANALYSIS_ONLY_PREPARE | CONTEXT_ANALYSIS_ONLY_VIEW))
+    return this;
+  return const_charset_converter(tocs, true, fully_qualified_func_name());
+}
+
+bool Item_func_sysconst::const_item() const
+{
+  if (current_thd->lex->context_analysis_only &
+      (CONTEXT_ANALYSIS_ONLY_PREPARE | CONTEXT_ANALYSIS_ONLY_VIEW))
+    return false;
+  return true;
+}
+
 bool Item_func_user::fix_fields(THD *thd, Item **ref)
 {
   return (Item_func_sysconst::fix_fields(thd, ref) ||
@@ -2401,20 +2426,27 @@ bool Item_func_current_role::fix_fields(THD *thd, Item **ref)
   if (Item_func_sysconst::fix_fields(thd, ref))
     return 1;
 
+  maybe_null= 1;
+  return 0;
+}
+
+String* Item_func_current_role::val_str(String * str)
+{
+  DBUG_ASSERT(fixed == 1);
+
+  THD *thd = current_thd;
   Security_context *ctx= context->security_ctx
                           ? context->security_ctx : thd->security_ctx;
-
   if (ctx->priv_role[0])
   {
-    if (str_value.copy(ctx->priv_role, strlen(ctx->priv_role),
-                       system_charset_info))
-      return 1;
-
-    str_value.mark_as_const();
-    return 0;
+    str_value.copy(ctx->priv_role, strlen(ctx->priv_role),
+                   system_charset_info);
+    null_value= 0;
+    return &str_value;
   }
-  null_value= maybe_null= 1;
-  return 0;
+  null_value= 1;
+  return NULL;
+
 }
 
 
diff --git a/sql/item_strfunc.h b/sql/item_strfunc.h
index 2886cb68f9b..d9b6a493d34 100644
--- a/sql/item_strfunc.h
+++ b/sql/item_strfunc.h
@@ -542,10 +542,7 @@ class Item_func_sysconst :public Item_str_func
 public:
   Item_func_sysconst()
   { collation.set(system_charset_info,DERIVATION_SYSCONST); }
-  Item *safe_charset_converter(CHARSET_INFO *tocs)
-  {
-    return const_charset_converter(tocs, true, fully_qualified_func_name());
-  }
+  Item *safe_charset_converter(CHARSET_INFO *tocs);
   /*
     Used to create correct Item name in new converted item in
     safe_charset_converter, return string representation of this function
@@ -557,6 +554,7 @@ class Item_func_sysconst :public Item_str_func
     return trace_unsupported_by_check_vcol_func_processor(
                                            fully_qualified_func_name());
   }
+  bool const_item() const;
 };
 
 
@@ -632,11 +630,7 @@ class Item_func_current_role :public Item_func_sysconst
   { return save_str_value_in_field(field, &str_value); }
   const char *func_name() const { return "current_role"; }
   const char *fully_qualified_func_name() const { return "current_role()"; }
-  String *val_str(String *)
-  {
-    DBUG_ASSERT(fixed == 1);
-    return (null_value ? 0 : &str_value);
-  }
+  String *val_str(String *);
 };
 
 


More information about the commits mailing list