From d374a7bca7338f1f12c15dbc0736becb0fcf2b48 Mon Sep 17 00:00:00 2001 From: Lucio Rossi Date: Thu, 25 Sep 2025 18:27:56 +0200 Subject: [PATCH 1/4] feat: catching PARSING_ERR for return/error type mismatch --- src/client.h | 3 +++ src/decoder.h | 12 ++++++++++-- src/error.h | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/client.h b/src/client.h index 8d95748..06cd8ae 100644 --- a/src/client.h +++ b/src/client.h @@ -68,6 +68,9 @@ class RPCClient { lastError.code = tmp_error.code; lastError.traceback = tmp_error.traceback; return true; + } else if (tmp_error.code == PARSING_ERR) { // catches the parsing error + lastError.code = tmp_error.code; + lastError.traceback = tmp_error.traceback; } return false; } diff --git a/src/decoder.h b/src/decoder.h index f3ab3d3..7df0e9a 100644 --- a/src/decoder.h +++ b/src/decoder.h @@ -76,9 +76,17 @@ class RpcDecoder { MsgPack::object::nil_t nil; if (unpacker.unpackable(nil)){ // No error - if (!unpacker.deserialize(nil, result)) return false; + if (!unpacker.deserialize(nil, result)) { + error.code = PARSING_ERR; + error.traceback = "Result not parsable (check type)"; + return false; + } } else { // RPC returned an error - if (!unpacker.deserialize(error, nil)) return false; + if (!unpacker.deserialize(error, nil)) { + error.code = PARSING_ERR; + error.traceback = "RPC Error not parsable (check type)"; + return false; + } } reset_packet(); diff --git a/src/error.h b/src/error.h index 925b30a..bece5d8 100644 --- a/src/error.h +++ b/src/error.h @@ -17,6 +17,7 @@ #include "MsgPack.h" #define NO_ERR 0x00 +#define PARSING_ERR 0xFC #define MALFORMED_CALL_ERR 0xFD #define FUNCTION_NOT_FOUND_ERR 0xFE #define GENERIC_ERR 0xFF From a593af1994ad514fb0c23bfd2f5f52155465928b Mon Sep 17 00:00:00 2001 From: Lucio Rossi Date: Thu, 25 Sep 2025 20:28:38 +0200 Subject: [PATCH 2/4] feat: decoder handles RPC response PARSING_ERR discarding it feat: RpcDecoder.discard --- src/decoder.h | 50 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/decoder.h b/src/decoder.h index 7df0e9a..5515042 100644 --- a/src/decoder.h +++ b/src/decoder.h @@ -62,16 +62,33 @@ class RpcDecoder { MsgPack::Unpacker unpacker; unpacker.clear(); - size_t res_size = get_packet_size(); - if (!unpacker.feed(_raw_buffer, res_size)) return false; + if (!unpacker.feed(_raw_buffer, _packet_size)) return false; MsgPack::arr_size_t resp_size; int resp_type; uint32_t resp_id; - if (!unpacker.deserialize(resp_size, resp_type, resp_id)) return false; - if (resp_size.size() != RESPONSE_SIZE) return false; - if (resp_type != RESP_MSG) return false; + if (!unpacker.deserialize(resp_size, resp_type, resp_id)) { + error.code = PARSING_ERR; + error.traceback = "Non parsable RPC response headers (check type)"; + discard(); + return false; + } + if (resp_size.size() != RESPONSE_SIZE) { + error.code = PARSING_ERR; + error.traceback = "Wrong RPC response size"; + discard(); + return false; + } + if (resp_type != RESP_MSG) { + // This should never happen + error.code = GENERIC_ERR; + error.traceback = "Unexpected response type"; + discard(); + return false; + } + + // DO NOT MODIFY THIS if resp_id is not what is expected then the RPC response belongs to s/o else if (resp_id != msg_id) return false; MsgPack::object::nil_t nil; @@ -79,18 +96,20 @@ class RpcDecoder { if (!unpacker.deserialize(nil, result)) { error.code = PARSING_ERR; error.traceback = "Result not parsable (check type)"; + discard(); return false; } } else { // RPC returned an error if (!unpacker.deserialize(error, nil)) { error.code = PARSING_ERR; error.traceback = "RPC Error not parsable (check type)"; + discard(); return false; } } + consume(_packet_size); reset_packet(); - consume(res_size); return true; } @@ -111,8 +130,7 @@ class RpcDecoder { unpacker.clear(); if (!unpacker.feed(_raw_buffer, _packet_size)) { // feed should not fail at this point - consume(_packet_size); - reset_packet(); + discard(); return ""; }; @@ -121,27 +139,23 @@ class RpcDecoder { MsgPack::arr_size_t req_size; if (!unpacker.deserialize(req_size, msg_type)) { - consume(_packet_size); - reset_packet(); + discard(); return ""; // Header not unpackable } if (msg_type == CALL_MSG && req_size.size() == REQUEST_SIZE) { uint32_t msg_id; if (!unpacker.deserialize(msg_id, method)) { - consume(_packet_size); - reset_packet(); + discard(); return ""; // Method not unpackable } } else if (msg_type == NOTIFY_MSG && req_size.size() == NOTIFY_SIZE) { if (!unpacker.deserialize(method)) { - consume(_packet_size); - reset_packet(); + discard(); return ""; // Method not unpackable } } else { - consume(_packet_size); - reset_packet(); + discard(); return ""; // Invalid request size/type } @@ -260,6 +274,10 @@ class RpcDecoder { return consume(packet_size); } + void discard() { + consume(_packet_size); + reset_packet(); + } void reset_packet() { _packet_type = NO_MSG; From e2bfcd7557c7f007aa6abd0a2891641d131daeb2 Mon Sep 17 00:00:00 2001 From: Lucio Rossi Date: Thu, 25 Sep 2025 22:10:56 +0200 Subject: [PATCH 3/4] feat: client.get_response fills in a per-call error param fixing lastError mismatch feat: RpcError .copy --- src/client.h | 16 +++++++--------- src/error.h | 5 +++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/client.h b/src/client.h index 06cd8ae..c133343 100644 --- a/src/client.h +++ b/src/client.h @@ -41,7 +41,8 @@ class RPCClient { } // blocking call - while (!get_response(msg_id_wait, result)){ + RpcError tmp_error; + while (!get_response(msg_id_wait, result, tmp_error)) { //delay(1); } @@ -60,17 +61,14 @@ class RPCClient { } template - bool get_response(const uint32_t wait_id, RType& result) { - RpcError tmp_error; + bool get_response(const uint32_t wait_id, RType& result, RpcError& error) { decoder->decode(); - if (decoder->get_response(wait_id, result, tmp_error)) { - lastError.code = tmp_error.code; - lastError.traceback = tmp_error.traceback; + if (decoder->get_response(wait_id, result, error)) { + lastError.copy(error); return true; - } else if (tmp_error.code == PARSING_ERR) { // catches the parsing error - lastError.code = tmp_error.code; - lastError.traceback = tmp_error.traceback; + } else if (error.code == PARSING_ERR) { // catches the parsing error + lastError.copy(error); } return false; } diff --git a/src/error.h b/src/error.h index bece5d8..c98cb7d 100644 --- a/src/error.h +++ b/src/error.h @@ -35,6 +35,11 @@ struct RpcError { RpcError(const int c, MsgPack::str_t tb) : code(c), traceback(std::move(tb)) {} + void copy(const RpcError& err) { + code = err.code; + traceback = err.traceback; + } + MSGPACK_DEFINE(code, traceback); // -> [code, traceback] }; From 7340e3b90ad40818185b5d8871e652f60f8826a1 Mon Sep 17 00:00:00 2001 From: Lucio Rossi Date: Fri, 26 Sep 2025 09:09:52 +0200 Subject: [PATCH 4/4] feat: get_discard_packages (client/server/decoder) mod: recoded decoder.get_response logic discarding broken packages with right ID --- src/client.h | 4 ++-- src/decoder.h | 41 +++++++++++++++++++++++------------------ src/server.h | 2 ++ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/client.h b/src/client.h index c133343..04f26da 100644 --- a/src/client.h +++ b/src/client.h @@ -67,12 +67,12 @@ class RPCClient { if (decoder->get_response(wait_id, result, error)) { lastError.copy(error); return true; - } else if (error.code == PARSING_ERR) { // catches the parsing error - lastError.copy(error); } return false; } + uint32_t get_discarded_packets() const {return decoder->get_discarded_packets();} + }; #endif //RPCLITE_CLIENT_H diff --git a/src/decoder.h b/src/decoder.h index 5515042..883abeb 100644 --- a/src/decoder.h +++ b/src/decoder.h @@ -68,50 +68,48 @@ class RpcDecoder { int resp_type; uint32_t resp_id; - if (!unpacker.deserialize(resp_size, resp_type, resp_id)) { + if (!unpacker.deserialize(resp_size, resp_type, resp_id)) return false; + + // ReSharper disable once CppDFAUnreachableCode + if (resp_id != msg_id) return false; + + // msg_id OK packet will be consumed. + if (resp_type != RESP_MSG) { + // This should never happen error.code = PARSING_ERR; - error.traceback = "Non parsable RPC response headers (check type)"; + error.traceback = "Unexpected response type"; discard(); - return false; + return true; } + if (resp_size.size() != RESPONSE_SIZE) { - error.code = PARSING_ERR; - error.traceback = "Wrong RPC response size"; - discard(); - return false; - } - if (resp_type != RESP_MSG) { // This should never happen - error.code = GENERIC_ERR; - error.traceback = "Unexpected response type"; + error.code = PARSING_ERR; + error.traceback = "Unexpected RPC response size"; discard(); - return false; + return true; } - // DO NOT MODIFY THIS if resp_id is not what is expected then the RPC response belongs to s/o else - if (resp_id != msg_id) return false; - MsgPack::object::nil_t nil; if (unpacker.unpackable(nil)){ // No error if (!unpacker.deserialize(nil, result)) { error.code = PARSING_ERR; error.traceback = "Result not parsable (check type)"; discard(); - return false; + return true; } } else { // RPC returned an error if (!unpacker.deserialize(error, nil)) { error.code = PARSING_ERR; error.traceback = "RPC Error not parsable (check type)"; discard(); - return false; + return true; } } consume(_packet_size); reset_packet(); return true; - } bool send_response(const MsgPack::Packer& packer) const { @@ -143,6 +141,7 @@ class RpcDecoder { return ""; // Header not unpackable } + // ReSharper disable once CppDFAUnreachableCode if (msg_type == CALL_MSG && req_size.size() == REQUEST_SIZE) { uint32_t msg_id; if (!unpacker.deserialize(msg_id, method)) { @@ -205,11 +204,13 @@ class RpcDecoder { if (type != CALL_MSG && type != RESP_MSG && type != NOTIFY_MSG) { consume(bytes_checked); + _discarded_packets++; break; // Not a valid RPC type (could be type=WRONG_MSG) } if ((type == CALL_MSG && container_size != REQUEST_SIZE) || (type == RESP_MSG && container_size != RESPONSE_SIZE) || (type == NOTIFY_MSG && container_size != NOTIFY_SIZE)) { consume(bytes_checked); + _discarded_packets++; break; // Not a valid RPC format } @@ -232,6 +233,8 @@ class RpcDecoder { size_t size() const {return _bytes_stored;} + uint32_t get_discarded_packets() const {return _discarded_packets;} + friend class DecoderTester; private: @@ -241,6 +244,7 @@ class RpcDecoder { int _packet_type = NO_MSG; size_t _packet_size = 0; uint32_t _msg_id = 0; + uint32_t _discarded_packets = 0; bool buffer_full() const { return _bytes_stored == BufferSize; } @@ -277,6 +281,7 @@ class RpcDecoder { void discard() { consume(_packet_size); reset_packet(); + _discarded_packets++; } void reset_packet() { diff --git a/src/server.h b/src/server.h index 3200b40..f3a5803 100644 --- a/src/server.h +++ b/src/server.h @@ -87,6 +87,8 @@ class RPCServer { } + uint32_t get_discarded_packets() const {return decoder->get_discarded_packets();} + private: RpcDecoder<>* decoder = nullptr; RpcFunctionDispatcher dispatcher{};