mirrors/dragonfly
1
0
Fork 0
mirror of https://github.com/dragonflydb/dragonfly.git synced 2025-05-11 10:25:47 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

chore(eval): Don't crash on unsupported script commands (#2138)

Browse source 
* chore(eval): Don't crash on unsupported script commands

---------

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
This commit is contained in:
Vladislav 2023-11-08 11:21:52 +03:00 • committed by GitHub
parent 6e6aca0978
commit 0da488d99e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

18
src/server/main_service.cc
View file

@ -966,6 +966,7 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
bool is_write_cmd = cid->opt_mask() & CO::WRITE;
bool under_multi = dfly_cntx.conn_state.exec_info.IsCollecting() && !is_trans_cmd;
// Check if the command is allowed to execute under this global state
bool allowed_by_state = true;
switch (etl.gstate()) {
case GlobalState::LOADING:
@ -999,9 +1000,6 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
if (dfly_cntx.monitor && (cmd_name != "RESET" && cmd_name != "QUIT"))
return ErrorReply{"Replica can't interact with the keyspace"};
if (under_script && (cid->opt_mask() & CO::NOSCRIPT))
return ErrorReply{"This Redis command is not allowed from script"};
if (!etl.is_master && is_write_cmd && !dfly_cntx.is_replicating)
return ErrorReply{"-READONLY You can't write against a read only replica."};
@ -1018,6 +1016,19 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
return err;
}
if (under_script && (cid->opt_mask() & CO::NOSCRIPT))
return ErrorReply{"This Redis command is not allowed from script"};
if (under_script) {
DCHECK(dfly_cntx.transaction);
// The following commands access shards arbitrarily without having keys, so they can only be run
// non atomically or globally.
Transaction::MultiMode mode = dfly_cntx.transaction->GetMultiMode();
bool shard_access = (cid->opt_mask()) & (CO::GLOBAL_TRANS | CO::NO_KEY_TRANSACTIONAL);
if (shard_access && (mode != Transaction::GLOBAL && mode != Transaction::NON_ATOMIC))
return ErrorReply("This Redis command is not allowed from script");
}
if (under_script && cid->IsTransactional()) {
OpStatus status =
CheckKeysDeclared(*dfly_cntx.conn_state.script_info, cid, tail_args, dfly_cntx.transaction);
@ -1529,6 +1540,7 @@ void Service::CallFromScript(ConnectionContext* cntx, Interpreter::CallArgs& ca)
if (ca.async) {
auto& info = cntx->conn_state.script_info;
ToUpper(&ca.args[0]);
// Full command verification happens during squashed execution
if (auto* cid = registry_.Find(ArgS(ca.args, 0)); cid != nullptr) {
auto replies = ca.error_abort ? ReplyMode::ONLY_ERR : ReplyMode::NONE;

23
src/server/multi_test.cc
View file

@ -686,6 +686,29 @@ TEST_F(MultiTest, ScriptFlagsEmbedded) {
EXPECT_THAT(Run({"eval", s2, "0"}), ErrArg("Invalid flag: this-is-an-error"));
}
TEST_F(MultiTest, ScriptBadCommand) {
const char* s1 = "redis.call('FLUSHALL')";
const char* s2 = "redis.call('FLUSHALL'); redis.set(KEYS[1], ARGS[1]);";
const char* s3 = "redis.acall('FLUSHALL'); redis.set(KEYS[1], ARGS[1]);";
const char* s4 = R"(
#!lua flags=disable-atomicity
redis.call('FLUSHALL');
return "OK";
)";
auto resp = Run({"eval", s1, "0"}); // tx won't be scheduled at all
EXPECT_THAT(resp, ErrArg("This Redis command is not allowed from script"));
resp = Run({"eval", s2, "1", "works", "false"}); // will be scheduled as lock ahead
EXPECT_THAT(resp, ErrArg("This Redis command is not allowed from script"));
resp = Run({"eval", s3, "1", "works", "false"}); // also async call will happen
EXPECT_THAT(resp, ErrArg("This Redis command is not allowed from script"));
resp = Run({"eval", s4, "0"});
EXPECT_EQ(resp, "OK");
}
TEST_F(MultiTest, MultiEvalModeConflict) {
if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::GLOBAL) {
GTEST_SKIP() << "Skipped MultiEvalModeConflict test because multi_exec_mode is global";