From d288e43da89f31ddc83e2d317d1f96087bfee116 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Mon, 23 Sep 2024 15:17:28 +0200 Subject: [PATCH 1/4] Fail on message sending if too big CURA-11103 Previously we would correctly send the message, but be unable to parse it at receiving time. Now we apply a more fail-fast strategy. --- src/Socket_p.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Socket_p.h b/src/Socket_p.h index e3eb2c42..2b9f2270 100644 --- a/src/Socket_p.h +++ b/src/Socket_p.h @@ -43,7 +43,7 @@ #define VERSION_MINOR 0 #define ARCUS_SIGNATURE 0x2BAD -#define SIG(n) (((n)&0xffff0000) >> 16) +#define SIG(n) (((n) & 0xffff0000) >> 16) #define SOCKET_CLOSE 0xf0f0f0f0 @@ -360,7 +360,11 @@ void Socket::Private::sendMessage(const MessagePtr& message) } std::string data = message->SerializeAsString(); - if (platform_socket.writeBytes(data.size(), data.data()) == -1) + if (data.size() > message_size_maximum) + { + error(ErrorCode::SendFailedError, "Message is too big to be sent"); + } + else if (platform_socket.writeBytes(data.size(), data.data()) == -1) { error(ErrorCode::SendFailedError, "Could not send message data"); } @@ -571,4 +575,4 @@ void Socket::Private::checkConnectionState() } } // namespace Arcus -#endif // SOCKET_P_H \ No newline at end of file +#endif // SOCKET_P_H From a0b95fdc1a37d0c7ee084f0a79647547513c09be Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Mon, 23 Sep 2024 15:28:18 +0200 Subject: [PATCH 2/4] Do not send any data if message is too big CURA-11103 --- src/Socket_p.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Socket_p.h b/src/Socket_p.h index 2b9f2270..28e6a2fe 100644 --- a/src/Socket_p.h +++ b/src/Socket_p.h @@ -338,37 +338,41 @@ void Socket::Private::run() // Send a message to the connected socket. void Socket::Private::sendMessage(const MessagePtr& message) { - uint32_t header = (ARCUS_SIGNATURE << 16) | (VERSION_MAJOR << 8) | (VERSION_MINOR); + const uint32_t message_size = message->ByteSizeLong(); + if (message_size > message_size_maximum) + { + error(ErrorCode::SendFailedError, "Message is too big to be sent"); + return; + } + + const uint32_t header = (ARCUS_SIGNATURE << 16) | (VERSION_MAJOR << 8) | (VERSION_MINOR); if (platform_socket.writeUInt32(header) == -1) { error(ErrorCode::SendFailedError, "Could not send message header"); return; } - uint32_t message_size = message->ByteSizeLong(); if (platform_socket.writeUInt32(message_size) == -1) { error(ErrorCode::SendFailedError, "Could not send message size"); return; } - uint32_t type_id = message_types.getMessageTypeId(message); + const uint32_t type_id = message_types.getMessageTypeId(message); if (platform_socket.writeUInt32(type_id) == -1) { error(ErrorCode::SendFailedError, "Could not send message type"); return; } - std::string data = message->SerializeAsString(); - if (data.size() > message_size_maximum) - { - error(ErrorCode::SendFailedError, "Message is too big to be sent"); - } - else if (platform_socket.writeBytes(data.size(), data.data()) == -1) + const std::string data = message->SerializeAsString(); + if (platform_socket.writeBytes(data.size(), data.data()) == -1) { error(ErrorCode::SendFailedError, "Could not send message data"); + return; } - DEBUG(std::string("Sending message of type ") + std::to_string(type_id) + " and size " + std::to_string(message_size)); + + DEBUG(std::string("Sent message of type ") + std::to_string(type_id) + " and size " + std::to_string(message_size)); } // Handle receiving data until we have a proper message. From 0e24426d54cc479c077b94076217973bdd8dce5f Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Mon, 23 Sep 2024 15:45:06 +0200 Subject: [PATCH 3/4] Set specific error code for message too big error CURA-11103 --- conandata.yml | 2 +- include/Arcus/Error.h | 1 + src/Socket_p.h | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/conandata.yml b/conandata.yml index 6feb46e5..4cf74beb 100644 --- a/conandata.yml +++ b/conandata.yml @@ -1 +1 @@ -version: "5.4.0-alpha.0" \ No newline at end of file +version: "5.4.1" diff --git a/include/Arcus/Error.h b/include/Arcus/Error.h index f3efa57a..0260887d 100644 --- a/include/Arcus/Error.h +++ b/include/Arcus/Error.h @@ -19,6 +19,7 @@ enum class ErrorCode BindFailedError, ///< Bind to IP and port failed. AcceptFailedError, ///< Accepting an incoming connection failed. SendFailedError, ///< Sending a message failed. + MessageTooBigError, ///< Sending a message failed because it was too big. ReceiveFailedError, ///< Receiving a message failed. UnknownMessageTypeError, ///< Received a message with an unknown message type. ParseFailedError, ///< Parsing the received message failed. diff --git a/src/Socket_p.h b/src/Socket_p.h index 28e6a2fe..60a46b97 100644 --- a/src/Socket_p.h +++ b/src/Socket_p.h @@ -341,7 +341,7 @@ void Socket::Private::sendMessage(const MessagePtr& message) const uint32_t message_size = message->ByteSizeLong(); if (message_size > message_size_maximum) { - error(ErrorCode::SendFailedError, "Message is too big to be sent"); + error(ErrorCode::MessageTooBigError, "Message is too big to be sent"); return; } From 0b4ae857f2bea31cfab79a3ef8e5caaf5ec7a3d1 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Tue, 24 Sep 2024 08:50:46 +0200 Subject: [PATCH 4/4] Add error code and raise message too big error at sending time CURA-11103 --- include/Arcus/Error.h | 2 ++ include/Arcus/Socket.h | 2 +- src/Socket.cpp | 12 ++++++++++-- src/Socket_p.h | 8 +------- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/Arcus/Error.h b/include/Arcus/Error.h index 0260887d..a35968ba 100644 --- a/include/Arcus/Error.h +++ b/include/Arcus/Error.h @@ -28,6 +28,8 @@ enum class ErrorCode InvalidStateError, ///< Socket is in an invalid state. InvalidMessageError, ///< Message being handled is a nullptr or otherwise invalid. Debug, // Debug messages + + // When changing this list, don't forget to apply the same changes on pyArcus/python/Error.sip }; /** diff --git a/include/Arcus/Socket.h b/include/Arcus/Socket.h index 8c6b3d61..dcb83994 100644 --- a/include/Arcus/Socket.h +++ b/include/Arcus/Socket.h @@ -114,7 +114,7 @@ class Socket /** * Send a message across the socket. */ - virtual void sendMessage(MessagePtr message); + virtual bool sendMessage(MessagePtr message); /** * Remove and return the next pending message from the queue with condition blocking. diff --git a/src/Socket.cpp b/src/Socket.cpp index 158c68ea..69487cac 100644 --- a/src/Socket.cpp +++ b/src/Socket.cpp @@ -203,21 +203,29 @@ void Socket::close() delete d->thread; d->thread = nullptr; } + // Notify all in case of closing because the waiting threads need to know // that this socket has been closed and they should not wait any more. d->message_received_condition_variable.notify_all(); } -void Socket::sendMessage(MessagePtr message) +bool Socket::sendMessage(MessagePtr message) { if (! message) { d->error(ErrorCode::InvalidMessageError, "Message cannot be nullptr"); - return; + return false; + } + + if (message->ByteSizeLong() > Private::message_size_maximum) + { + d->error(ErrorCode::MessageTooBigError, "Message is too big to be sent"); + return false; } std::lock_guard lock(d->sendQueueMutex); d->sendQueue.push_back(message); + return true; } MessagePtr Socket::takeNextMessage() diff --git a/src/Socket_p.h b/src/Socket_p.h index 60a46b97..33f5873f 100644 --- a/src/Socket_p.h +++ b/src/Socket_p.h @@ -338,13 +338,6 @@ void Socket::Private::run() // Send a message to the connected socket. void Socket::Private::sendMessage(const MessagePtr& message) { - const uint32_t message_size = message->ByteSizeLong(); - if (message_size > message_size_maximum) - { - error(ErrorCode::MessageTooBigError, "Message is too big to be sent"); - return; - } - const uint32_t header = (ARCUS_SIGNATURE << 16) | (VERSION_MAJOR << 8) | (VERSION_MINOR); if (platform_socket.writeUInt32(header) == -1) { @@ -352,6 +345,7 @@ void Socket::Private::sendMessage(const MessagePtr& message) return; } + const uint32_t message_size = message->ByteSizeLong(); if (platform_socket.writeUInt32(message_size) == -1) { error(ErrorCode::SendFailedError, "Could not send message size");