diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index ffb968a0c..9d236e4d5 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -1981,14 +1981,7 @@ void cClientHandle::Tick(float a_Dt) m_BreakProgress += m_Player->GetMiningProgressPerTick(Block); } - try - { - ProcessProtocolIn(); - } - catch (const std::exception & Oops) - { - Kick(Oops.what()); - } + ProcessProtocolIn(); if (IsDestroyed()) { @@ -3171,7 +3164,7 @@ void cClientHandle::PacketBufferFull(void) { // Too much data in the incoming queue, the server is probably too busy, kick the client: LOGERROR("Too much data in queue for client \"%s\" @ %s, kicking them.", m_Username.c_str(), m_IPString.c_str()); - SendDisconnect("Server busy"); + SendDisconnect("The server is busy; please try again later."); } @@ -3249,10 +3242,19 @@ void cClientHandle::ProcessProtocolIn(void) std::swap(IncomingData, m_IncomingData); } - if (!IncomingData.empty()) + if (IncomingData.empty()) + { + return; + } + + try { m_Protocol.HandleIncomingData(*this, IncomingData); } + catch (const std::exception & Oops) + { + Kick(Oops.what()); + } } diff --git a/src/Protocol/ProtocolRecognizer.cpp b/src/Protocol/ProtocolRecognizer.cpp index dc6b93b01..3af4f9654 100644 --- a/src/Protocol/ProtocolRecognizer.cpp +++ b/src/Protocol/ProtocolRecognizer.cpp @@ -25,18 +25,6 @@ -struct UnsupportedButPingableProtocolException : public std::runtime_error -{ - explicit UnsupportedButPingableProtocolException() : - std::runtime_error("") - { - } -}; - - - - - struct TriedToJoinWithUnsupportedProtocolException : public std::runtime_error { explicit TriedToJoinWithUnsupportedProtocolException(const std::string & a_Message) : @@ -90,31 +78,44 @@ AString cMultiVersionProtocol::GetVersionTextFromInt(cProtocol::Version a_Protoc void cMultiVersionProtocol::HandleIncomingDataInRecognitionStage(cClientHandle & a_Client, std::string_view a_Data) { - // We read more than the handshake packet here, oh well. + // NOTE: If a new protocol is added or an old one is removed, adjust MCS_CLIENT_VERSIONS and MCS_PROTOCOL_VERSIONS macros in the header file + + /* Write all incoming data unmodified into m_Buffer. + Writing everything is always okay to do: + 1. We can be sure protocol encryption hasn't started yet since m_Protocol hasn't been called, hence no decryption needs to take place + 2. The extra data are processed at the end of this function */ if (!m_Buffer.Write(a_Data.data(), a_Data.size())) { - a_Client.Kick("Your client sent too much data; please try again later."); + a_Client.PacketBufferFull(); return; } - try + // TODO: recover from git history + // Unlengthed protocol, ... + + // Lengthed protocol, try if it has the entire initial handshake packet: + if ( + UInt32 PacketLen; + + // If not enough bytes for the packet length, keep waiting + !m_Buffer.ReadVarInt(PacketLen) || + + // If not enough bytes for the packet, keep waiting + // (More of a sanity check to make sure no one tries anything funny, since ReadXXX can wait for data themselves) + !m_Buffer.CanReadBytes(PacketLen) + ) { - // Note that a_Data is assigned to a subview containing the data to pass to m_Protocol or UnsupportedPing - - TryRecognizeProtocol(a_Client, a_Data); - if (m_Protocol == nullptr) - { - m_Buffer.ResetRead(); - return; - } - - // The protocol recogniser succesfully identified, switch mode: - HandleIncomingData = [this](cClientHandle &, const std::string_view a_In) - { - m_Protocol->DataReceived(m_Buffer, a_In.data(), a_In.size()); - }; + m_Buffer.ResetRead(); + return; } - catch (const UnsupportedButPingableProtocolException &) + + /* Figure out the client's version. + 1. m_Protocol != nullptr: the protocol is supported and we have a handler + 2. m_Protocol == nullptr: the protocol is unsupported, handling is a special case done by ourselves + 3. Exception: the data sent were garbage, the client handle deals with it by disconnecting */ + m_Protocol = TryRecognizeLengthedProtocol(a_Client, a_Data); + + if (m_Protocol == nullptr) { // Got a server list ping for an unrecognised version, // switch into responding to unknown protocols mode: @@ -123,9 +124,17 @@ void cMultiVersionProtocol::HandleIncomingDataInRecognitionStage(cClientHandle & HandleIncomingDataInOldPingResponseStage(a_Clyent, a_In); }; } + else + { + // The protocol recogniser succesfully identified, switch mode: + HandleIncomingData = [this](cClientHandle &, const std::string_view a_In) + { + m_Protocol->DataReceived(m_Buffer, a_In.data(), a_In.size()); + }; + } - // Explicitly process any remaining data with the new handler: - HandleIncomingData(a_Client, a_Data); + // Explicitly process any remaining data (already written to m_Buffer) with the new handler: + HandleIncomingData(a_Client, {}); } @@ -136,7 +145,7 @@ void cMultiVersionProtocol::HandleIncomingDataInOldPingResponseStage(cClientHand { if (!m_Buffer.Write(a_Data.data(), a_Data.size())) { - a_Client.Kick("Server list ping failed, too much data."); + a_Client.PacketBufferFull(); return; } @@ -170,7 +179,7 @@ void cMultiVersionProtocol::HandleIncomingDataInOldPingResponseStage(cClientHand } else { - a_Client.Kick("Server list ping failed, unrecognized packet."); + a_Client.PacketUnknown(PacketID); return; } @@ -206,34 +215,7 @@ void cMultiVersionProtocol::SendDisconnect(cClientHandle & a_Client, const AStri -void cMultiVersionProtocol::TryRecognizeProtocol(cClientHandle & a_Client, std::string_view & a_Data) -{ - // NOTE: If a new protocol is added or an old one is removed, adjust MCS_CLIENT_VERSIONS and MCS_PROTOCOL_VERSIONS macros in the header file - - // Lengthed protocol, try if it has the entire initial handshake packet: - UInt32 PacketLen; - if (!m_Buffer.ReadVarInt(PacketLen)) - { - // Not enough bytes for the packet length, keep waiting - return; - } - - if (!m_Buffer.CanReadBytes(PacketLen)) - { - // Not enough bytes for the packet, keep waiting - // More of a sanity check to make sure no one tries anything funny (since ReadXXX can wait for data themselves): - return; - } - - m_Protocol = TryRecognizeLengthedProtocol(a_Client, a_Data); - ASSERT(m_Protocol != nullptr); -} - - - - - -std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(cClientHandle & a_Client, std::string_view & a_Data) +std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(cClientHandle & a_Client, const std::string_view a_Data) { UInt32 PacketType; UInt32 ProtocolVersion; @@ -243,8 +225,8 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c if (!m_Buffer.ReadVarInt(PacketType) || (PacketType != 0x00)) { - // Not an initial handshake packet, we don't know how to talk to them - LOGINFO("Client \"%s\" uses an unsupported protocol (lengthed, initial packet %u)", + // Not an initial handshake packet, we don't know how to talk to them: + LOGD("Client \"%s\" uses an unsupported protocol (lengthed, initial packet %u)", a_Client.GetIPString().c_str(), PacketType ); @@ -277,15 +259,6 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c // TODO: this should be a protocol property, not ClientHandle: a_Client.SetProtocolVersion(ProtocolVersion); - // The protocol has just been recognized, advance data start - // to after the handshake and leave the rest to the protocol: - a_Data = a_Data.substr(m_Buffer.GetUsedSpace() - m_Buffer.GetReadableSpace()); - - // We read more than we can handle, purge the rest: - [[maybe_unused]] const bool Success = - m_Buffer.SkipRead(m_Buffer.GetReadableSpace()); - ASSERT(Success); - // All good, eat up the data: m_Buffer.CommitRead(); @@ -319,7 +292,8 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c ); } - throw UnsupportedButPingableProtocolException(); + // No cProtocol can handle the client: + return nullptr; } } } diff --git a/src/Protocol/ProtocolRecognizer.h b/src/Protocol/ProtocolRecognizer.h index b19adaefe..56d5645c0 100644 --- a/src/Protocol/ProtocolRecognizer.h +++ b/src/Protocol/ProtocolRecognizer.h @@ -47,22 +47,17 @@ public: private: - /** Handles data reception in a newly-created client handle that doesn't yet have known protocol. + /** Handles data reception in a newly-created client handle that doesn't yet have a known protocol. a_Data contains a view of data that were just received. - Calls TryRecognizeProtocol to populate m_Protocol, and transitions to another mode depending on success. */ + Tries to recognize a protocol, populate m_Protocol, and transitions to another mode depending on success. */ void HandleIncomingDataInRecognitionStage(cClientHandle & a_Client, std::string_view a_Data); /** Handles and responds to unsupported clients sending pings. */ - void HandleIncomingDataInOldPingResponseStage(cClientHandle & a_Client, const std::string_view a_Data); - - /* Tries to recognize a protocol based on a_Data and m_Buffer contents. - a_Data is replaced with a sub-view, with handshake packet removed. */ - void TryRecognizeProtocol(cClientHandle & a_Client, std::string_view & a_Data); + void HandleIncomingDataInOldPingResponseStage(cClientHandle & a_Client, std::string_view a_Data); /** Tries to recognize a protocol in the lengthed family (1.7+), based on m_Buffer. - The packet length and type have already been read, type is 0. Returns a cProtocol_XXX instance if recognized. */ - std::unique_ptr TryRecognizeLengthedProtocol(cClientHandle & a_Client, std::string_view & a_Data); + std::unique_ptr TryRecognizeLengthedProtocol(cClientHandle & a_Client, std::string_view a_Data); /** Sends one packet inside a cByteBuffer. This is used only when handling an outdated server ping. */ diff --git a/src/Protocol/Protocol_1_8.cpp b/src/Protocol/Protocol_1_8.cpp index be855678e..a4876c448 100644 --- a/src/Protocol/Protocol_1_8.cpp +++ b/src/Protocol/Protocol_1_8.cpp @@ -177,6 +177,13 @@ void cProtocol_1_8_0::DataReceived(cByteBuffer & a_Buffer, const char * a_Data, { if (m_IsEncrypted) { + // An artefact of the protocol recogniser, will be removed when decryption done in-place: + if (a_Size == 0) + { + AddReceivedData(a_Buffer, nullptr, 0); + return; + } + std::byte Decrypted[512]; while (a_Size > 0) { diff --git a/src/UI/SlotArea.cpp b/src/UI/SlotArea.cpp index dbdebd320..1de2a9c6a 100644 --- a/src/UI/SlotArea.cpp +++ b/src/UI/SlotArea.cpp @@ -2815,7 +2815,6 @@ void cSlotAreaHorse::Clicked(cPlayer & a_Player, int a_SlotNum, eClickAction a_C } default: break; } - } default: break; }