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.

6 files changed, 28 insertions, 23 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); }
diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp
index c4488f35d..ab72d9521 100644
--- a/src/network/network_gui.cpp
+++ b/src/network/network_gui.cpp
@@ -664,7 +664,9 @@ public:
 			DrawString(r.left + WD_FRAMERECT_LEFT, r.right - WD_FRAMERECT_RIGHT, y, STR_NETWORK_SERVER_LIST_SERVER_VERSION); // server version
 			y += FONT_HEIGHT_NORMAL;
 
-			SetDParamStr(0, sel->address.GetAddressAsString());
+			char network_addr_buffer[NETWORK_HOSTNAME_LENGTH + 6 + 7];
+			sel->address.GetAddressAsString(network_addr_buffer, lastof(network_addr_buffer));
+			SetDParamStr(0, network_addr_buffer);
 			DrawString(r.left + WD_FRAMERECT_LEFT, r.right - WD_FRAMERECT_RIGHT, y, STR_NETWORK_SERVER_LIST_SERVER_ADDRESS); // server address
 			y += FONT_HEIGHT_NORMAL;
 
diff --git a/src/network/network_udp.cpp b/src/network/network_udp.cpp
index 03344fe57..0fd20b5f4 100644
--- a/src/network/network_udp.cpp
+++ b/src/network/network_udp.cpp
@@ -244,7 +244,7 @@ void ServerNetworkUDPSocketHandler::Receive_CLIENT_GET_NEWGRFS(Packet *p, Networ
 	uint8 in_reply_count = 0;
 	size_t packet_len = 0;
 
-	DEBUG(net, 6, "[udp] newgrf data request from %s", client_addr->GetAddressAsString());
+	DEBUG(net, 6, "[udp] newgrf data request from %s", client_addr->GetAddressAsString().c_str());
 
 	num_grfs = p->Recv_uint8 ();
 	if (num_grfs > NETWORK_MAX_GRF_COUNT) return;
@@ -307,7 +307,7 @@ void ClientNetworkUDPSocketHandler::Receive_SERVER_RESPONSE(Packet *p, NetworkAd
 	/* Just a fail-safe.. should never happen */
 	if (_network_udp_server) return;
 
-	DEBUG(net, 4, "[udp] server response from %s", client_addr->GetAddressAsString());
+	DEBUG(net, 4, "[udp] server response from %s", client_addr->GetAddressAsString().c_str());
 
 	/* Find next item */
 	item = NetworkGameListAddItem(*client_addr);
@@ -407,7 +407,7 @@ void ClientNetworkUDPSocketHandler::Receive_SERVER_NEWGRFS(Packet *p, NetworkAdd
 	uint8 num_grfs;
 	uint i;
 
-	DEBUG(net, 6, "[udp] newgrf data reply from %s", client_addr->GetAddressAsString());
+	DEBUG(net, 6, "[udp] newgrf data reply from %s", client_addr->GetAddressAsString().c_str());
 
 	num_grfs = p->Recv_uint8 ();
 	if (num_grfs > NETWORK_MAX_GRF_COUNT) return;
@@ -477,7 +477,7 @@ void NetworkUDPQueryMasterServer()
 
 	_udp_client_socket->SendPacket(&p, &out_addr, true);
 
-	DEBUG(net, 2, "[udp] master server queried at %s", out_addr.GetAddressAsString());
+	DEBUG(net, 2, "[udp] master server queried at %s", out_addr.GetAddressAsString().c_str());
 }
 
 /** Find all servers */
@@ -541,8 +541,8 @@ static void NetworkUDPAdvertiseThread()
 	if (_session_key == 0 && session_key_retries++ == 2) {
 		DEBUG(net, 0, "[udp] advertising to the master server is failing");
 		DEBUG(net, 0, "[udp]   we are not receiving the session key from the server");
-		DEBUG(net, 0, "[udp]   please allow udp packets from %s to you to be delivered", out_addr.GetAddressAsString(false));
-		DEBUG(net, 0, "[udp]   please allow udp packets from you to %s to be delivered", out_addr.GetAddressAsString(false));
+		DEBUG(net, 0, "[udp]   please allow udp packets from %s to you to be delivered", out_addr.GetAddressAsString(false).c_str());
+		DEBUG(net, 0, "[udp]   please allow udp packets from you to %s to be delivered", out_addr.GetAddressAsString(false).c_str());
 	}
 	if (_session_key != 0 && _network_advertise_retries == 0) {
 		DEBUG(net, 0, "[udp] advertising to the master server is failing");