Fix slave can't resync with master after password change (#520)

Co-authored-by: zoumingfo <zoumingfo@baidu.com>

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

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

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<ReplicationThread *>(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) {

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<void()> &&pre_fullsync_cb,
                std::function<void()> &&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_;

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<ReplicationThread>(
-      new ReplicationThread(host, master_listen_port, this, config_->masterauth));
+      new ReplicationThread(host, master_listen_port, this));
   auto s = replication_thread_->Start(
       [this]() {
         PrepareRestoreDB();

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
+        }
+    }
+}

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