From 1d8d01074fdb52946f81110bebf1d001185b394b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Blot?= <nerzhul@users.noreply.github.com>
Date: Wed, 16 Aug 2017 23:48:29 +0200
Subject: [PATCH] ClientInterface: add a function to verify (correctly) if user
 limit was reached (#6258)

* ClientInterface: add a function to verify (correctly) if user limit was reached

CS_HelloSent is a better indicator of active slots than CS_Created, which are session objects created after init packet reception

Switch existing checks to ClientInterface::isUserLimitReached()

Use range-based for loop for getClientIds() used function too

This will fix #6254 (not the memory overhead if init is flooded)
---
 src/clientiface.cpp                 | 17 +++++++++++++----
 src/clientiface.h                   |  4 +++-
 src/network/serverpackethandler.cpp |  4 ++--
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/clientiface.cpp b/src/clientiface.cpp
index cdb64e192..f07f02012 100644
--- a/src/clientiface.cpp
+++ b/src/clientiface.cpp
@@ -622,15 +622,24 @@ std::vector<u16> ClientInterface::getClientIDs(ClientState min_state)
 	std::vector<u16> reply;
 	MutexAutoLock clientslock(m_clients_mutex);
 
-	for (RemoteClientMap::iterator i = m_clients.begin();
-		i != m_clients.end(); ++i) {
-		if (i->second->getState() >= min_state)
-			reply.push_back(i->second->peer_id);
+	for (const auto &m_client : m_clients) {
+		if (m_client.second->getState() >= min_state)
+			reply.push_back(m_client.second->peer_id);
 	}
 
 	return reply;
 }
 
+/**
+ * Verify if user limit was reached.
+ * User limit count all clients from HelloSent state (MT protocol user) to Active state
+ * @return true if user limit was reached
+ */
+bool ClientInterface::isUserLimitReached()
+{
+	return getClientIDs(CS_HelloSent).size() >= g_settings->getU16("max_users");
+}
+
 void ClientInterface::step(float dtime)
 {
 	m_print_info_timer += dtime;
diff --git a/src/clientiface.h b/src/clientiface.h
index edc389cbb..b19a91b6f 100644
--- a/src/clientiface.h
+++ b/src/clientiface.h
@@ -427,6 +427,9 @@ public:
 	/* get list of active client id's */
 	std::vector<u16> getClientIDs(ClientState min_state=CS_Active);
 
+	/* verify is server user limit was reached */
+	bool isUserLimitReached();
+
 	/* get list of client player names */
 	const std::vector<std::string> &getPlayerNames() const { return m_clients_names; }
 
@@ -472,7 +475,6 @@ public:
 	}
 
 	static std::string state2Name(ClientState state);
-
 protected:
 	//TODO find way to avoid this functions
 	void lock() { m_clients_mutex.lock(); }
diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index 2151450d2..b0fb17b72 100644
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -211,7 +211,7 @@ void Server::handleCommand_Init(NetworkPacket* pkt)
 
 	// Enforce user limit.
 	// Don't enforce for users that have some admin right
-	if (m_clients.getClientIDs(CS_Created).size() >= g_settings->getU16("max_users") &&
+	if (m_clients.isUserLimitReached() &&
 			!checkPriv(playername, "server") &&
 			!checkPriv(playername, "ban") &&
 			!checkPriv(playername, "privs") &&
@@ -520,7 +520,7 @@ void Server::handleCommand_Init_Legacy(NetworkPacket* pkt)
 
 	// Enforce user limit.
 	// Don't enforce for users that have some admin right
-	if (m_clients.getClientIDs(CS_Created).size() >= g_settings->getU16("max_users") &&
+	if (m_clients.isUserLimitReached() >= g_settings->getU16("max_users") &&
 			!checkPriv(playername, "server") &&
 			!checkPriv(playername, "ban") &&
 			!checkPriv(playername, "privs") &&