# HG changeset patch # User Patric Stout # Date 2021-08-23 17:37:51 # Node ID d1582e09c6edaf1ccfd1cdc5da8269f8c3a19a14 # Parent 9fafc6f16577f4327b6dee1e4e52ffe1fd8e23eb Fix #9501: [Network] crash when more than one game-info query was pending (#9502) diff --git a/src/network/CMakeLists.txt b/src/network/CMakeLists.txt --- a/src/network/CMakeLists.txt +++ b/src/network/CMakeLists.txt @@ -22,6 +22,8 @@ add_files( network_gui.cpp network_gui.h network_internal.h + network_query.cpp + network_query.h network_server.cpp network_server.h network_stun.cpp diff --git a/src/network/network.cpp b/src/network/network.cpp --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -14,6 +14,7 @@ #include "../date_func.h" #include "network_admin.h" #include "network_client.h" +#include "network_query.h" #include "network_server.h" #include "network_content.h" #include "network_udp.h" @@ -638,9 +639,7 @@ public: void OnConnect(SOCKET s) override { - _networking = true; - new ClientNetworkGameSocketHandler(s, this->connection_string); - MyClient::SendInformationQuery(); + QueryNetworkGameSocketHandler::QueryServer(s, this->connection_string); } }; @@ -652,8 +651,6 @@ void NetworkQueryServer(const std::strin { if (!_network_available) return; - NetworkInitialize(); - new TCPQueryConnecter(connection_string); } @@ -1020,6 +1017,7 @@ void NetworkBackgroundLoop() _network_coordinator_client.SendReceive(); TCPConnecter::CheckCallbacks(); NetworkHTTPSocketHandler::HTTPReceive(); + QueryNetworkGameSocketHandler::SendReceive(); NetworkBackgroundUDPLoop(); } diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp --- a/src/network/network_client.cpp +++ b/src/network/network_client.cpp @@ -23,7 +23,6 @@ #include "../gfx_func.h" #include "../error.h" #include "../rev.h" -#include "core/game_info.h" #include "network.h" #include "network_base.h" #include "network_client.h" @@ -328,17 +327,6 @@ static_assert(NETWORK_SERVER_ID_LENGTH = * DEF_CLIENT_SEND_COMMAND has no parameters ************/ -/** - * Query the server for server information. - */ -NetworkRecvStatus ClientNetworkGameSocketHandler::SendInformationQuery() -{ - my_client->status = STATUS_GAME_INFO; - my_client->SendPacket(new Packet(PACKET_CLIENT_GAME_INFO)); - - return NETWORK_RECV_STATUS_OKAY; -} - /** Tell the server we would like to join. */ NetworkRecvStatus ClientNetworkGameSocketHandler::SendJoin() { @@ -557,26 +545,6 @@ NetworkRecvStatus ClientNetworkGameSocke return NETWORK_RECV_STATUS_SERVER_BANNED; } -NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_GAME_INFO(Packet *p) -{ - if (this->status != STATUS_GAME_INFO) return NETWORK_RECV_STATUS_MALFORMED_PACKET; - - NetworkGameList *item = NetworkGameListAddItem(this->connection_string); - - /* Clear any existing GRFConfig chain. */ - ClearGRFConfigList(&item->info.grfconfig); - /* Retrieve the NetworkGameInfo from the packet. */ - DeserializeNetworkGameInfo(p, &item->info); - /* Check for compatability with the client. */ - CheckGameCompatibility(item->info); - /* Ensure we consider the server online. */ - item->online = true; - - UpdateNetworkGameWindow(); - - return NETWORK_RECV_STATUS_CLOSE_QUERY; -} - /* This packet contains info about the client (playas and name) * as client we save this in NetworkClientInfo, linked via 'client_id' * which is always an unique number on a server. */ @@ -665,15 +633,6 @@ NetworkRecvStatus ClientNetworkGameSocke NetworkErrorCode error = (NetworkErrorCode)p->Recv_uint8(); - /* If we query a server that is 1.11.1 or older, we get an - * NETWORK_ERROR_NOT_EXPECTED on requesting the game info. Show a special - * error popup in that case. - */ - if (error == NETWORK_ERROR_NOT_EXPECTED && this->status == STATUS_GAME_INFO) { - ShowErrorMessage(STR_NETWORK_ERROR_SERVER_TOO_OLD, INVALID_STRING_ID, WL_CRITICAL); - return NETWORK_RECV_STATUS_CLOSE_QUERY; - } - StringID err = STR_NETWORK_ERROR_LOSTCONNECTION; if (error < (ptrdiff_t)lengthof(network_error_strings)) err = network_error_strings[error]; /* In case of kicking a client, we assume there is a kick message in the packet if we can read one byte */ diff --git a/src/network/network_client.h b/src/network/network_client.h --- a/src/network/network_client.h +++ b/src/network/network_client.h @@ -22,7 +22,6 @@ private: /** Status of the connection with the server. */ enum ServerStatus { STATUS_INACTIVE, ///< The client is not connected nor active. - STATUS_GAME_INFO, ///< We are trying to get the game information. STATUS_JOIN, ///< We are trying to join a server. STATUS_NEWGRFS_CHECK, ///< Last action was checking NewGRFs. STATUS_AUTH_GAME, ///< Last action was requesting game (server) password. @@ -44,7 +43,6 @@ protected: NetworkRecvStatus Receive_SERVER_FULL(Packet *p) override; NetworkRecvStatus Receive_SERVER_BANNED(Packet *p) override; NetworkRecvStatus Receive_SERVER_ERROR(Packet *p) override; - NetworkRecvStatus Receive_SERVER_GAME_INFO(Packet *p) override; NetworkRecvStatus Receive_SERVER_CLIENT_INFO(Packet *p) override; NetworkRecvStatus Receive_SERVER_NEED_GAME_PASSWORD(Packet *p) override; NetworkRecvStatus Receive_SERVER_NEED_COMPANY_PASSWORD(Packet *p) override; @@ -80,8 +78,6 @@ public: NetworkRecvStatus CloseConnection(NetworkRecvStatus status) override; void ClientError(NetworkRecvStatus res); - static NetworkRecvStatus SendInformationQuery(); - static NetworkRecvStatus SendJoin(); static NetworkRecvStatus SendCommand(const CommandPacket *cp); static NetworkRecvStatus SendError(NetworkErrorCode errorno); diff --git a/src/network/network_query.cpp b/src/network/network_query.cpp new file mode 100644 --- /dev/null +++ b/src/network/network_query.cpp @@ -0,0 +1,145 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** @file network_query.cpp Query part of the network protocol. */ + +#include "../stdafx.h" +#include "core/game_info.h" +#include "network_query.h" +#include "network_gamelist.h" +#include "../error.h" + +#include "table/strings.h" + +#include "../safeguards.h" + +std::vector> QueryNetworkGameSocketHandler::queries = {}; + +NetworkRecvStatus QueryNetworkGameSocketHandler::CloseConnection(NetworkRecvStatus status) +{ + assert(status != NETWORK_RECV_STATUS_OKAY); + assert(this->sock != INVALID_SOCKET); + + return status; +} + +/** + * Check the connection's state, i.e. is the connection still up? + */ +bool QueryNetworkGameSocketHandler::CheckConnection() +{ + std::chrono::steady_clock::duration lag = std::chrono::steady_clock::now() - this->last_packet; + + /* If there was no response in 5 seconds, terminate the query. */ + if (lag > std::chrono::seconds(5)) { + this->CloseConnection(NETWORK_RECV_STATUS_CONNECTION_LOST); + return false; + } + + return true; +} + +/** + * Check whether we received/can send some data from/to the server and + * when that's the case handle it appropriately. + * @return true when everything went okay. + */ +bool QueryNetworkGameSocketHandler::Receive() +{ + if (this->CanSendReceive()) { + NetworkRecvStatus res = this->ReceivePackets(); + if (res != NETWORK_RECV_STATUS_OKAY) { + this->CloseConnection(res); + return false; + } + } + return true; +} + +/** Send the packets of this socket handler. */ +void QueryNetworkGameSocketHandler::Send() +{ + this->SendPackets(); +} + +/** + * Query the server for server information. + */ +NetworkRecvStatus QueryNetworkGameSocketHandler::SendGameInfo() +{ + this->SendPacket(new Packet(PACKET_CLIENT_GAME_INFO)); + return NETWORK_RECV_STATUS_OKAY; +} + +NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_FULL(Packet *p) +{ + /* We try to join a server which is full */ + ShowErrorMessage(STR_NETWORK_ERROR_SERVER_FULL, INVALID_STRING_ID, WL_CRITICAL); + return NETWORK_RECV_STATUS_SERVER_FULL; +} + +NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_BANNED(Packet *p) +{ + /* We try to join a server where we are banned */ + ShowErrorMessage(STR_NETWORK_ERROR_SERVER_BANNED, INVALID_STRING_ID, WL_CRITICAL); + return NETWORK_RECV_STATUS_SERVER_BANNED; +} + +NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_GAME_INFO(Packet *p) +{ + NetworkGameList *item = NetworkGameListAddItem(this->connection_string); + + /* Clear any existing GRFConfig chain. */ + ClearGRFConfigList(&item->info.grfconfig); + /* Retrieve the NetworkGameInfo from the packet. */ + DeserializeNetworkGameInfo(p, &item->info); + /* Check for compatability with the client. */ + CheckGameCompatibility(item->info); + /* Ensure we consider the server online. */ + item->online = true; + + UpdateNetworkGameWindow(); + + return NETWORK_RECV_STATUS_CLOSE_QUERY; +} + +NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_ERROR(Packet *p) +{ + NetworkErrorCode error = (NetworkErrorCode)p->Recv_uint8(); + + /* If we query a server that is 1.11.1 or older, we get an + * NETWORK_ERROR_NOT_EXPECTED on requesting the game info. Show a special + * error popup in that case. + */ + if (error == NETWORK_ERROR_NOT_EXPECTED) { + ShowErrorMessage(STR_NETWORK_ERROR_SERVER_TOO_OLD, INVALID_STRING_ID, WL_CRITICAL); + return NETWORK_RECV_STATUS_CLOSE_QUERY; + } + + ShowErrorMessage(STR_NETWORK_ERROR_LOSTCONNECTION, INVALID_STRING_ID, WL_CRITICAL); + return NETWORK_RECV_STATUS_SERVER_ERROR; +} + +/** + * Check if any query needs to send or receive. + */ +/* static */ void QueryNetworkGameSocketHandler::SendReceive() +{ + for (auto it = QueryNetworkGameSocketHandler::queries.begin(); it != QueryNetworkGameSocketHandler::queries.end(); /* nothing */) { + if (!(*it)->Receive()) { + it = QueryNetworkGameSocketHandler::queries.erase(it); + } else if (!(*it)->CheckConnection()) { + it = QueryNetworkGameSocketHandler::queries.erase(it); + } else { + it++; + } + } + + for (auto &query : QueryNetworkGameSocketHandler::queries) { + query->Send(); + } +} diff --git a/src/network/network_query.h b/src/network/network_query.h new file mode 100644 --- /dev/null +++ b/src/network/network_query.h @@ -0,0 +1,59 @@ +/* + * This file is part of OpenTTD. + * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. + * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . + */ + +/** @file network_query.h Query part of the network protocol. */ + +#ifndef NETWORK_QUERY_H +#define NETWORK_QUERY_H + +#include "network_internal.h" + +/** Class for handling the client side of quering a game server. */ +class QueryNetworkGameSocketHandler : public ZeroedMemoryAllocator, public NetworkGameSocketHandler { +private: + static std::vector> queries; ///< Pending queries. + std::string connection_string; ///< Address we are connected to. + +protected: + NetworkRecvStatus Receive_SERVER_FULL(Packet *p) override; + NetworkRecvStatus Receive_SERVER_BANNED(Packet *p) override; + NetworkRecvStatus Receive_SERVER_ERROR(Packet *p) override; + NetworkRecvStatus Receive_SERVER_GAME_INFO(Packet *p) override; + + NetworkRecvStatus SendGameInfo(); + + bool CheckConnection(); + void Send(); + bool Receive(); + +public: + /** + * Create a new socket for the client side of quering game server. + * @param s The socket to connect with. + * @param connection_string The connection string of the server. + */ + QueryNetworkGameSocketHandler(SOCKET s, const std::string &connection_string) : NetworkGameSocketHandler(s), connection_string(connection_string) {} + + /** + * Start to query a server based on an open socket. + * @param s The socket to connect with. + * @param connection_string The connection string of the server. + */ + static void QueryServer(SOCKET s, const std::string &connection_string) + { + auto query = std::make_unique(s, connection_string); + query->SendGameInfo(); + + QueryNetworkGameSocketHandler::queries.push_back(std::move(query)); + } + + static void SendReceive(); + + NetworkRecvStatus CloseConnection(NetworkRecvStatus status) override; +}; + +#endif /* NETWORK_QUERY_H */