diff --git a/src/gateway/ClientHandler.cpp b/src/gateway/ClientHandler.cpp index 80f73f38..7ee1d076 100644 --- a/src/gateway/ClientHandler.cpp +++ b/src/gateway/ClientHandler.cpp @@ -74,7 +74,7 @@ void ClientHandler::state_update(ClientState new_state) { enter_states[context_.state](context_); } -void ClientHandler::packet_skip(BinaryStream& stream) { +void ClientHandler::skip(BinaryStream& stream) { CLIENT_DEBUG_FILTER(logger_, LF_NETWORK, context_) << ClientState_to_string(context_.state) << " skipping " << protocol::to_string(opcode_) @@ -88,7 +88,7 @@ void ClientHandler::handle_ping(BinaryStream& stream) { protocol::CMSG_PING packet; - if(!packet_deserialise(packet, stream)) { + if(!deserialise(packet, stream)) { return; } @@ -121,10 +121,8 @@ void ClientHandler::stop_timer() { const std::string& ClientHandler::client_identify() const { if(context_.client_id) { if(client_id_ext_.empty()) { - client_id_ext_ = std::format("{} ({}, {}): ", - client_id_, - context_.client_id->username, - context_.client_id->id + client_id_ext_ = std::format( + "{} ({}, {}): ", client_id_, context_.client_id->username, context_.client_id->id ); } diff --git a/src/gateway/ClientHandler.h b/src/gateway/ClientHandler.h index 088e8ba0..ae1d6ec1 100644 --- a/src/gateway/ClientHandler.h +++ b/src/gateway/ClientHandler.h @@ -21,6 +21,7 @@ #include #include #include +#include namespace ember { @@ -47,8 +48,11 @@ class ClientHandler final { void close(); const std::string& client_identify() const; - bool packet_deserialise(auto& packet, BinaryStream& stream); - void packet_skip(BinaryStream& stream); + template + std::optional deserialise(BinaryStream& stream); + + bool deserialise(protocol::is_packet auto& packet, BinaryStream& stream); + void skip(BinaryStream& stream); void state_update(ClientState new_state); void handle_message(BinaryStream& stream); diff --git a/src/gateway/ClientHandler.inl b/src/gateway/ClientHandler.inl index 59038603..486398c9 100644 --- a/src/gateway/ClientHandler.inl +++ b/src/gateway/ClientHandler.inl @@ -9,58 +9,73 @@ #pragma once #include "ConnectionDefines.h" +#include #include +#include -bool ClientHandler::packet_deserialise(auto& packet, BinaryStream& stream) { - if(packet->read_from_stream(stream) != protocol::State::DONE) { - const auto state = stream.state(); - - /* - * READ_LIMIT_ERR: - * Deserialisation failed due to an attempt to read beyond the - * message boundary. This could be caused by an error in the message - * definition or a malicious client spoofing the size in the - * header. We can recover from this. - * - * BUFF_LIMIT_ERR: - * Deserialisation failed due to a buffer underrun - this should never - * happen and message framing has likely been lost if this ever - * occurs. Don't try to recover. - */ - if(state == BinaryStream::State::READ_LIMIT_ERR) { +bool ClientHandler::deserialise(protocol::is_packet auto& packet, BinaryStream& stream) { + if(packet->read_from_stream(stream) == protocol::State::DONE) { + + if(stream.read_limit() != stream.total_read()) { LOG_DEBUG_FILTER(logger_, LF_NETWORK) - << "Deserialisation of " - << protocol::to_string(packet.opcode) - << " failed, skipping any remaining data" << LOG_ASYNC; - - stream.skip(stream.read_limit() - stream.total_read()); - } else if(state == BinaryStream::State::BUFF_LIMIT_ERR) { - LOG_ERROR_FILTER(logger_, LF_NETWORK) - << "Message framing lost at " - << protocol::to_string(packet.opcode) - << " from " << client_identify() << LOG_ASYNC; - - close(); - } else { - LOG_ERROR_FILTER(logger_, LF_NETWORK) - << "Deserialisation failed but stream has not errored for " + << "Skipping superfluous stream data in message " << protocol::to_string(packet.opcode) << " from " << client_identify() << LOG_ASYNC; - close(); + stream.skip(stream.read_limit() - stream.total_read()); } - return false; - } + return true; + } + + /* + * READ_LIMIT_ERR: + * Deserialisation failed due to an attempt to read beyond the + * message boundary. This could be caused by an error in the message + * definition or a malicious client spoofing the size in the + * header. We can recover from this. + * + * BUFF_LIMIT_ERR: + * Deserialisation failed due to a buffer underrun - this should never + * happen and message framing has likely been lost if this ever + * occurs. Don't try to recover. + */ + const auto state = stream.state(); - if(stream.read_limit() != stream.total_read()) { + if(state == BinaryStream::State::READ_LIMIT_ERR) { LOG_DEBUG_FILTER(logger_, LF_NETWORK) - << "Skipping superfluous stream data in message " + << "Deserialisation of " << protocol::to_string(packet.opcode) - << " from " << client_identify() << LOG_ASYNC; + << " failed, skipping any remaining data" << LOG_ASYNC; stream.skip(stream.read_limit() - stream.total_read()); + } else if(state == BinaryStream::State::BUFF_LIMIT_ERR) { + LOG_ERROR_FILTER(logger_, LF_NETWORK) + << "Message framing lost at " + << protocol::to_string(packet.opcode) + << " from " << client_identify() << LOG_ASYNC; + + close(); + } else { + LOG_ERROR_FILTER(logger_, LF_NETWORK) + << "Deserialisation failed but stream has not errored for " + << protocol::to_string(packet.opcode) + << " from " << client_identify() << LOG_ASYNC; + + close(); } - return true; -}; \ No newline at end of file + return false; +} + +template +std::optional ClientHandler::deserialise(BinaryStream& stream) { + T packet; + const auto result = deserialise(packet, stream); + + if(result) { + return packet; + } else { + return std::nullopt; + } +} \ No newline at end of file diff --git a/src/gateway/packetlog/PacketLogger.h b/src/gateway/packetlog/PacketLogger.h index 962f8015..cb04dcde 100644 --- a/src/gateway/packetlog/PacketLogger.h +++ b/src/gateway/packetlog/PacketLogger.h @@ -9,7 +9,7 @@ #pragma once #include "PacketSink.h" -#include +#include #include #include #include diff --git a/src/gateway/states/Authentication.cpp b/src/gateway/states/Authentication.cpp index 568aefb3..ec4b7f3e 100644 --- a/src/gateway/states/Authentication.cpp +++ b/src/gateway/states/Authentication.cpp @@ -71,12 +71,14 @@ void handle_authentication(ClientContext& ctx) { auto& auth_ctx = std::get(ctx.state_ctx); - if(!ctx.handler->packet_deserialise(auth_ctx.packet, *ctx.stream)) { + if(!ctx.handler->deserialise(auth_ctx.packet, *ctx.stream)) { return; } - CLIENT_DEBUG_GLOB(ctx) << "Received session proof for " - << auth_ctx.packet->username << LOG_ASYNC; + CLIENT_DEBUG_GLOB(ctx) + << "Received session proof for " + << auth_ctx.packet->username + << LOG_ASYNC; if(auth_ctx.packet->build == 0) { // todo - check game build & server ID @@ -306,7 +308,7 @@ void handle_packet(ClientContext& ctx, protocol::ClientOpcode opcode) { handle_authentication(ctx); break; default: - ctx.handler->packet_skip(*ctx.stream); + ctx.handler->skip(*ctx.stream); } } diff --git a/src/gateway/states/CharacterList.cpp b/src/gateway/states/CharacterList.cpp index 9c11f39a..03ed53ae 100644 --- a/src/gateway/states/CharacterList.cpp +++ b/src/gateway/states/CharacterList.cpp @@ -73,7 +73,7 @@ void character_rename(ClientContext& ctx) { protocol::CMSG_CHAR_RENAME packet; - if(!ctx.handler->packet_deserialise(packet, *ctx.stream)) { + if(!ctx.handler->deserialise(packet, *ctx.stream)) { return; } @@ -146,7 +146,7 @@ void character_create(ClientContext& ctx) { protocol::CMSG_CHAR_CREATE packet; - if(!ctx.handler->packet_deserialise(packet, *ctx.stream)) { + if(!ctx.handler->deserialise(packet, *ctx.stream)) { return; } @@ -173,7 +173,7 @@ void character_delete(ClientContext& ctx) { protocol::CMSG_CHAR_DELETE packet; - if(!ctx.handler->packet_deserialise(packet, *ctx.stream)) { + if(!ctx.handler->deserialise(packet, *ctx.stream)) { return; } @@ -202,7 +202,7 @@ void player_login(ClientContext& ctx) { protocol::CMSG_PLAYER_LOGIN packet; - if(!ctx.handler->packet_deserialise(packet, *ctx.stream)) { + if(!ctx.handler->deserialise(packet, *ctx.stream)) { return; } @@ -242,7 +242,7 @@ void handle_packet(ClientContext& ctx, protocol::ClientOpcode opcode) { player_login(ctx); break; default: - ctx.handler->packet_skip(*ctx.stream); + ctx.handler->skip(*ctx.stream); } } diff --git a/src/gateway/states/SessionClose.cpp b/src/gateway/states/SessionClose.cpp index 2ffd4d8a..45fae084 100644 --- a/src/gateway/states/SessionClose.cpp +++ b/src/gateway/states/SessionClose.cpp @@ -16,7 +16,7 @@ void enter(ClientContext& ctx) { } void handle_packet(ClientContext& ctx, protocol::ClientOpcode opcode) { - ctx.handler->packet_skip(*ctx.stream); + ctx.handler->skip(*ctx.stream); } void handle_event(ClientContext& ctx, const Event* event) { diff --git a/src/libs/protocol/CMakeLists.txt b/src/libs/protocol/CMakeLists.txt index 560054c3..a631d89a 100644 --- a/src/libs/protocol/CMakeLists.txt +++ b/src/libs/protocol/CMakeLists.txt @@ -42,6 +42,7 @@ set(LIBRARY_SRC include/protocol/ResultCodes.h include/protocol/PacketHeaders.h include/protocol/SmartMessage.h + include/protocol/Concepts.h ) add_library(${LIBRARY_NAME} ${LIBRARY_SRC}) diff --git a/src/libs/protocol/include/protocol/Concepts.h b/src/libs/protocol/include/protocol/Concepts.h new file mode 100644 index 00000000..67cbf02e --- /dev/null +++ b/src/libs/protocol/include/protocol/Concepts.h @@ -0,0 +1,16 @@ +/* + * Copyright (c) 2024 Ember + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#pragma once + +namespace ember::protocol { + +template +concept is_packet = requires { typename T::packet_tag; }; + +} // protocol, ember \ No newline at end of file diff --git a/src/libs/protocol/include/protocol/Packet.h b/src/libs/protocol/include/protocol/Packet.h index ce69e8bc..b2511d64 100644 --- a/src/libs/protocol/include/protocol/Packet.h +++ b/src/libs/protocol/include/protocol/Packet.h @@ -9,6 +9,7 @@ #pragma once #include +#include #include #include @@ -19,19 +20,18 @@ enum class State { INITIAL, DONE, ERRORED }; -template +template struct Packet final { struct packet_tag_t{}; using packet_tag = packet_tag_t; - using OpcodeType = typename HeaderT::OpcodeType; - using SizeType = typename HeaderT::SizeType; - using PayloadType = Payload; + using OpcodeType = typename HeaderType::OpcodeType; + using SizeType = typename HeaderType::SizeType; static constexpr OpcodeType opcode = op_; - static constexpr std::size_t HEADER_WIRE_SIZE = HeaderT::WIRE_SIZE; + static constexpr std::size_t HEADER_WIRE_SIZE = HeaderType::WIRE_SIZE; - PayloadType payload; + Payload payload; State read_from_stream(auto& stream) { return payload.read_from_stream(stream); @@ -62,7 +62,4 @@ using ServerPacket = Packet; template using ClientPacket = Packet; -template -concept is_packet = requires { typename T::packet_tag; }; - } // protocol, ember \ No newline at end of file