Cache client IP in RemoteClient so it can always be retrieved (#10887)

specifically: after the peer has already disappeared

6 files changed, 84 insertions, 99 deletions

diff --git a/src/clientiface.cpp b/src/clientiface.cpp
index 01852c5d1..797afd3c1 100644
--- a/src/clientiface.cpp
+++ b/src/clientiface.cpp
@@ -452,9 +452,6 @@ void RemoteClient::notifyEvent(ClientStateEvent event)
 		case CSE_Hello:
 			m_state = CS_HelloSent;
 			break;
-		case CSE_InitLegacy:
-			m_state = CS_AwaitingInit2;
-			break;
 		case CSE_Disconnect:
 			m_state = CS_Disconnecting;
 			break;
diff --git a/src/clientiface.h b/src/clientiface.h
index eabffb0b6..cc5292b71 100644
--- a/src/clientiface.h
+++ b/src/clientiface.h
@@ -25,6 +25,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "serialization.h"             // for SER_FMT_VER_INVALID
 #include "network/networkpacket.h"
 #include "network/networkprotocol.h"
+#include "network/address.h"
 #include "porting.h"
 
 #include <list>
@@ -188,7 +189,6 @@ enum ClientStateEvent
 {
 	CSE_Hello,
 	CSE_AuthAccept,
-	CSE_InitLegacy,
 	CSE_GotInit2,
 	CSE_SetDenied,
 	CSE_SetDefinitionsSent,
@@ -338,17 +338,24 @@ public:
 	u8 getMajor() const { return m_version_major; }
 	u8 getMinor() const { return m_version_minor; }
 	u8 getPatch() const { return m_version_patch; }
-	const std::string &getFull() const { return m_full_version; }
+	const std::string &getFullVer() const { return m_full_version; }
 	
 	void setLangCode(const std::string &code) { m_lang_code = code; }
 	const std::string &getLangCode() const { return m_lang_code; }
+
+	void setCachedAddress(const Address &addr) { m_addr = addr; }
+	const Address &getAddress() const { return m_addr; }
+
 private:
 	// Version is stored in here after INIT before INIT2
 	u8 m_pending_serialization_version = SER_FMT_VER_INVALID;
 
 	/* current state of client */
 	ClientState m_state = CS_Created;
-	
+
+	// Cached here so retrieval doesn't have to go to connection API
+	Address m_addr;
+
 	// Client sent language code
 	std::string m_lang_code;
 
@@ -412,7 +419,7 @@ private:
 
 	/*
 		client information
-	 */
+	*/
 	u8 m_version_major = 0;
 	u8 m_version_minor = 0;
 	u8 m_version_patch = 0;
diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index c636d01e1..882cf71c1 100644
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -56,12 +56,12 @@ void Server::handleCommand_Init(NetworkPacket* pkt)
 	session_t peer_id = pkt->getPeerId();
 	RemoteClient *client = getClient(peer_id, CS_Created);
 
+	Address addr;
 	std::string addr_s;
 	try {
-		Address address = getPeerAddress(peer_id);
-		addr_s = address.serializeString();
-	}
-	catch (con::PeerNotFoundException &e) {
+		addr = m_con->GetPeerAddress(peer_id);
+		addr_s = addr.serializeString();
+	} catch (con::PeerNotFoundException &e) {
 		/*
 		 * no peer for this packet found
 		 * most common reason is peer timeout, e.g. peer didn't
@@ -73,13 +73,14 @@ void Server::handleCommand_Init(NetworkPacket* pkt)
 		return;
 	}
 
-	// If net_proto_version is set, this client has already been handled
 	if (client->getState() > CS_Created) {
 		verbosestream << "Server: Ignoring multiple TOSERVER_INITs from " <<
 			addr_s << " (peer_id=" << peer_id << ")" << std::endl;
 		return;
 	}
 
+	client->setCachedAddress(addr);
+
 	verbosestream << "Server: Got TOSERVER_INIT from " << addr_s <<
 		" (peer_id=" << peer_id << ")" << std::endl;
 
diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp
index 6f934bb9d..0ae699c9f 100644
--- a/src/script/lua_api/l_server.cpp
+++ b/src/script/lua_api/l_server.cpp
@@ -116,24 +116,18 @@ int ModApiServer::l_get_player_privs(lua_State *L)
 int ModApiServer::l_get_player_ip(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
-	const char * name = luaL_checkstring(L, 1);
-	RemotePlayer *player = dynamic_cast<ServerEnvironment *>(getEnv(L))->getPlayer(name);
-	if(player == NULL)
-	{
+
+	Server *server = getServer(L);
+
+	const char *name = luaL_checkstring(L, 1);
+	RemotePlayer *player = server->getEnv().getPlayer(name);
+	if (!player) {
 		lua_pushnil(L); // no such player
 		return 1;
 	}
-	try
-	{
-		Address addr = getServer(L)->getPeerAddress(player->getPeerId());
-		std::string ip_str = addr.serializeString();
-		lua_pushstring(L, ip_str.c_str());
-		return 1;
-	} catch (const con::PeerNotFoundException &) {
-		dstream << FUNCTION_NAME << ": peer was not found" << std::endl;
-		lua_pushnil(L); // error
-		return 1;
-	}
+
+	lua_pushstring(L, server->getPeerAddress(player->getPeerId()).serializeString().c_str());
+	return 1;
 }
 
 // get_player_information(name)
@@ -150,26 +144,18 @@ int ModApiServer::l_get_player_information(lua_State *L)
 		return 1;
 	}
 
-	Address addr;
-	try {
-		addr = server->getPeerAddress(player->getPeerId());
-	} catch (const con::PeerNotFoundException &) {
-		dstream << FUNCTION_NAME << ": peer was not found" << std::endl;
-		lua_pushnil(L); // error
-		return 1;
-	}
-
-	float min_rtt, max_rtt, avg_rtt, min_jitter, max_jitter, avg_jitter;
-	ClientState state;
-	u32 uptime;
-	u16 prot_vers;
-	u8 ser_vers, major, minor, patch;
-	std::string vers_string, lang_code;
+	/*
+		Be careful not to introduce a depdendency on the connection to
+		the peer here. This function is >>REQUIRED<< to still be able to return
+		values even when the peer unexpectedly disappears.
+		Hence all the ConInfo values here are optional.
+	*/
 
 	auto getConInfo = [&] (con::rtt_stat_type type, float *value) -> bool {
 		return server->getClientConInfo(player->getPeerId(), type, value);
 	};
 
+	float min_rtt, max_rtt, avg_rtt, min_jitter, max_jitter, avg_jitter;
 	bool have_con_info =
 		getConInfo(con::MIN_RTT, &min_rtt) &&
 		getConInfo(con::MAX_RTT, &max_rtt) &&
@@ -178,11 +164,9 @@ int ModApiServer::l_get_player_information(lua_State *L)
 		getConInfo(con::MAX_JITTER, &max_jitter) &&
 		getConInfo(con::AVG_JITTER, &avg_jitter);
 
-	bool r = server->getClientInfo(player->getPeerId(), &state, &uptime,
-		&ser_vers, &prot_vers, &major, &minor, &patch, &vers_string,
-		&lang_code);
-	if (!r) {
-		dstream << FUNCTION_NAME << ": peer was not found" << std::endl;
+	ClientInfo info;
+	if (!server->getClientInfo(player->getPeerId(), info)) {
+		warningstream << FUNCTION_NAME << ": no client info?!" << std::endl;
 		lua_pushnil(L); // error
 		return 1;
 	}
@@ -191,13 +175,13 @@ int ModApiServer::l_get_player_information(lua_State *L)
 	int table = lua_gettop(L);
 
 	lua_pushstring(L,"address");
-	lua_pushstring(L, addr.serializeString().c_str());
+	lua_pushstring(L, info.addr.serializeString().c_str());
 	lua_settable(L, table);
 
 	lua_pushstring(L,"ip_version");
-	if (addr.getFamily() == AF_INET) {
+	if (info.addr.getFamily() == AF_INET) {
 		lua_pushnumber(L, 4);
-	} else if (addr.getFamily() == AF_INET6) {
+	} else if (info.addr.getFamily() == AF_INET6) {
 		lua_pushnumber(L, 6);
 	} else {
 		lua_pushnumber(L, 0);
@@ -231,11 +215,11 @@ int ModApiServer::l_get_player_information(lua_State *L)
 	}
 
 	lua_pushstring(L,"connection_uptime");
-	lua_pushnumber(L, uptime);
+	lua_pushnumber(L, info.uptime);
 	lua_settable(L, table);
 
 	lua_pushstring(L,"protocol_version");
-	lua_pushnumber(L, prot_vers);
+	lua_pushnumber(L, info.prot_vers);
 	lua_settable(L, table);
 
 	lua_pushstring(L, "formspec_version");
@@ -243,32 +227,32 @@ int ModApiServer::l_get_player_information(lua_State *L)
 	lua_settable(L, table);
 
 	lua_pushstring(L, "lang_code");
-	lua_pushstring(L, lang_code.c_str());
+	lua_pushstring(L, info.lang_code.c_str());
 	lua_settable(L, table);
 
 #ifndef NDEBUG
 	lua_pushstring(L,"serialization_version");
-	lua_pushnumber(L, ser_vers);
+	lua_pushnumber(L, info.ser_vers);
 	lua_settable(L, table);
 
 	lua_pushstring(L,"major");
-	lua_pushnumber(L, major);
+	lua_pushnumber(L, info.major);
 	lua_settable(L, table);
 
 	lua_pushstring(L,"minor");
-	lua_pushnumber(L, minor);
+	lua_pushnumber(L, info.minor);
 	lua_settable(L, table);
 
 	lua_pushstring(L,"patch");
-	lua_pushnumber(L, patch);
+	lua_pushnumber(L, info.patch);
 	lua_settable(L, table);
 
 	lua_pushstring(L,"version_string");
-	lua_pushstring(L, vers_string.c_str());
+	lua_pushstring(L, info.vers_string.c_str());
 	lua_settable(L, table);
 
 	lua_pushstring(L,"state");
-	lua_pushstring(L,ClientInterface::state2Name(state).c_str());
+	lua_pushstring(L, ClientInterface::state2Name(info.state).c_str());
 	lua_settable(L, table);
 #endif
 
@@ -296,23 +280,18 @@ int ModApiServer::l_get_ban_description(lua_State *L)
 int ModApiServer::l_ban_player(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
-	const char * name = luaL_checkstring(L, 1);
-	RemotePlayer *player = dynamic_cast<ServerEnvironment *>(getEnv(L))->getPlayer(name);
-	if (player == NULL) {
+
+	Server *server = getServer(L);
+
+	const char *name = luaL_checkstring(L, 1);
+	RemotePlayer *player = server->getEnv().getPlayer(name);
+	if (!player) {
 		lua_pushboolean(L, false); // no such player
 		return 1;
 	}
-	try
-	{
-		Address addr = getServer(L)->getPeerAddress(
-			dynamic_cast<ServerEnvironment *>(getEnv(L))->getPlayer(name)->getPeerId());
-		std::string ip_str = addr.serializeString();
-		getServer(L)->setIpBanned(ip_str, name);
-	} catch(const con::PeerNotFoundException &) {
-		dstream << FUNCTION_NAME << ": peer was not found" << std::endl;
-		lua_pushboolean(L, false); // error
-		return 1;
-	}
+
+	std::string ip_str = server->getPeerAddress(player->getPeerId()).serializeString();
+	server->setIpBanned(ip_str, name);
 	lua_pushboolean(L, true);
 	return 1;
 }
diff --git a/src/server.cpp b/src/server.cpp
index aba7b6401..76a817701 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -1242,20 +1242,8 @@ bool Server::getClientConInfo(session_t peer_id, con::rtt_stat_type type, float*
 	return *retval != -1;
 }
 
-bool Server::getClientInfo(
-		session_t    peer_id,
-		ClientState* state,
-		u32*         uptime,
-		u8*          ser_vers,
-		u16*         prot_vers,
-		u8*          major,
-		u8*          minor,
-		u8*          patch,
-		std::string* vers_string,
-		std::string* lang_code
-	)
-{
-	*state = m_clients.getClientState(peer_id);
+bool Server::getClientInfo(session_t peer_id, ClientInfo &ret)
+{
 	m_clients.lock();
 	RemoteClient* client = m_clients.lockedGetClientNoEx(peer_id, CS_Invalid);
 
@@ -1264,15 +1252,18 @@ bool Server::getClientInfo(
 		return false;
 	}
 
-	*uptime = client->uptime();
-	*ser_vers = client->serialization_version;
-	*prot_vers = client->net_proto_version;
+	ret.state = client->getState();
+	ret.addr = client->getAddress();
+	ret.uptime = client->uptime();
+	ret.ser_vers = client->serialization_version;
+	ret.prot_vers = client->net_proto_version;
+
+	ret.major = client->getMajor();
+	ret.minor = client->getMinor();
+	ret.patch = client->getPatch();
+	ret.vers_string = client->getFullVer();
 
-	*major = client->getMajor();
-	*minor = client->getMinor();
-	*patch = client->getPatch();
-	*vers_string = client->getFull();
-	*lang_code = client->getLangCode();
+	ret.lang_code = client->getLangCode();
 
 	m_clients.unlock();
 
@@ -3339,7 +3330,8 @@ void Server::hudSetHotbarSelectedImage(RemotePlayer *player, const std::string &
 
 Address Server::getPeerAddress(session_t peer_id)
 {
-	return m_con->GetPeerAddress(peer_id);
+	// Note that this is only set after Init was received in Server::handleCommand_Init
+	return getClient(peer_id, CS_Invalid)->getAddress();
 }
 
 void Server::setLocalPlayerAnimations(RemotePlayer *player,
diff --git a/src/server.h b/src/server.h
index 1dd181794..7071d2d07 100644
--- a/src/server.h
+++ b/src/server.h
@@ -126,6 +126,17 @@ struct MinimapMode {
 	u16 scale = 1;
 };
 
+// structure for everything getClientInfo returns, for convenience
+struct ClientInfo {
+	ClientState state;
+	Address addr;
+	u32 uptime;
+	u8 ser_vers;
+	u16 prot_vers;
+	u8 major, minor, patch;
+	std::string vers_string, lang_code;
+};
+
 class Server : public con::PeerHandler, public MapEventReceiver,
 		public IGameDef
 {
@@ -326,9 +337,7 @@ public:
 	void DenyAccess_Legacy(session_t peer_id, const std::wstring &reason);
 	void DisconnectPeer(session_t peer_id);
 	bool getClientConInfo(session_t peer_id, con::rtt_stat_type type, float *retval);
-	bool getClientInfo(session_t peer_id, ClientState *state, u32 *uptime,
-			u8* ser_vers, u16* prot_vers, u8* major, u8* minor, u8* patch,
-			std::string* vers_string, std::string* lang_code);
+	bool getClientInfo(session_t peer_id, ClientInfo &ret);
 
 	void printToConsoleOnly(const std::string &text);