bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Merge 'cql: enable setting permissions on resources with quoted UDT names' from Wojciech Mitros

Browse Source 
This series fixes an issue with altering permissions on UDFs with
parameter types that are UDTs with quoted names and adds
a test for it.

The issue was caused by the format of the temporary string
that represented the UDT in `auth::resource`. After parsing the
user input to a raw type, we created a string representing the
UDT using `ut_name::to_string()`. The segment of the resulting
string that represented the name of the UDT was not quoted,
making us unable to parse it again when the UDT was being
`prepare`d. Other than for this purpose, the `ut_name::to_string()`
is used only for logging, so the solution was modifying it to
maybe quote the UDT name.

Ref: https://github.com/scylladb/scylladb/pull/12869

Closes #13257

* github.com:scylladb/scylladb:
  cql-pytest: test permissions for UDTs with quoted names
  cql: maybe quote user type name in ut_name::to_string()
  cql: add a check for currently used stack in parser
  cql-pytest: add an optional name parameter to new_type()
This commit is contained in:
Nadav Har'El
2023-05-10 19:10:29 +03:00
parent 1f45c7364c b03fce524b
commit f1cad230bb
9 changed files with 35 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cql3/cql3_type.cc
View File

@@ -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)

12
cql3/statements/alter_type_statement.cc
View File

@@ -60,14 +60,14 @@ future<std::vector<mutation>> 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();
}

2
cql3/statements/create_type_statement.cc
View File

@@ -134,7 +134,7 @@ future<std::pair<::shared_ptr<cql_transport::event::schema_change>, 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) {

2
cql3/statements/drop_type_statement.cc
View File

@@ -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()));
}
}

4
cql3/ut_name.cc
View File

@@ -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();
}
}

4
cql3/ut_name.hh
View File

@@ -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();
}
};

5
cql3/util.cc
View File

@@ -88,6 +88,11 @@ void do_with_parser_impl(const sstring_view& cql, noncopyable_function<void (cql
ucontext_t uc;
auto r = getcontext(&uc);
assert(r == 0);
if (stack.get() <= (char*)&uc && (char*)&uc < stack.get() + stack_size) {
// We are already running on the large stack, so just call the
// parser directly.
return do_with_parser_impl_impl(cql, std::move(f));
}
uc.uc_stack.ss_sp = stack.get();
uc.uc_stack.ss_size = stack_size;
uc.uc_link = nullptr;

22
test/cql-pytest/test_permissions.py
View File

@@ -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'<function {keyspace}.{weird_fun}(int)>' in resources_with_execute
assert f'<function {keyspace}.{weird_fun}(frozen<{udt_name}>)>' 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,

4
test/cql-pytest/util.py
View File

@@ -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