Authenticator: avoid move assignments to self (#5315)

If authentication was off cClientHandle::m_Username ended up moved into itself. Add a copy to avoid this. Thanks @Seadragon91!

src/ClientHandle.cpp

@@ -335,49 +335,47 @@ bool cClientHandle::BungeeAuthenticate()
 
 void cClientHandle::Authenticate(AString && a_Name, const cUUID & a_UUID, Json::Value && a_Properties)
 {
+	cCSLock Lock(m_CSState);
+	/*
+	LOGD("Processing authentication for client %s @ %s (%p), state = %d",
+		m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this), m_State.load()
+	);
+	//*/
+
+	if (m_State != csAuthenticating)
 	{
-		cCSLock lock(m_CSState);
+		return;
-		/*
+	}
-		LOGD("Processing authentication for client %s @ %s (%p), state = %d",
-			m_Username.c_str(), m_IPString.c_str(), static_cast<void *>(this), m_State.load()
-		);
-		//*/
 
-		if (m_State != csAuthenticating)
+	ASSERT(m_Player == nullptr);
-		{
-			return;
-		}
 
-		ASSERT(m_Player == nullptr);
+	if (!BungeeAuthenticate())
+	{
+		return;
+	}
 
-		if (!BungeeAuthenticate())
+	m_Username = std::move(a_Name);
-		{
-			return;
-		}
 
-		m_Username = std::move(a_Name);
+	// Only assign UUID and properties if not already pre-assigned (BungeeCord sends those in the Handshake packet):
+	if (m_UUID.IsNil())
+	{
+		m_UUID = a_UUID;
+	}
+	if (m_Properties.empty())
+	{
+		m_Properties = std::move(a_Properties);
+	}
 
-		// Only assign UUID and properties if not already pre-assigned (BungeeCord sends those in the Handshake packet):
+	// Send login success (if the protocol supports it):
-		if (m_UUID.IsNil())
+	m_Protocol->SendLoginSuccess();
-		{
-			m_UUID = a_UUID;
-		}
-		if (m_Properties.empty())
-		{
-			m_Properties = std::move(a_Properties);
-		}
 
-		// Send login success (if the protocol supports it):
+	if (m_ForgeHandshake.IsForgeClient)
-		m_Protocol->SendLoginSuccess();
+	{
-
+		m_ForgeHandshake.BeginForgeHandshake(*this);
-		if (m_ForgeHandshake.IsForgeClient)
+	}
-		{
+	else
-			m_ForgeHandshake.BeginForgeHandshake(*this);
+	{
-		}
+		FinishAuthenticate();
-		else
-		{
-			FinishAuthenticate();
-		}
 	}
 }
 
@@ -745,8 +743,8 @@ bool cClientHandle::HandleLogin()
 		m_State = csAuthenticating;
 	}  // lock(m_CSState)
 
-	// Schedule for authentication; until then, let the player wait (but do not block)
+	// Schedule for authentication; until then, let the player wait (but do not block):
-	cRoot::Get()->GetAuthenticator().Authenticate(GetUniqueID(), std::move(m_Username), m_Protocol->GetAuthServerID());
+	cRoot::Get()->GetAuthenticator().Authenticate(GetUniqueID(), m_Username, m_Protocol->GetAuthServerID());
 	return true;
 }
 

src/Protocol/Authenticator.cpp

@@ -57,17 +57,23 @@ void cAuthenticator::ReadSettings(cSettingsRepositoryInterface & a_Settings)
 
 
 
-void cAuthenticator::Authenticate(int a_ClientID, AString && a_Username, const AString & a_ServerHash)
+void cAuthenticator::Authenticate(int a_ClientID, const std::string_view a_Username, const std::string_view a_ServerHash)
 {
 	if (!m_ShouldAuthenticate)
 	{
-		const auto UUID = cClientHandle::GenerateOfflineUUID(a_Username);
+		// An "authenticated" username, which is just what the client sent since auth is off.
-		cRoot::Get()->GetServer()->AuthenticateUser(a_ClientID, std::move(a_Username), UUID, Json::Value{});
+		std::string OfflineUsername(a_Username);
+
+		// A specially constructed UUID based wholly on the username.
+		const auto OfflineUUID = cClientHandle::GenerateOfflineUUID(OfflineUsername);
+
+		// "Authenticate" the user based on what little information we have:
+		cRoot::Get()->GetServer()->AuthenticateUser(a_ClientID, std::move(OfflineUsername), OfflineUUID, Json::Value());
 		return;
 	}
 
 	cCSLock Lock(m_CS);
-	m_Queue.emplace_back(a_ClientID, std::move(a_Username), a_ServerHash);
+	m_Queue.emplace_back(a_ClientID, a_Username, a_ServerHash);
 	m_QueueNonempty.Set();
 }
 
@@ -114,17 +120,17 @@ void cAuthenticator::Execute(void)
 
 		cAuthenticator::cUser User = std::move(m_Queue.front());
 		int & ClientID = User.m_ClientID;
-		AString & UserName = User.m_Name;
+		AString & Username = User.m_Name;
 		AString & ServerID = User.m_ServerID;
 		m_Queue.pop_front();
 		Lock.Unlock();
 
 		cUUID UUID;
 		Json::Value Properties;
-		if (AuthWithYggdrasil(UserName, ServerID, UUID, Properties))
+		if (AuthWithYggdrasil(Username, ServerID, UUID, Properties))
 		{
-			LOGINFO("User %s authenticated with UUID %s", UserName.c_str(), UUID.ToShortString().c_str());
+			LOGINFO("User %s authenticated with UUID %s", Username.c_str(), UUID.ToShortString().c_str());
-			cRoot::Get()->GetServer()->AuthenticateUser(ClientID, std::move(UserName), UUID, std::move(Properties));
+			cRoot::Get()->GetServer()->AuthenticateUser(ClientID, std::move(Username), UUID, std::move(Properties));
 		}
 		else
 		{

src/Protocol/Authenticator.h

@@ -41,7 +41,7 @@ public:
 	void ReadSettings(cSettingsRepositoryInterface & a_Settings);
 
 	/** Queues a request for authenticating a user. If the auth fails, the user will be kicked */
-	void Authenticate(int a_ClientID, AString && a_Username, const AString & a_ServerHash);
+	void Authenticate(int a_ClientID, std::string_view a_Username, std::string_view a_ServerHash);
 
 	/** Starts the authenticator thread. The thread may be started and stopped repeatedly */
 	void Start(cSettingsRepositoryInterface & a_Settings);
@@ -58,7 +58,7 @@ private:
 		AString m_Name;
 		AString m_ServerID;
 
-		cUser(int a_ClientID, const AString & a_Name, const AString & a_ServerID) :
+		cUser(int a_ClientID, const std::string_view a_Name, const std::string_view a_ServerID) :
 			m_ClientID(a_ClientID),
 			m_Name(a_Name),
 			m_ServerID(a_ServerID)