From 4c76239818f5159314f30883f98b977d30aaa26c Mon Sep 17 00:00:00 2001 From: rubenwardy Date: Fri, 22 Jan 2021 15:09:26 +0000 Subject: Remove dead code (#10845) --- src/network/networkexceptions.h | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'src/network') diff --git a/src/network/networkexceptions.h b/src/network/networkexceptions.h index f4913928c..58a3bb490 100644 --- a/src/network/networkexceptions.h +++ b/src/network/networkexceptions.h @@ -56,12 +56,6 @@ public: InvalidIncomingDataException(const char *s) : BaseException(s) {} }; -class InvalidOutgoingDataException : public BaseException -{ -public: - InvalidOutgoingDataException(const char *s) : BaseException(s) {} -}; - class NoIncomingDataException : public BaseException { public: @@ -103,4 +97,4 @@ class SendFailedException : public BaseException { public: SendFailedException(const std::string &s) : BaseException(s) {} -}; \ No newline at end of file +}; -- cgit v1.2.3 From 112a6adb10d3a5a2e55012a36580607d12ce9758 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sun, 31 Jan 2021 20:36:47 +0100 Subject: Cache client IP in RemoteClient so it can always be retrieved (#10887) specifically: after the peer has already disappeared --- src/clientiface.cpp | 3 -- src/clientiface.h | 15 ++++-- src/network/serverpackethandler.cpp | 11 ++-- src/script/lua_api/l_server.cpp | 101 ++++++++++++++---------------------- src/server.cpp | 38 ++++++-------- src/server.h | 15 ++++-- 6 files changed, 84 insertions(+), 99 deletions(-) (limited to 'src/network') 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 @@ -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(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(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(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); -- cgit v1.2.3 From 7ebd5da9cd4a227dcdc140a495f264a97277b3a3 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Tue, 2 Feb 2021 19:10:35 +0100 Subject: Server GotBlocks(): Lock clients to avoid multithreading issues --- src/network/serverpackethandler.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/network') diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index 882cf71c1..4d79f375c 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -438,18 +438,20 @@ void Server::handleCommand_GotBlocks(NetworkPacket* pkt) u8 count; *pkt >> count; - RemoteClient *client = getClient(pkt->getPeerId()); - if ((s16)pkt->getSize() < 1 + (int)count * 6) { throw con::InvalidIncomingDataException ("GOTBLOCKS length is too short"); } + m_clients.lock(); + RemoteClient *client = m_clients.lockedGetClientNoEx(pkt->getPeerId()); + for (u16 i = 0; i < count; i++) { v3s16 p; *pkt >> p; client->GotBlock(p); } + m_clients.unlock(); } void Server::process_PlayerPos(RemotePlayer *player, PlayerSAO *playersao, -- cgit v1.2.3 From c834d2ab25694ef2d67dc24f85f304269d202c8e Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 29 Jan 2021 14:03:27 +0100 Subject: Drop wide/narrow conversion functions The only valid usecase for these is interfacing with OS APIs that want a locale/OS-specific multibyte encoding. But they weren't used for that anywhere, instead UTF-8 is pretty much assumed when it comes to that. Since these are only a potential source of bugs and do not fulfil their purpose at all, drop them entirely. --- src/chat.cpp | 4 +-- src/client/client.cpp | 4 +-- src/client/keycode.cpp | 3 +- src/gui/guiConfirmRegistration.cpp | 3 +- src/network/serverpackethandler.cpp | 8 ++--- src/script/lua_api/l_server.cpp | 2 +- src/server.cpp | 61 ++++++++++++++----------------------- src/server.h | 8 ++--- src/unittest/test_utilities.cpp | 4 +-- src/util/string.cpp | 59 ++--------------------------------- src/util/string.h | 28 ----------------- 11 files changed, 41 insertions(+), 143 deletions(-) (limited to 'src/network') diff --git a/src/chat.cpp b/src/chat.cpp index 2f65e68b3..c9317a079 100644 --- a/src/chat.cpp +++ b/src/chat.cpp @@ -485,8 +485,8 @@ void ChatPrompt::nickCompletion(const std::list& names, bool backwa // find all names that start with the selected prefix std::vector completions; for (const std::string &name : names) { - if (str_starts_with(narrow_to_wide(name), prefix, true)) { - std::wstring completion = narrow_to_wide(name); + std::wstring completion = utf8_to_wide(name); + if (str_starts_with(completion, prefix, true)) { if (prefix_start == 0) completion += L": "; completions.push_back(completion); diff --git a/src/client/client.cpp b/src/client/client.cpp index 61888b913..ef4a3cdfc 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -1196,7 +1196,7 @@ void Client::sendChatMessage(const std::wstring &message) if (canSendChatMessage()) { u32 now = time(NULL); float time_passed = now - m_last_chat_message_sent; - m_last_chat_message_sent = time(NULL); + m_last_chat_message_sent = now; m_chat_message_allowance += time_passed * (CLIENT_CHAT_MESSAGE_LIMIT_PER_10S / 8.0f); if (m_chat_message_allowance > CLIENT_CHAT_MESSAGE_LIMIT_PER_10S) @@ -1832,7 +1832,7 @@ void Client::makeScreenshot() sstr << "Failed to save screenshot '" << filename << "'"; } pushToChatQueue(new ChatMessage(CHATMESSAGE_TYPE_SYSTEM, - narrow_to_wide(sstr.str()))); + utf8_to_wide(sstr.str()))); infostream << sstr.str() << std::endl; image->drop(); } diff --git a/src/client/keycode.cpp b/src/client/keycode.cpp index 6a0e9f569..ce5214f54 100644 --- a/src/client/keycode.cpp +++ b/src/client/keycode.cpp @@ -316,7 +316,8 @@ KeyPress::KeyPress(const char *name) int chars_read = mbtowc(&Char, name, 1); FATAL_ERROR_IF(chars_read != 1, "Unexpected multibyte character"); m_name = ""; - warningstream << "KeyPress: Unknown key '" << name << "', falling back to first char."; + warningstream << "KeyPress: Unknown key '" << name + << "', falling back to first char." << std::endl; } KeyPress::KeyPress(const irr::SEvent::SKeyInput &in, bool prefer_character) diff --git a/src/gui/guiConfirmRegistration.cpp b/src/gui/guiConfirmRegistration.cpp index 020a2796a..4a798c39b 100644 --- a/src/gui/guiConfirmRegistration.cpp +++ b/src/gui/guiConfirmRegistration.cpp @@ -192,8 +192,7 @@ void GUIConfirmRegistration::acceptInput() bool GUIConfirmRegistration::processInput() { - std::wstring m_password_ws = narrow_to_wide(m_password); - if (m_password_ws != m_pass_confirm) { + if (utf8_to_wide(m_password) != m_pass_confirm) { gui::IGUIElement *e = getElementFromId(ID_message); if (e) e->setVisible(true); diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index 4d79f375c..02af06abc 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -778,15 +778,13 @@ void Server::handleCommand_ChatMessage(NetworkPacket* pkt) return; } - // Get player name of this client std::string name = player->getName(); - std::wstring wname = narrow_to_wide(name); - std::wstring answer_to_sender = handleChat(name, wname, message, true, player); + std::wstring answer_to_sender = handleChat(name, message, true, player); if (!answer_to_sender.empty()) { // Send the answer to sender - SendChatMessage(peer_id, ChatMessage(CHATMESSAGE_TYPE_NORMAL, - answer_to_sender, wname)); + SendChatMessage(peer_id, ChatMessage(CHATMESSAGE_TYPE_SYSTEM, + answer_to_sender)); } } diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp index 78cf4b403..bf5292521 100644 --- a/src/script/lua_api/l_server.cpp +++ b/src/script/lua_api/l_server.cpp @@ -44,7 +44,7 @@ int ModApiServer::l_request_shutdown(lua_State *L) int ModApiServer::l_get_server_status(lua_State *L) { NO_MAP_LOCK_REQUIRED; - lua_pushstring(L, wide_to_narrow(getServer(L)->getStatusString()).c_str()); + lua_pushstring(L, getServer(L)->getStatusString().c_str()); return 1; } diff --git a/src/server.cpp b/src/server.cpp index 90496129e..907bc6d24 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2955,7 +2955,7 @@ void Server::handleChatInterfaceEvent(ChatEvent *evt) } } -std::wstring Server::handleChat(const std::string &name, const std::wstring &wname, +std::wstring Server::handleChat(const std::string &name, std::wstring wmessage, bool check_shout_priv, RemotePlayer *player) { // If something goes wrong, this player is to blame @@ -2993,7 +2993,7 @@ std::wstring Server::handleChat(const std::string &name, const std::wstring &wna auto message = trim(wide_to_utf8(wmessage)); if (message.find_first_of("\n\r") != std::wstring::npos) { - return L"New lines are not permitted in chat messages"; + return L"Newlines are not permitted in chat messages"; } // Run script hook, exit if script ate the chat message @@ -3014,10 +3014,10 @@ std::wstring Server::handleChat(const std::string &name, const std::wstring &wna the Cyrillic alphabet and some characters on older Android devices */ #ifdef __ANDROID__ - line += L"<" + wname + L"> " + wmessage; + line += L"<" + utf8_to_wide(name) + L"> " + wmessage; #else - line += narrow_to_wide(m_script->formatChatMessage(name, - wide_to_narrow(wmessage))); + line += utf8_to_wide(m_script->formatChatMessage(name, + wide_to_utf8(wmessage))); #endif } @@ -3030,35 +3030,23 @@ std::wstring Server::handleChat(const std::string &name, const std::wstring &wna /* Send the message to others */ - actionstream << "CHAT: " << wide_to_narrow(unescape_enriched(line)) << std::endl; + actionstream << "CHAT: " << wide_to_utf8(unescape_enriched(line)) << std::endl; - std::vector clients = m_clients.getClientIDs(); - - /* - Send the message back to the inital sender - if they are using protocol version >= 29 - */ - - session_t peer_id_to_avoid_sending = - (player ? player->getPeerId() : PEER_ID_INEXISTENT); + ChatMessage chatmsg(line); - if (player && player->protocol_version >= 29) - peer_id_to_avoid_sending = PEER_ID_INEXISTENT; + std::vector clients = m_clients.getClientIDs(); + for (u16 cid : clients) + SendChatMessage(cid, chatmsg); - for (u16 cid : clients) { - if (cid != peer_id_to_avoid_sending) - SendChatMessage(cid, ChatMessage(line)); - } return L""; } void Server::handleAdminChat(const ChatEventChat *evt) { std::string name = evt->nick; - std::wstring wname = utf8_to_wide(name); std::wstring wmessage = evt->evt_msg; - std::wstring answer = handleChat(name, wname, wmessage); + std::wstring answer = handleChat(name, wmessage); // If asked to send answer to sender if (!answer.empty()) { @@ -3095,46 +3083,43 @@ PlayerSAO *Server::getPlayerSAO(session_t peer_id) return player->getPlayerSAO(); } -std::wstring Server::getStatusString() +std::string Server::getStatusString() { - std::wostringstream os(std::ios_base::binary); - os << L"# Server: "; + std::ostringstream os(std::ios_base::binary); + os << "# Server: "; // Version - os << L"version=" << narrow_to_wide(g_version_string); + os << "version=" << g_version_string; // Uptime - os << L", uptime=" << m_uptime_counter->get(); + os << ", uptime=" << m_uptime_counter->get(); // Max lag estimate - os << L", max_lag=" << (m_env ? m_env->getMaxLagEstimate() : 0); + os << ", max_lag=" << (m_env ? m_env->getMaxLagEstimate() : 0); // Information about clients bool first = true; - os << L", clients={"; + os << ", clients={"; if (m_env) { std::vector clients = m_clients.getClientIDs(); for (session_t client_id : clients) { RemotePlayer *player = m_env->getPlayer(client_id); // Get name of player - std::wstring name = L"unknown"; - if (player) - name = narrow_to_wide(player->getName()); + const char *name = player ? player->getName() : ""; // Add name to information string if (!first) - os << L", "; + os << ", "; else first = false; - os << name; } } - os << L"}"; + os << "}"; if (m_env && !((ServerMap*)(&m_env->getMap()))->isSavingEnabled()) - os << std::endl << L"# Server: " << " WARNING: Map saving is disabled."; + os << std::endl << "# Server: " << " WARNING: Map saving is disabled."; if (!g_settings->get("motd").empty()) - os << std::endl << L"# Server: " << narrow_to_wide(g_settings->get("motd")); + os << std::endl << "# Server: " << g_settings->get("motd"); return os.str(); } diff --git a/src/server.h b/src/server.h index 5c143a657..0b4084aa9 100644 --- a/src/server.h +++ b/src/server.h @@ -219,7 +219,7 @@ public: void onMapEditEvent(const MapEditEvent &event); // Connection must be locked when called - std::wstring getStatusString(); + std::string getStatusString(); inline double getUptime() const { return m_uptime_counter->get(); } // read shutdown state @@ -495,10 +495,8 @@ private: void handleChatInterfaceEvent(ChatEvent *evt); // This returns the answer to the sender of wmessage, or "" if there is none - std::wstring handleChat(const std::string &name, const std::wstring &wname, - std::wstring wmessage_input, - bool check_shout_priv = false, - RemotePlayer *player = NULL); + std::wstring handleChat(const std::string &name, std::wstring wmessage_input, + bool check_shout_priv = false, RemotePlayer *player = nullptr); void handleAdminChat(const ChatEventChat *evt); // When called, connection mutex should be locked diff --git a/src/unittest/test_utilities.cpp b/src/unittest/test_utilities.cpp index 5559cdbf2..93ba3f844 100644 --- a/src/unittest/test_utilities.cpp +++ b/src/unittest/test_utilities.cpp @@ -247,8 +247,8 @@ void TestUtilities::testStartsWith() void TestUtilities::testStrEqual() { - UASSERT(str_equal(narrow_to_wide("abc"), narrow_to_wide("abc"))); - UASSERT(str_equal(narrow_to_wide("ABC"), narrow_to_wide("abc"), true)); + UASSERT(str_equal(utf8_to_wide("abc"), utf8_to_wide("abc"))); + UASSERT(str_equal(utf8_to_wide("ABC"), utf8_to_wide("abc"), true)); } diff --git a/src/util/string.cpp b/src/util/string.cpp index 7e6d6d3b3..611ad35cb 100644 --- a/src/util/string.cpp +++ b/src/util/string.cpp @@ -175,62 +175,6 @@ wchar_t *utf8_to_wide_c(const char *str) return ret_c; } -// You must free the returned string! -// The returned string is allocated using new -wchar_t *narrow_to_wide_c(const char *str) -{ - wchar_t *nstr = nullptr; -#if defined(_WIN32) - int nResult = MultiByteToWideChar(CP_UTF8, 0, (LPCSTR) str, -1, 0, 0); - if (nResult == 0) { - errorstream<<"gettext: MultiByteToWideChar returned null"< wcs(wcl + 1); - size_t len = mbstowcs(*wcs, mbs.c_str(), wcl); - if (len == (size_t)(-1)) - return L""; - wcs[len] = 0; - return *wcs; -#endif -} - - -std::string wide_to_narrow(const std::wstring &wcs) -{ -#ifdef __ANDROID__ - return wide_to_utf8(wcs); -#else - size_t mbl = wcs.size() * 4; - SharedBuffer mbs(mbl+1); - size_t len = wcstombs(*mbs, wcs.c_str(), mbl); - if (len == (size_t)(-1)) - return "Character conversion failed!"; - - mbs[len] = 0; - return *mbs; -#endif -} - std::string urlencode(const std::string &str) { @@ -757,7 +701,8 @@ void translate_string(const std::wstring &s, Translations *translations, } else { // This is an escape sequence *inside* the template string to translate itself. // This should not happen, show an error message. - errorstream << "Ignoring escape sequence '" << wide_to_narrow(escape_sequence) << "' in translation" << std::endl; + errorstream << "Ignoring escape sequence '" + << wide_to_utf8(escape_sequence) << "' in translation" << std::endl; } } diff --git a/src/util/string.h b/src/util/string.h index ec14e9a2d..d4afcaec8 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -73,16 +73,6 @@ std::string wide_to_utf8(const std::wstring &input); // The returned string is allocated using new[] wchar_t *utf8_to_wide_c(const char *str); -// NEVER use those two functions unless you have a VERY GOOD reason to -// they just convert between wide and multibyte encoding -// multibyte encoding depends on current locale, this is no good, especially on Windows - -// You must free the returned string! -// The returned string is allocated using new -wchar_t *narrow_to_wide_c(const char *str); -std::wstring narrow_to_wide(const std::string &mbs); -std::string wide_to_narrow(const std::wstring &wcs); - std::string urlencode(const std::string &str); std::string urldecode(const std::string &str); u32 readFlagString(std::string str, const FlagDesc *flagdesc, u32 *flagmask); @@ -355,11 +345,6 @@ inline s32 mystoi(const std::string &str, s32 min, s32 max) return i; } - -// MSVC2010 includes it's own versions of these -//#if !defined(_MSC_VER) || _MSC_VER < 1600 - - /** * Returns a 32-bit value reprensented by the string \p str (decimal). * @see atoi(3) for further limitations @@ -369,17 +354,6 @@ inline s32 mystoi(const std::string &str) return atoi(str.c_str()); } - -/** - * Returns s 32-bit value represented by the wide string \p str (decimal). - * @see atoi(3) for further limitations - */ -inline s32 mystoi(const std::wstring &str) -{ - return mystoi(wide_to_narrow(str)); -} - - /** * Returns a float reprensented by the string \p str (decimal). * @see atof(3) @@ -389,8 +363,6 @@ inline float mystof(const std::string &str) return atof(str.c_str()); } -//#endif - #define stoi mystoi #define stof mystof -- cgit v1.2.3 From 674d67f312c815e7f10dc00705e352bc392fc2af Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 29 Jan 2021 15:24:07 +0100 Subject: Encode high codepoints as surrogates to safely transport wchar_t over network fixes #7643 --- src/network/networkpacket.cpp | 49 ++++++++++++++++++++++++++++++------- src/network/networkpacket.h | 2 +- src/network/serverpackethandler.cpp | 15 +----------- src/server.cpp | 3 ++- src/unittest/test_connection.cpp | 35 ++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 25 deletions(-) (limited to 'src/network') diff --git a/src/network/networkpacket.cpp b/src/network/networkpacket.cpp index 6d0abb12c..a71e26572 100644 --- a/src/network/networkpacket.cpp +++ b/src/network/networkpacket.cpp @@ -50,7 +50,7 @@ void NetworkPacket::checkReadOffset(u32 from_offset, u32 field_size) } } -void NetworkPacket::putRawPacket(u8 *data, u32 datasize, session_t peer_id) +void NetworkPacket::putRawPacket(const u8 *data, u32 datasize, session_t peer_id) { // If a m_command is already set, we are rewriting on same packet // This is not permitted @@ -145,6 +145,8 @@ void NetworkPacket::putLongString(const std::string &src) putRawString(src.c_str(), msgsize); } +static constexpr bool NEED_SURROGATE_CODING = sizeof(wchar_t) > 2; + NetworkPacket& NetworkPacket::operator>>(std::wstring& dst) { checkReadOffset(m_read_offset, 2); @@ -160,9 +162,16 @@ NetworkPacket& NetworkPacket::operator>>(std::wstring& dst) checkReadOffset(m_read_offset, strLen * 2); dst.reserve(strLen); - for(u16 i=0; i= 0xD800 && c < 0xDC00 && i+1 < strLen) { + i++; + m_read_offset += sizeof(u16); + + wchar_t c2 = readU16(&m_data[m_read_offset]); + c = 0x10000 + ( ((c & 0x3ff) << 10) | (c2 & 0x3ff) ); + } + dst.push_back(c); m_read_offset += sizeof(u16); } @@ -175,15 +184,37 @@ NetworkPacket& NetworkPacket::operator<<(const std::wstring &src) throw PacketError("String too long"); } - u16 msgsize = src.size(); + if (!NEED_SURROGATE_CODING || src.size() == 0) { + *this << static_cast(src.size()); + for (u16 i = 0; i < src.size(); i++) + *this << static_cast(src[i]); - *this << msgsize; + return *this; + } - // Write string - for (u16 i=0; i(0xfff0); + + for (u16 i = 0; i < src.size(); i++) { + wchar_t c = src[i]; + if (c > 0xffff) { + // Encode high code-points as surrogate pairs + u32 n = c - 0x10000; + *this << static_cast(0xD800 | (n >> 10)) + << static_cast(0xDC00 | (n & 0x3ff)); + written += 2; + } else { + *this << static_cast(c); + written++; + } } + if (written > WIDE_STRING_MAX_LEN) + throw PacketError("String too long"); + writeU16(&m_data[len_offset], written); + return *this; } diff --git a/src/network/networkpacket.h b/src/network/networkpacket.h index e77bfb744..c7ff03b8e 100644 --- a/src/network/networkpacket.h +++ b/src/network/networkpacket.h @@ -34,7 +34,7 @@ public: ~NetworkPacket(); - void putRawPacket(u8 *data, u32 datasize, session_t peer_id); + void putRawPacket(const u8 *data, u32 datasize, session_t peer_id); void clear(); // Getters diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index 02af06abc..270b8e01f 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -752,21 +752,8 @@ void Server::handleCommand_InventoryAction(NetworkPacket* pkt) void Server::handleCommand_ChatMessage(NetworkPacket* pkt) { - /* - u16 command - u16 length - wstring message - */ - u16 len; - *pkt >> len; - std::wstring message; - for (u16 i = 0; i < len; i++) { - u16 tmp_wchar; - *pkt >> tmp_wchar; - - message += (wchar_t)tmp_wchar; - } + *pkt >> message; session_t peer_id = pkt->getPeerId(); RemotePlayer *player = m_env->getPlayer(peer_id); diff --git a/src/server.cpp b/src/server.cpp index 907bc6d24..b815558fb 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1482,7 +1482,8 @@ void Server::SendChatMessage(session_t peer_id, const ChatMessage &message) NetworkPacket pkt(TOCLIENT_CHAT_MESSAGE, 0, peer_id); u8 version = 1; u8 type = message.type; - pkt << version << type << std::wstring(L"") << message.message << (u64)message.timestamp; + pkt << version << type << message.sender << message.message + << static_cast(message.timestamp); if (peer_id != PEER_ID_INEXISTENT) { RemotePlayer *player = m_env->getPlayer(peer_id); diff --git a/src/unittest/test_connection.cpp b/src/unittest/test_connection.cpp index c5e4085e1..c3aacc536 100644 --- a/src/unittest/test_connection.cpp +++ b/src/unittest/test_connection.cpp @@ -39,6 +39,7 @@ public: void runTests(IGameDef *gamedef); + void testNetworkPacketSerialize(); void testHelpers(); void testConnectSendReceive(); }; @@ -47,6 +48,7 @@ static TestConnection g_test_instance; void TestConnection::runTests(IGameDef *gamedef) { + TEST(testNetworkPacketSerialize); TEST(testHelpers); TEST(testConnectSendReceive); } @@ -78,6 +80,39 @@ struct Handler : public con::PeerHandler const char *name; }; +void TestConnection::testNetworkPacketSerialize() +{ + const static u8 expected[] = { + 0x00, 0x7b, + 0x00, 0x02, 0xd8, 0x42, 0xdf, 0x9a + }; + + if (sizeof(wchar_t) == 2) + warningstream << __func__ << " may fail on this platform." << std::endl; + + { + NetworkPacket pkt(123, 0); + + // serializing wide strings should do surrogate encoding, we test that here + pkt << std::wstring(L"\U00020b9a"); + + SharedBuffer buf = pkt.oldForgePacket(); + UASSERTEQ(int, buf.getSize(), sizeof(expected)); + UASSERT(!memcmp(expected, &buf[0], buf.getSize())); + } + + { + NetworkPacket pkt; + pkt.putRawPacket(expected, sizeof(expected), 0); + + // same for decoding + std::wstring pkt_s; + pkt >> pkt_s; + + UASSERT(pkt_s == L"\U00020b9a"); + } +} + void TestConnection::testHelpers() { // Some constants for testing -- cgit v1.2.3