Fix: Thread unsafe use of NetworkAddress::GetAddressAsString

Remove static buffer form of NetworkAddress::GetAddressAsString.
This is used in multiple threads concurrently, and is not thread-safe.

Replace it with a form returning std::string.

4 files changed, 19 insertions, 16 deletions

diff --git a/src/network/core/address.cpp b/src/network/core/address.cpp
index 44ba45385..c2fecc7ff 100644
--- a/src/network/core/address.cpp
+++ b/src/network/core/address.cpp
@@ -96,12 +96,11 @@ void NetworkAddress::GetAddressAsString(char *buffer, const char *last, bool wit
  * Get the address as a string, e.g. 127.0.0.1:12345.
  * @param with_family whether to add the family (e.g. IPvX).
  * @return the address
- * @note NOT thread safe
  */
-const char *NetworkAddress::GetAddressAsString(bool with_family)
+std::string NetworkAddress::GetAddressAsString(bool with_family)
 {
 	/* 6 = for the : and 5 for the decimal port number */
-	static char buf[NETWORK_HOSTNAME_LENGTH + 6 + 7];
+	char buf[NETWORK_HOSTNAME_LENGTH + 6 + 7];
 	this->GetAddressAsString(buf, lastof(buf), with_family);
 	return buf;
 }
@@ -289,7 +288,8 @@ static SOCKET ConnectLoopProc(addrinfo *runp)
 {
 	const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype);
 	const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family);
-	const char *address = NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString();
+	char address[NETWORK_HOSTNAME_LENGTH + 6 + 7];
+	NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(address, lastof(address));
 
 	SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol);
 	if (sock == INVALID_SOCKET) {
@@ -319,7 +319,7 @@ static SOCKET ConnectLoopProc(addrinfo *runp)
  */
 SOCKET NetworkAddress::Connect()
 {
-	DEBUG(net, 1, "Connecting to %s", this->GetAddressAsString());
+	DEBUG(net, 1, "Connecting to %s", this->GetAddressAsString().c_str());
 
 	return this->Resolve(AF_UNSPEC, SOCK_STREAM, AI_ADDRCONFIG, nullptr, ConnectLoopProc);
 }
@@ -333,7 +333,8 @@ static SOCKET ListenLoopProc(addrinfo *runp)
 {
 	const char *type = NetworkAddress::SocketTypeAsString(runp->ai_socktype);
 	const char *family = NetworkAddress::AddressFamilyAsString(runp->ai_family);
-	const char *address = NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString();
+	char address[NETWORK_HOSTNAME_LENGTH + 6 + 7];
+	NetworkAddress(runp->ai_addr, (int)runp->ai_addrlen).GetAddressAsString(address, lastof(address));
 
 	SOCKET sock = socket(runp->ai_family, runp->ai_socktype, runp->ai_protocol);
 	if (sock == INVALID_SOCKET) {
diff --git a/src/network/core/address.h b/src/network/core/address.h
index 980d7227d..65546818b 100644
--- a/src/network/core/address.h
+++ b/src/network/core/address.h
@@ -15,6 +15,8 @@
 #include "../../string_func.h"
 #include "../../core/smallmap_type.hpp"
 
+#include <string>
+
 class NetworkAddress;
 typedef std::vector<NetworkAddress> NetworkAddressList; ///< Type for a list of addresses.
 typedef SmallMap<NetworkAddress, SOCKET> SocketList;    ///< Type for a mapping between address and socket.
@@ -91,7 +93,7 @@ public:
 
 	const char *GetHostname();
 	void GetAddressAsString(char *buffer, const char *last, bool with_family = true);
-	const char *GetAddressAsString(bool with_family = true);
+	std::string GetAddressAsString(bool with_family = true);
 	const sockaddr_storage *GetAddress();
 
 	/**
diff --git a/src/network/core/tcp_content.cpp b/src/network/core/tcp_content.cpp
index 989fbefaf..6fb3b3379 100644
--- a/src/network/core/tcp_content.cpp
+++ b/src/network/core/tcp_content.cpp
@@ -171,9 +171,9 @@ bool NetworkContentSocketHandler::HandlePacket(Packet *p)
 
 		default:
 			if (this->HasClientQuit()) {
-				DEBUG(net, 0, "[tcp/content] received invalid packet type %d from %s", type, this->client_addr.GetAddressAsString());
+				DEBUG(net, 0, "[tcp/content] received invalid packet type %d from %s", type, this->client_addr.GetAddressAsString().c_str());
 			} else {
-				DEBUG(net, 0, "[tcp/content] received illegal packet from %s", this->client_addr.GetAddressAsString());
+				DEBUG(net, 0, "[tcp/content] received illegal packet from %s", this->client_addr.GetAddressAsString().c_str());
 			}
 			return false;
 	}
@@ -224,7 +224,7 @@ bool NetworkContentSocketHandler::ReceivePackets()
  */
 bool NetworkContentSocketHandler::ReceiveInvalidPacket(PacketContentType type)
 {
-	DEBUG(net, 0, "[tcp/content] received illegal packet type %d from %s", type, this->client_addr.GetAddressAsString());
+	DEBUG(net, 0, "[tcp/content] received illegal packet type %d from %s", type, this->client_addr.GetAddressAsString().c_str());
 	return false;
 }
 
diff --git a/src/network/core/udp.cpp b/src/network/core/udp.cpp
index 123683873..57352412b 100644
--- a/src/network/core/udp.cpp
+++ b/src/network/core/udp.cpp
@@ -100,10 +100,10 @@ void NetworkUDPSocketHandler::SendPacket(Packet *p, NetworkAddress *recv, bool a
 
 		/* Send the buffer */
 		int res = sendto(s.second, (const char*)p->buffer, p->size, 0, (const struct sockaddr *)send.GetAddress(), send.GetAddressLength());
-		DEBUG(net, 7, "[udp] sendto(%s)", send.GetAddressAsString());
+		DEBUG(net, 7, "[udp] sendto(%s)", send.GetAddressAsString().c_str());
 
 		/* Check for any errors, but ignore it otherwise */
-		if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %i", send.GetAddressAsString(), GET_LAST_ERROR());
+		if (res == -1) DEBUG(net, 1, "[udp] sendto(%s) failed with: %i", send.GetAddressAsString().c_str(), GET_LAST_ERROR());
 
 		if (!all) break;
 	}
@@ -136,7 +136,7 @@ void NetworkUDPSocketHandler::ReceivePackets()
 			/* If the size does not match the packet must be corrupted.
 			 * Otherwise it will be marked as corrupted later on. */
 			if (nbytes != p.size) {
-				DEBUG(net, 1, "received a packet with mismatching size from %s", address.GetAddressAsString());
+				DEBUG(net, 1, "received a packet with mismatching size from %s", address.GetAddressAsString().c_str());
 				continue;
 			}
 
@@ -313,9 +313,9 @@ void NetworkUDPSocketHandler::HandleUDPPacket(Packet *p, NetworkAddress *client_
 
 		default:
 			if (this->HasClientQuit()) {
-				DEBUG(net, 0, "[udp] received invalid packet type %d from %s", type, client_addr->GetAddressAsString());
+				DEBUG(net, 0, "[udp] received invalid packet type %d from %s", type, client_addr->GetAddressAsString().c_str());
 			} else {
-				DEBUG(net, 0, "[udp] received illegal packet from %s", client_addr->GetAddressAsString());
+				DEBUG(net, 0, "[udp] received illegal packet from %s", client_addr->GetAddressAsString().c_str());
 			}
 			break;
 	}
@@ -328,7 +328,7 @@ void NetworkUDPSocketHandler::HandleUDPPacket(Packet *p, NetworkAddress *client_
  */
 void NetworkUDPSocketHandler::ReceiveInvalidPacket(PacketUDPType type, NetworkAddress *client_addr)
 {
-	DEBUG(net, 0, "[udp] received packet type %d on wrong port from %s", type, client_addr->GetAddressAsString());
+	DEBUG(net, 0, "[udp] received packet type %d on wrong port from %s", type, client_addr->GetAddressAsString().c_str());
 }
 
 void NetworkUDPSocketHandler::Receive_CLIENT_FIND_SERVER(Packet *p, NetworkAddress *client_addr) { this->ReceiveInvalidPacket(PACKET_UDP_CLIENT_FIND_SERVER, client_addr); }