maria-developers team mailing list archive

Thread
Date

Re: [Commits] 55a4858fe6f: MDEV-12666: CURRENT_ROLE() does not work in a view

To: maria-developers@xxxxxxxxxxxxxxxxxxx, commits@xxxxxxxxxxx, Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>
From: Alexander Barkov <bar@xxxxxxxxxxx>
Date: Tue, 23 May 2017 12:13:52 +0400
In-reply-to: <20170522141511.501871321394@vicentiu-desktop>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

Hi Vicentiu,

It seems that Item_func_sysconst abuses const_charset_converter().

I have the following proposals:

- Instead of adding a new code into Item::const_charset_converter(),
can you please override Item_func_sysconst::safe_charset_converter()?

The direction proposed by Sergei's looks good:

   Item *safe_charset_converter(THD *thd, CHARSET_INFO *tocs)
   {
+    if (thd->lex->context_analysis_only & CONTEXT_ANALYSIS_ONLY_VIEW)
+      return Item_str_func::safe_charset_converter(thd, tocs);

return const_charset_converter(thd, tocs, true,fully_qualified_func_name()

   }

But it probably needs to be extended to handle prepared statement.
At PREPARE-time it should not call const_charset_converter().
At EXECUTE-time it should be Ok to call const_charset_converter().


So can you please change Item_func_sysconst::const_item()
to report false when in views and during PREPARE-time?

And then change safe_charset_converter() as follows:

Item *safe_charset_converter(THD *thd, CHARSET_INFO *tocs)
{
  if (!const_item())
    return Item_str_func::safe_charset_converter(thd, tocs);

return const_charset_converter(thd, tocs, true,fully_qualified_func_name()

}

- Please add tests for DATABASE() and CURRENT_USER

- Please add tests for PS with changing context between PREPARE and EXECUTE.

Thanks.


On 05/22/2017 06:15 PM, vicentiu wrote:

revision-id: 55a4858fe6f3fdc63a072b93b9bc3d8e51dc5e94 (mariadb-10.0.30-76-g55a4858fe6f)
parent(s): 8d1827f5540a0def82b1e2cbc557245c3e5e14ca
author: Vicențiu Ciorbaru
committer: Vicențiu Ciorbaru
timestamp: 2017-05-22 17:06:01 +0300
message:

MDEV-12666: CURRENT_ROLE() 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 solution proposed is to not replace any function with an Item_null,
but instead use an Item_static_string_func.

---
  .../suite/roles/current_role_view-12666.result     | 48 ++++++++++++++++++++
  .../suite/roles/current_role_view-12666.test       | 52 ++++++++++++++++++++++
  sql/item.cc                                        | 13 +++++-
  3 files changed, 112 insertions(+), 1 deletion(-)

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..525748b4875
--- /dev/null
+++ b/mysql-test/suite/roles/current_role_view-12666.result
@@ -0,0 +1,48 @@
+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');
+CREATE OR REPLACE
+DEFINER = no_role@localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+show create view v_view_role_test;
+View	Create View	character_set_client	collation_connection
+v_view_role_test	CREATE ALGORITHM=UNDEFINED DEFINER=`no_role`@`localhost` SQL SECURITY INVOKER VIEW `v_view_role_test` 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
+SET ROLE test_role;
+CREATE OR REPLACE
+DEFINER = no_role@localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test_2
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+show create view v_view_role_test_2;
+View	Create View	character_set_client	collation_connection
+v_view_role_test_2	CREATE ALGORITHM=UNDEFINED DEFINER=`no_role`@`localhost` SQL SECURITY INVOKER VIEW `v_view_role_test_2` 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
+SELECT CURRENT_USER();
+CURRENT_USER()
+root@localhost
+SELECT CURRENT_ROLE();
+CURRENT_ROLE()
+test_role
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+id	role_name
+1	test_role
+SELECT * FROM v_view_role_test;
+id	role_name
+1	test_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;
+drop view v_view_role_test_2;
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..86c74260b6e
--- /dev/null
+++ b/mysql-test/suite/roles/current_role_view-12666.test
@@ -0,0 +1,52 @@
+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');
+
+
+CREATE OR REPLACE
+DEFINER = no_role@localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+
+show create view v_view_role_test;
+
+SET ROLE test_role;
+
+CREATE OR REPLACE
+DEFINER = no_role@localhost
+SQL SECURITY INVOKER
+VIEW v_view_role_test_2
+AS
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+
+show create view v_view_role_test_2;
+
+
+SELECT CURRENT_USER();
+SELECT CURRENT_ROLE();
+
+SELECT * FROM view_role_test WHERE role_name = CURRENT_ROLE();
+SELECT * FROM v_view_role_test;
+
+
+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;
+drop view v_view_role_test_2;
diff --git a/sql/item.cc b/sql/item.cc
index 4ce8396f71e..2ee3a6d61dc 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1263,9 +1263,20 @@ Item *Item::const_charset_converter(CHARSET_INFO *tocs,
    DBUG_ASSERT(fixed);
    StringBuffer<64>tmp;
    String *s= val_str(&tmp);
-  if (!s)
+
+  /* Functions that return NULL during prepare must not be replaced with
+     Item_null as they may return non-null values during execution.
+     Ex: CURRENT_ROLE() during view creation time can be NULL if no role
+     is set. */
+  if (!s && !func_name)
      return new Item_null((char *) func_name, tocs);

+  if (!s)
+  {
+    tmp.set_ascii("", 0);
+    s = &tmp;
+  }
+
    if (!needs_charset_converter(s->length(), tocs))
    {
      if (collation.collation == &my_charset_bin && tocs != &my_charset_bin &&
_______________________________________________
commits mailing list
commits@xxxxxxxxxxx
https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits