diff --git a/cql3/functions/functions.cc b/cql3/functions/functions.cc index d0d71b730e..ebfa4a5111 100644 --- a/cql3/functions/functions.cc +++ b/cql3/functions/functions.cc @@ -181,13 +181,18 @@ inline shared_ptr make_from_json_function(database& db, const sstring& keyspace, data_type t) { return make_native_scalar_function("fromjson", t, {utf8_type}, - [&db, &keyspace, t](cql_serialization_format sf, const std::vector& parameters) -> bytes_opt { - rjson::value json_value = rjson::parse(utf8_type->to_string(parameters[0].value())); - bytes_opt parsed_json_value; - if (!json_value.IsNull()) { - parsed_json_value.emplace(from_json_object(*t, json_value, sf)); + [&db, keyspace, t](cql_serialization_format sf, const std::vector& parameters) -> bytes_opt { + try { + rjson::value json_value = rjson::parse(utf8_type->to_string(parameters[0].value())); + bytes_opt parsed_json_value; + if (!json_value.IsNull()) { + parsed_json_value.emplace(from_json_object(*t, json_value, sf)); + } + return parsed_json_value; + } catch(rjson::error& e) { + throw exceptions::function_execution_exception("fromJson", + format("Failed parsing fromJson parameter: {}", e.what()), keyspace, {t->name()}); } - return parsed_json_value; }); } diff --git a/cql3/functions/native_scalar_function.hh b/cql3/functions/native_scalar_function.hh index b1205f5641..907efd614f 100644 --- a/cql3/functions/native_scalar_function.hh +++ b/cql3/functions/native_scalar_function.hh @@ -78,7 +78,22 @@ public: return Pure; } virtual bytes_opt execute(cql_serialization_format sf, const std::vector& parameters) override { - return _func(sf, parameters); + try { + return _func(sf, parameters); + } catch(exceptions::cassandra_exception&) { + // If the function's code took the time to produce an official + // cassandra_exception, pass it through. Otherwise, below we will + // wrap the unknown exception in a function_execution_exception. + throw; + } catch(...) { + std::vector args; + args.reserve(arg_types().size()); + for (const data_type& a : arg_types()) { + args.push_back(a->name()); + } + throw exceptions::function_execution_exception(name().name, + format("Failed execution of function {}: {}", name(), std::current_exception()), name().keyspace, std::move(args)); + } } }; diff --git a/exceptions/exceptions.hh b/exceptions/exceptions.hh index 7a6e120765..58ecd5ca00 100644 --- a/exceptions/exceptions.hh +++ b/exceptions/exceptions.hh @@ -340,4 +340,18 @@ public: unsupported_operation_exception(const sstring& msg) : std::runtime_error("unsupported operation: " + msg) {} }; +class function_execution_exception : public cassandra_exception { +public: + const sstring ks_name; + const sstring func_name; + const std::vector args; + function_execution_exception(sstring func_name_, sstring detail, sstring ks_name_, std::vector args_) noexcept + : cassandra_exception{exception_code::FUNCTION_FAILURE, + format("execution of {} failed: {}", func_name_, detail)} + , ks_name(std::move(ks_name_)) + , func_name(std::move(func_name_)) + , args(std::move(args_)) + { } +}; + } diff --git a/test/cql-pytest/test_json.py b/test/cql-pytest/test_json.py index 5fa5317279..c0c2b29a6d 100644 --- a/test/cql-pytest/test_json.py +++ b/test/cql-pytest/test_json.py @@ -26,7 +26,7 @@ from util import unique_name, new_test_table -from cassandra.protocol import FunctionFailure +from cassandra.protocol import FunctionFailure, InvalidRequest import pytest import random @@ -34,58 +34,62 @@ import random @pytest.fixture(scope="session") def table1(cql, test_keyspace): table = test_keyspace + "." + unique_name() - cql.execute(f"CREATE TABLE {table} (p int PRIMARY KEY, v int, a ascii)") + cql.execute(f"CREATE TABLE {table} (p int PRIMARY KEY, v int, a ascii, b boolean)") yield table cql.execute("DROP TABLE " + table) # Test that failed fromJson() parsing an invalid JSON results in the expected # error - FunctionFailure - and not some weird internal error. # Reproduces issue #7911. -@pytest.mark.xfail(reason="issue #7911") def test_failed_json_parsing_unprepared(cql, table1): p = random.randint(1,1000000000) with pytest.raises(FunctionFailure): cql.execute(f"INSERT INTO {table1} (p, v) VALUES ({p}, fromJson('dog'))") -@pytest.mark.xfail(reason="issue #7911") def test_failed_json_parsing_prepared(cql, table1): p = random.randint(1,1000000000) stmt = cql.prepare(f"INSERT INTO {table1} (p, v) VALUES (?, fromJson(?))") with pytest.raises(FunctionFailure): - cql.execute(stmt, [0, 'dog']) + cql.execute(stmt, [p, 'dog']) # Similarly, if the JSON parsing did not fail, but yielded a type which is # incompatible with the type we want it to yield, we should get a clean # FunctionFailure, not some internal server error. # We have here examples of returning a string where a number was expected, -# and returning a unicode string where ASCII was expected. +# and returning a unicode string where ASCII was expected, and returning +# a number of the wrong type # Reproduces issue #7911. -@pytest.mark.xfail(reason="issue #7911") def test_fromjson_wrong_type_unprepared(cql, table1): p = random.randint(1,1000000000) with pytest.raises(FunctionFailure): cql.execute(f"INSERT INTO {table1} (p, v) VALUES ({p}, fromJson('\"dog\"'))") with pytest.raises(FunctionFailure): cql.execute(f"INSERT INTO {table1} (p, a) VALUES ({p}, fromJson('3'))") -@pytest.mark.xfail(reason="issue #7911") def test_fromjson_wrong_type_prepared(cql, table1): p = random.randint(1,1000000000) stmt = cql.prepare(f"INSERT INTO {table1} (p, v) VALUES (?, fromJson(?))") with pytest.raises(FunctionFailure): - cql.execute(stmt, [0, '"dog"']) + cql.execute(stmt, [p, '"dog"']) stmt = cql.prepare(f"INSERT INTO {table1} (p, a) VALUES (?, fromJson(?))") with pytest.raises(FunctionFailure): - cql.execute(stmt, [0, '3']) -@pytest.mark.xfail(reason="issue #7911") + cql.execute(stmt, [p, '3']) def test_fromjson_bad_ascii_unprepared(cql, table1): p = random.randint(1,1000000000) with pytest.raises(FunctionFailure): cql.execute(f"INSERT INTO {table1} (p, a) VALUES ({p}, fromJson('\"שלום\"'))") -@pytest.mark.xfail(reason="issue #7911") def test_fromjson_bad_ascii_prepared(cql, table1): p = random.randint(1,1000000000) stmt = cql.prepare(f"INSERT INTO {table1} (p, a) VALUES (?, fromJson(?))") with pytest.raises(FunctionFailure): - cql.execute(stmt, [0, '"שלום"']) + cql.execute(stmt, [p, '"שלום"']) +def test_fromjson_nonint_unprepared(cql, table1): + p = random.randint(1,1000000000) + with pytest.raises(FunctionFailure): + cql.execute(f"INSERT INTO {table1} (p, v) VALUES ({p}, fromJson('1.2'))") +def test_fromjson_nonint_prepared(cql, table1): + p = random.randint(1,1000000000) + stmt = cql.prepare(f"INSERT INTO {table1} (p, v) VALUES (?, fromJson(?))") + with pytest.raises(FunctionFailure): + cql.execute(stmt, [p, '1.2']) # The JSON standard does not define or limit the range or precision of # numbers. However, if a number is assigned to a Scylla number type, the @@ -105,7 +109,27 @@ def test_fromjson_int_overflow_prepared(cql, table1): p = random.randint(1,1000000000) stmt = cql.prepare(f"INSERT INTO {table1} (p, v) VALUES (?, fromJson(?))") with pytest.raises(FunctionFailure): - cql.execute(stmt, [0, '2147483648']) + cql.execute(stmt, [p, '2147483648']) + +# Cassandra allows the strings "true" and "false", not just the JSON constants +# true and false, to be assigned to a boolean column. However, very strangely, +# it only allows this for prepared statements, and *not* for unprepared +# statements - which result in an InvalidRequest! +# Reproduces #7915. +def test_fromjson_boolean_string_unprepared(cql, table1): + p = random.randint(1,1000000000) + with pytest.raises(InvalidRequest): + cql.execute(f"INSERT INTO {table1} (p, b) VALUES ({p}, '\"true\"')") + with pytest.raises(InvalidRequest): + cql.execute(f"INSERT INTO {table1} (p, b) VALUES ({p}, '\"false\"')") +@pytest.mark.xfail(reason="issue #7915") +def test_fromjson_boolean_string_prepared(cql, table1): + p = random.randint(1,1000000000) + stmt = cql.prepare(f"INSERT INTO {table1} (p, b) VALUES (?, fromJson(?))") + cql.execute(stmt, [p, '"true"']) + assert list(cql.execute(f"SELECT p, b from {table1} where p = {p}")) == [(p, True)] + cql.execute(stmt, [p, '"false"']) + assert list(cql.execute(f"SELECT p, b from {table1} where p = {p}")) == [(p, False)] # Test that null argument is allowed for fromJson(), with unprepared statement # Reproduces issue #7912. diff --git a/transport/server.cc b/transport/server.cc index 531261600e..ade14c02b9 100644 --- a/transport/server.cc +++ b/transport/server.cc @@ -578,7 +578,17 @@ future>> } catch (const exceptions::prepared_query_not_found_exception& ex) { try { ++_server._stats.errors[ex.code()]; } catch(...) {} return make_unprepared_error(stream, ex.code(), ex.what(), ex.id, trace_state); + } catch (const exceptions::function_execution_exception& ex) { + try { ++_server._stats.errors[ex.code()]; } catch(...) {} + return make_function_failure_error(stream, ex.code(), ex.what(), ex.ks_name, ex.func_name, ex.args, trace_state); } catch (const exceptions::cassandra_exception& ex) { + // Note: the CQL protocol specifies that many types of errors have + // mandatory parameters. These cassandra_exception subclasses MUST + // be handled above. This default "cassandra_exception" case is + // only appropriate for the specific types of errors which do not have + // additional information, such as invalid_request_exception. + // TODO: consider listing those types explicitly, instead of the + // catch-all type cassandra_exception. try { ++_server._stats.errors[ex.code()]; } catch(...) {} return make_error(stream, ex.code(), ex.what(), trace_state); } catch (std::exception& ex) { @@ -1339,6 +1349,17 @@ std::unique_ptr cql_server::connection::make_unprepared_er return response; } +std::unique_ptr cql_server::connection::make_function_failure_error(int16_t stream, exceptions::exception_code err, sstring msg, sstring ks_name, sstring func_name, std::vector args, const tracing::trace_state_ptr& tr_state) const +{ + auto response = std::make_unique(stream, cql_binary_opcode::ERROR, tr_state); + response->write_int(static_cast(err)); + response->write_string(msg); + response->write_string(ks_name); + response->write_string(func_name); + response->write_string_list(args); + return response; +} + std::unique_ptr cql_server::connection::make_error(int16_t stream, exceptions::exception_code err, sstring msg, const tracing::trace_state_ptr& tr_state) const { auto response = std::make_unique(stream, cql_binary_opcode::ERROR, tr_state); diff --git a/transport/server.hh b/transport/server.hh index 665e207cf5..1b3e25addd 100644 --- a/transport/server.hh +++ b/transport/server.hh @@ -235,6 +235,7 @@ private: std::unique_ptr make_mutation_write_failure_error(int16_t stream, exceptions::exception_code err, sstring msg, db::consistency_level cl, int32_t received, int32_t numfailures, int32_t blockfor, db::write_type type, const tracing::trace_state_ptr& tr_state) const; std::unique_ptr make_already_exists_error(int16_t stream, exceptions::exception_code err, sstring msg, sstring ks_name, sstring cf_name, const tracing::trace_state_ptr& tr_state) const; std::unique_ptr make_unprepared_error(int16_t stream, exceptions::exception_code err, sstring msg, bytes id, const tracing::trace_state_ptr& tr_state) const; + std::unique_ptr make_function_failure_error(int16_t stream, exceptions::exception_code err, sstring msg, sstring ks_name, sstring func_name, std::vector args, const tracing::trace_state_ptr& tr_state) const; std::unique_ptr make_error(int16_t stream, exceptions::exception_code err, sstring msg, const tracing::trace_state_ptr& tr_state) const; std::unique_ptr make_ready(int16_t stream, const tracing::trace_state_ptr& tr_state) const; std::unique_ptr make_supported(int16_t stream, const tracing::trace_state_ptr& tr_state) const;