From a086682ecb0ba41e672b6f57497e1f0991f13e9e Mon Sep 17 00:00:00 2001 From: Wojciech Mitros Date: Mon, 20 Mar 2023 14:45:37 +0100 Subject: [PATCH 1/4] cql-pytest: add an optional name parameter to new_type() Currently, when creating a UDT, we're always generating a new name for it. This patch enables setting the name to a specific string instead. --- test/cql-pytest/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cql-pytest/util.py b/test/cql-pytest/util.py index dc3a8e52c6..7e73bfe2df 100644 --- a/test/cql-pytest/util.py +++ b/test/cql-pytest/util.py @@ -91,8 +91,8 @@ def new_test_table(cql, keyspace, schema, extra=""): # A utility function for creating a new temporary user-defined type. @contextmanager -def new_type(cql, keyspace, cmd): - type_name = keyspace + "." + unique_name() +def new_type(cql, keyspace, cmd, name=None): + type_name = keyspace + "." + (name or unique_name()) cql.execute("CREATE TYPE " + type_name + " " + cmd) try: yield type_name From fc8dcc1a623e967555dbad8c91b7099d920b2f6c Mon Sep 17 00:00:00 2001 From: Wojciech Mitros Date: Wed, 22 Mar 2023 11:11:35 +0100 Subject: [PATCH 2/4] cql: add a check for currently used stack in parser While in debug mode, we may switch the default stack to a larger one when parsing cql. We may, however, invoke the parser recusively, causing us to switch to the big stack while currently using it. After the reset, we assume that the stack is empty, so after switching to the same stack, we write over its previous contents. This is fixed by checking if we're already using the large stack, which is achieved by comparing the address of a local variable to the start and end of the large stack. --- cql3/util.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cql3/util.cc b/cql3/util.cc index 65a6f309eb..b338297dd4 100644 --- a/cql3/util.cc +++ b/cql3/util.cc @@ -88,6 +88,11 @@ void do_with_parser_impl(const sstring_view& cql, noncopyable_function Date: Mon, 20 Mar 2023 18:40:47 +0100 Subject: [PATCH 3/4] cql: maybe quote user type name in ut_name::to_string() Currently, the ut_name::to_string() is used only in 2 cases: the first one is in logs or as part of error messages, and the second one is during parsing, temporarily storing the user defined type name in the auth::resource for later preparation with database and data_dictionary context. This patch changes the string so that the 'name' part of the ut_name (as opposed to the 'keyspace' part) is now quoted when needed. This does not worsen the logging set of cases, but it does help with parsing of the resulting string, when finishing preparing the auth::resource. After the modification, a more fitting name for the function is "ut_name::to_cql_string()", so the function is renamed to that. --- cql3/cql3_type.cc | 4 ++-- cql3/statements/alter_type_statement.cc | 12 ++++++------ cql3/statements/create_type_statement.cc | 2 +- cql3/statements/drop_type_statement.cc | 2 +- cql3/ut_name.cc | 4 ++-- cql3/ut_name.hh | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cql3/cql3_type.cc b/cql3/cql3_type.cc index 69a16dcd5e..37a32fba26 100644 --- a/cql3/cql3_type.cc +++ b/cql3/cql3_type.cc @@ -205,10 +205,10 @@ class cql3_type::raw_ut : public raw { virtual sstring to_string() const override { if (is_frozen()) { - return format("frozen<{}>", _name.to_string()); + return format("frozen<{}>", _name.to_cql_string()); } - return _name.to_string(); + return _name.to_cql_string(); } public: raw_ut(ut_name name) diff --git a/cql3/statements/alter_type_statement.cc b/cql3/statements/alter_type_statement.cc index 7d72ae5837..f9af0166e2 100644 --- a/cql3/statements/alter_type_statement.cc +++ b/cql3/statements/alter_type_statement.cc @@ -60,14 +60,14 @@ future> alter_type_statement::prepare_announcement_mutatio auto to_update = all_types.find(_name.get_user_type_name()); // Shouldn't happen, unless we race with a drop if (to_update == all_types.end()) { - throw exceptions::invalid_request_exception(format("No user type named {} exists.", _name.to_string())); + throw exceptions::invalid_request_exception(format("No user type named {} exists.", _name.to_cql_string())); } for (auto&& schema : ks.metadata()->cf_meta_data() | boost::adaptors::map_values) { for (auto&& column : schema->partition_key_columns()) { if (column.type->references_user_type(_name.get_keyspace(), _name.get_user_type_name())) { throw exceptions::invalid_request_exception(format("Cannot add new field to type {} because it is used in the partition key column {} of table {}.{}", - _name.to_string(), column.name_as_text(), schema->ks_name(), schema->cf_name())); + _name.to_cql_string(), column.name_as_text(), schema->ks_name(), schema->cf_name())); } } } @@ -134,7 +134,7 @@ user_type alter_type_statement::add_or_alter::do_add(data_dictionary::database d { if (to_update->idx_of_field(_field_name->name())) { throw exceptions::invalid_request_exception(format("Cannot add new field {} to type {}: a field of the same name already exists", - _field_name->to_string(), _name.to_string())); + _field_name->to_string(), _name.to_cql_string())); } if (to_update->size() == max_udt_fields) { @@ -147,7 +147,7 @@ user_type alter_type_statement::add_or_alter::do_add(data_dictionary::database d auto&& add_type = _field_type->prepare(db, keyspace()).get_type(); if (add_type->references_user_type(to_update->_keyspace, to_update->_name)) { throw exceptions::invalid_request_exception(format("Cannot add new field {} of type {} to type {} as this would create a circular reference", - *_field_name, *_field_type, _name.to_string())); + *_field_name, *_field_type, _name.to_cql_string())); } new_types.push_back(std::move(add_type)); return user_type_impl::get_instance(to_update->_keyspace, to_update->_name, std::move(new_names), std::move(new_types), to_update->is_multi_cell()); @@ -157,7 +157,7 @@ user_type alter_type_statement::add_or_alter::do_alter(data_dictionary::database { auto idx = to_update->idx_of_field(_field_name->name()); if (!idx) { - throw exceptions::invalid_request_exception(format("Unknown field {} in type {}", _field_name->to_string(), _name.to_string())); + throw exceptions::invalid_request_exception(format("Unknown field {} in type {}", _field_name->to_string(), _name.to_cql_string())); } auto previous = to_update->field_types()[*idx]; @@ -194,7 +194,7 @@ user_type alter_type_statement::renames::make_updated_type(data_dictionary::data auto&& from = rename.first; auto idx = to_update->idx_of_field(from->name()); if (!idx) { - throw exceptions::invalid_request_exception(format("Unknown field {} in type {}", from->to_string(), _name.to_string())); + throw exceptions::invalid_request_exception(format("Unknown field {} in type {}", from->to_string(), _name.to_cql_string())); } new_names[*idx] = rename.second->name(); } diff --git a/cql3/statements/create_type_statement.cc b/cql3/statements/create_type_statement.cc index bf18b844a7..e92e0f7459 100644 --- a/cql3/statements/create_type_statement.cc +++ b/cql3/statements/create_type_statement.cc @@ -134,7 +134,7 @@ future, std::vector< _name.get_string_type_name()); } else { if (!_if_not_exists) { - co_await coroutine::return_exception(exceptions::invalid_request_exception(format("A user type of name {} already exists", _name.to_string()))); + co_await coroutine::return_exception(exceptions::invalid_request_exception(format("A user type of name {} already exists", _name.to_cql_string()))); } } } catch (data_dictionary::no_such_keyspace& e) { diff --git a/cql3/statements/drop_type_statement.cc b/cql3/statements/drop_type_statement.cc index fa53c0412c..a76347a8c9 100644 --- a/cql3/statements/drop_type_statement.cc +++ b/cql3/statements/drop_type_statement.cc @@ -56,7 +56,7 @@ void drop_type_statement::validate_while_executing(query_processor& qp) const { if (_if_exists) { return; } else { - throw exceptions::invalid_request_exception(format("No user type named {} exists.", _name.to_string())); + throw exceptions::invalid_request_exception(format("No user type named {} exists.", _name.to_cql_string())); } } diff --git a/cql3/ut_name.cc b/cql3/ut_name.cc index d933169a6f..abc51706ef 100644 --- a/cql3/ut_name.cc +++ b/cql3/ut_name.cc @@ -39,8 +39,8 @@ sstring ut_name::get_string_type_name() const return _ut_name->to_string(); } -sstring ut_name::to_string() const { - return (has_keyspace() ? (_ks_name.value() + ".") : "") + _ut_name->to_string(); +sstring ut_name::to_cql_string() const { + return (has_keyspace() ? (_ks_name.value() + ".") : "") + _ut_name->to_cql_string(); } } diff --git a/cql3/ut_name.hh b/cql3/ut_name.hh index e14b2727da..f73f612104 100644 --- a/cql3/ut_name.hh +++ b/cql3/ut_name.hh @@ -37,10 +37,10 @@ public: sstring get_string_type_name() const; - sstring to_string() const; + sstring to_cql_string() const; friend std::ostream& operator<<(std::ostream& os, const ut_name& n) { - return os << n.to_string(); + return os << n.to_cql_string(); } }; From b03fce524b74d2c3d165869a052fbd4822bd3af7 Mon Sep 17 00:00:00 2001 From: Wojciech Mitros Date: Mon, 20 Mar 2023 14:54:35 +0100 Subject: [PATCH 4/4] cql-pytest: test permissions for UDTs with quoted names Currently, we only tested whether permissions with UDFs that have quoted names work correctly. This patch adds the missing test that confirms that we can also use UDTs (as UDF parameter types) when altering permissions. --- test/cql-pytest/test_permissions.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/cql-pytest/test_permissions.py b/test/cql-pytest/test_permissions.py index fb6bb2b501..83aca2033c 100644 --- a/test/cql-pytest/test_permissions.py +++ b/test/cql-pytest/test_permissions.py @@ -164,27 +164,33 @@ def test_udf_permissions_serialization(cql): # are properly handled, with right permissions granted. # Cassandra doesn't quote names properly, so the test fails def test_udf_permissions_quoted_names(cassandra_bug, cql): - schema = "a int primary key" + udt_name = f'"{unique_name()}weird_udt[t^t]a^b^[]"' + schema = f"a frozen<{udt_name}> primary key" with new_test_keyspace(cql, "WITH REPLICATION = { 'class': 'SimpleStrategy', 'replication_factor': 1 }") as keyspace: - with new_test_table(cql, keyspace, schema) as table: - fun_body_lua = "(i int) CALLED ON NULL INPUT RETURNS bigint LANGUAGE lua AS 'return 42;'" - fun_body_java = "(i int) CALLED ON NULL INPUT RETURNS bigint LANGUAGE java AS 'return 42;'" + with new_type(cql, keyspace, "(a text, b int)", udt_name) as udt, new_test_table(cql, keyspace, schema) as table: + fun_body_lua = f"(i {udt}) CALLED ON NULL INPUT RETURNS bigint LANGUAGE lua AS 'return 42;'" + fun_body_java = f"(i {udt}) CALLED ON NULL INPUT RETURNS bigint LANGUAGE java AS 'return 42;'" fun_body = fun_body_lua try: with new_function(cql, keyspace, fun_body): pass except: fun_body = fun_body_java - with new_function(cql, keyspace, fun_body, '"weird[name]"') as weird_fun: + with new_function(cql, keyspace, fun_body, f'"{unique_name()}weird[name1^name2]x^y"') as weird_fun: with new_user(cql) as username: with new_session(cql, username) as user_session: - grant(cql, 'EXECUTE', f'FUNCTION {keyspace}.{weird_fun}(int)', username) + grant(cql, 'EXECUTE', f'FUNCTION {keyspace}.{weird_fun}({udt})', username) grant(cql, 'SELECT', table, username) - cql.execute(f"INSERT INTO {table}(a) VALUES (7)") + cql.execute(f"INSERT INTO {table}(a) VALUES ({{a:'hello', b:42}})") + assert list([r[0] for r in user_session.execute(f"SELECT {keyspace}.{weird_fun}(a) FROM {table}")]) == [42] resources_with_execute = [row.resource for row in cql.execute(f"LIST EXECUTE OF {username}")] - assert f'' in resources_with_execute + assert f')>' in resources_with_execute + + revoke(cql, 'EXECUTE', f'FUNCTION {keyspace}.{weird_fun}({udt})', username) + check_enforced(cql, username, 'EXECUTE', f'FUNCTION {keyspace}.{weird_fun}({udt})', + lambda: user_session.execute(f"SELECT {keyspace}.{weird_fun}(a) FROM {table}")) # Test that dropping a function without specifying its signature works with the DROP permission if there's only # one function with the given name, and that it fails if there are multiple functions with the same name,