diff --git a/src/config_type.h b/src/config_type.h index 892de3cb4ca..b5d3afaab50 100644 --- a/src/config_type.h +++ b/src/config_type.h @@ -153,7 +153,7 @@ class EnumField : public ConfigField { Status Set(const std::string &v) override { int e = configEnumGetValue(enums_, v.c_str()); if (e == INT_MIN) { - return Status(Status::NotOK, "invaild enum option"); + return Status(Status::NotOK, "invalid enum option"); } *receiver_ = e; return Status::OK(); diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc index 62d33b6afca..f666881d388 100644 --- a/src/redis_cmd.cc +++ b/src/redis_cmd.cc @@ -60,7 +60,7 @@ class CommandAuth : public Commander { } const auto requirepass = config->requirepass; if (!requirepass.empty() && user_password != requirepass) { - *output = Redis::Error("ERR invaild password"); + *output = Redis::Error("ERR invalid password"); return Status::OK(); } conn->SetNamespace(kDefaultNamespace); diff --git a/src/replication.cc b/src/replication.cc index e6fe74bd418..8ff557f1e51 100644 --- a/src/replication.cc +++ b/src/replication.cc @@ -168,7 +168,9 @@ ReplicationThread::CallbacksStateMachine::CallbacksStateMachine( ReplicationThread *repl, ReplicationThread::CallbacksStateMachine::CallbackList &&handlers) : repl_(repl), handlers_(std::move(handlers)) { - if (!repl_->auth_.empty()) { + // Note: It may cause data races to use 'masterauth' directly. + // It is acceptable because password change is a low frequency operation. + if (!repl_->srv_->GetConfig()->masterauth.empty()) { handlers_.emplace_front(CallbacksStateMachine::READ, "auth read", authReadCB); handlers_.emplace_front(CallbacksStateMachine::WRITE, "auth write", authWriteCB); } @@ -258,10 +260,9 @@ void ReplicationThread::CallbacksStateMachine::Stop() { } ReplicationThread::ReplicationThread(std::string host, uint32_t port, - Server *srv, std::string auth) + Server *srv) : host_(std::move(host)), port_(port), - auth_(std::move(auth)), srv_(srv), storage_(srv->storage_), repl_state_(kReplConnecting), @@ -364,8 +365,7 @@ void ReplicationThread::run() { ReplicationThread::CBState ReplicationThread::authWriteCB(bufferevent *bev, void *ctx) { auto self = static_cast(ctx); - const auto auth_len_str = std::to_string(self->auth_.length()); - send_string(bev, Redis::MultiBulkString({"AUTH", self->auth_})); + send_string(bev, Redis::MultiBulkString({"AUTH", self->srv_->GetConfig()->masterauth})); LOG(INFO) << "[replication] Auth request was sent, waiting for response"; self->repl_state_ = kReplSendAuth; return CBState::NEXT; @@ -633,7 +633,7 @@ ReplicationThread::CBState ReplicationThread::fullSyncReadCB(bufferevent *bev, free(line); target_dir = self->srv_->GetConfig()->sync_checkpoint_dir; - // Clean invaild files of checkpoint, "CURRENT" file must be invalid + // Clean invalid files of checkpoint, "CURRENT" file must be invalid // because we identify one file by its file number but only "CURRENT" // file doesn't have number. auto iter = std::find(need_files.begin(), need_files.end(), "CURRENT"); @@ -783,9 +783,10 @@ Status ReplicationThread::sendAuth(int sock_fd) { size_t line_len; // Send auth when needed - if (!auth_.empty()) { + std::string auth = srv_->GetConfig()->masterauth; + if (!auth.empty()) { evbuffer *evbuf = evbuffer_new(); - const auto auth_command = Redis::MultiBulkString({"AUTH", auth_}); + const auto auth_command = Redis::MultiBulkString({"AUTH", auth}); auto s = Util::SockSend(sock_fd, auth_command); if (!s.IsOK()) return Status(Status::NotOK, "send auth command err:"+s.Msg()); while (true) { diff --git a/src/replication.h b/src/replication.h index 51ed9be9078..44cf0fef5d4 100644 --- a/src/replication.h +++ b/src/replication.h @@ -65,7 +65,7 @@ class FeedSlaveThread { class ReplicationThread { public: explicit ReplicationThread(std::string host, uint32_t port, - Server *srv, std::string auth = ""); + Server *srv); Status Start(std::function &&pre_fullsync_cb, std::function &&post_fullsync_cb); void Stop(); @@ -124,7 +124,6 @@ class ReplicationThread { bool stop_flag_ = false; std::string host_; uint32_t port_; - std::string auth_; Server *srv_ = nullptr; Engine::Storage *storage_ = nullptr; ReplState repl_state_; diff --git a/src/server.cc b/src/server.cc index ce1e32afae5..62a5f670e38 100644 --- a/src/server.cc +++ b/src/server.cc @@ -178,7 +178,7 @@ Status Server::AddMaster(std::string host, uint32_t port, bool force_reconnect) uint32_t master_listen_port = port; if (GetConfig()->master_use_repl_port) master_listen_port += 1; replication_thread_ = std::unique_ptr( - new ReplicationThread(host, master_listen_port, this, config_->masterauth)); + new ReplicationThread(host, master_listen_port, this)); auto s = replication_thread_->Start( [this]() { PrepareRestoreDB(); diff --git a/tests/tcl/tests/integration/replication.tcl b/tests/tcl/tests/integration/replication.tcl index 129b35c8e7c..cdce19e5d0f 100644 --- a/tests/tcl/tests/integration/replication.tcl +++ b/tests/tcl/tests/integration/replication.tcl @@ -244,3 +244,46 @@ start_server {tags {"repl"}} { } } } + +start_server {tags {"repl"}} { + set slave [srv 0 client] + set slave_ip [srv 0 host] + set slave_port [srv 0 port] + start_server {} { + set master [srv 0 client] + set master_ip [srv 0 host] + set master_port [srv 0 port] + test {Slave can re-sync with master after password change} { + $slave slaveof $master_ip $master_port + after 200 + catch {$slave info replication} var + assert_match {*role:slave*} $var + catch {$master info replication} var + assert_match {*role:master*[$slave_ip]*[$slave_port]*} $var + + # Change password and break repl connection + $master config set requirepass pass + $slave config set requirepass pass + $slave config set masterauth pass + + set ret [$master client list] + set lines [split $ret "\n"] + global conn_addr + foreach line $lines { + if {[string match "*cmd=psync*" $line]} { + set list [split $line " "] + foreach str $list { + if {[string match "*addr=*" $str]} { + set conn_addr [string range $str 5 end] + } + } + } + } + + $master client kill $conn_addr + after 200 + catch {$master info replication} var + assert_match {*role:master*[$slave_ip]*[$slave_port]*} $var + } + } +} diff --git a/tests/tcl/tests/unit/auth.tcl b/tests/tcl/tests/unit/auth.tcl index cb12d414eb5..97e84f0163e 100644 --- a/tests/tcl/tests/unit/auth.tcl +++ b/tests/tcl/tests/unit/auth.tcl @@ -9,7 +9,7 @@ start_server {tags {"auth"} overrides {requirepass foobar}} { test {AUTH fails when a wrong password is given} { catch {r auth wrong!} err set _ $err - } {*invaild password*} + } {*invalid password*} test {Arbitrary command gives an error when AUTH is required} { catch {r set foo bar} err