From 42a9ff8f2992aca01001e0507dd575f1396d9379 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Fri, 4 Aug 2023 10:50:48 +0200 Subject: [PATCH 01/10] Refs #19347: Fix encapsulation format in WLP. Improve WLP checks Signed-off-by: Eduardo Ponz --- .../rtps/builtin/liveliness/WLPListener.h | 10 ++++ src/cpp/rtps/builtin/liveliness/WLP.cpp | 4 +- .../rtps/builtin/liveliness/WLPListener.cpp | 56 ++++++++++++++----- 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/include/fastdds/rtps/builtin/liveliness/WLPListener.h b/include/fastdds/rtps/builtin/liveliness/WLPListener.h index a1859a202b7..f9e54a6d5d0 100644 --- a/include/fastdds/rtps/builtin/liveliness/WLPListener.h +++ b/include/fastdds/rtps/builtin/liveliness/WLPListener.h @@ -82,6 +82,16 @@ class WLPListener: public ReaderListener { */ bool computeKey(CacheChange_t* change); + /** + * @brief Check that the ParticipantMessageData kind is a valid one for WLP + * + * @param A pointer to the first octet of the kind array. The function assumes 4 elements in the + * array. + * + * @return True if the kind corresponds with one for WLP, false otherwise. + */ + bool is_wlp_kind(octet* kind); + //! A pointer to the writer liveliness protocol WLP* mp_WLP; diff --git a/src/cpp/rtps/builtin/liveliness/WLP.cpp b/src/cpp/rtps/builtin/liveliness/WLP.cpp index 6c1b49f6979..775c5ebd41e 100644 --- a/src/cpp/rtps/builtin/liveliness/WLP.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLP.cpp @@ -900,9 +900,9 @@ bool WLP::send_liveliness_message( if (change != nullptr) { - change->serializedPayload.encapsulation = (uint16_t)PL_DEFAULT_ENCAPSULATION; + change->serializedPayload.encapsulation = (uint16_t)DEFAULT_ENCAPSULATION; change->serializedPayload.data[0] = 0; - change->serializedPayload.data[1] = PL_DEFAULT_ENCAPSULATION; + change->serializedPayload.data[1] = DEFAULT_ENCAPSULATION; change->serializedPayload.data[2] = 0; change->serializedPayload.data[3] = 0; diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index fc13dcabca1..ad3b9379a48 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -74,22 +74,28 @@ void WLPListener::onNewCacheChangeAdded( break; } } - if (change->serializedPayload.length > 0) + + // Data should have at least 4 bytes of representation header, 12 of GuidPrefix, and 4 of kind. + if (change->serializedPayload.length >= 20) { - if (PL_CDR_BE == change->serializedPayload.data[1]) + // Encapsulation in the second byte of the representation header. + change->serializedPayload.encapsulation = (uint16_t)change->serializedPayload.data[1]; + + // Extract GuidPrefix + memcpy(guidP.value, change->serializedPayload.data + 4, 12); + + // Extract liveliness kind + if (is_wlp_kind(&change->serializedPayload.data[16])) { - change->serializedPayload.encapsulation = (uint16_t)PL_CDR_BE; + // Adjust and cast to LivelinessQosPolicyKind enum, where AUTOMATIC_LIVELINESS_QOS == 0 + livelinessKind = (LivelinessQosPolicyKind)(change->serializedPayload.data[19] - 0x01); } else { - change->serializedPayload.encapsulation = (uint16_t)PL_CDR_LE; - } - - for (size_t i = 0; i < 12; ++i) - { - guidP.value[i] = change->serializedPayload.data[i + 4]; + logInfo(RTPS_LIVELINESS,"Ignoring not WLP ParticipantDataMessage"); + history->remove_change(change); + return; } - livelinessKind = (LivelinessQosPolicyKind)(change->serializedPayload.data[19] - 0x01); } else @@ -99,6 +105,8 @@ void WLPListener::onNewCacheChangeAdded( &guidP, &livelinessKind)) { + logInfo(RTPS_LIVELINESS,"Ignoring not WLP ParticipantDataMessage"); + history->remove_change(change); return; } } @@ -130,12 +138,17 @@ bool WLPListener::separateKey( GuidPrefix_t* guidP, LivelinessQosPolicyKind* liveliness) { - for (uint8_t i = 0; i < 12; ++i) + bool ret = false; + if (is_wlp_kind(&key.value[12])) { - guidP->value[i] = key.value[i]; + // Extract GuidPrefix + memcpy(guidP->value, key.value, 12); + + // Extract liveliness kind + *liveliness = (LivelinessQosPolicyKind)key.value[15]; + ret = true; } - *liveliness = (LivelinessQosPolicyKind)key.value[15]; - return true; + return ret; } bool WLPListener::computeKey( @@ -154,6 +167,21 @@ bool WLPListener::computeKey( return true; } +bool WLPListener::is_wlp_kind(octet* kind) +{ + /* + * From RTPS 2.5 9.6.3.1, the ParticipantMessageData kinds for WLP are: + * - PARTICIPANT_MESSAGE_DATA_KIND_AUTOMATIC_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x01} + * - PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x02} + */ + bool is_wlp = true; + is_wlp &= kind[0] == 0; + is_wlp &= kind[1] == 0; + is_wlp &= kind[2] == 0; + is_wlp &= kind[3] == 0x01 || kind[3] == 0x02; + return is_wlp; +} + } /* namespace rtps */ } /* namespace eprosima */ } // namespace eprosima From 7c351959884f2551bbe466be568014d81170bc5c Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Fri, 4 Aug 2023 11:11:37 +0200 Subject: [PATCH 02/10] Refs #19347: Correctly set CDR endianess for BE Signed-off-by: Eduardo Ponz --- include/fastdds/rtps/common/SerializedPayload.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fastdds/rtps/common/SerializedPayload.h b/include/fastdds/rtps/common/SerializedPayload.h index 17e2aeea6c2..4422c43efe5 100644 --- a/include/fastdds/rtps/common/SerializedPayload.h +++ b/include/fastdds/rtps/common/SerializedPayload.h @@ -42,7 +42,7 @@ namespace rtps { #define PL_CDR_LE 0x0003 #if FASTDDS_IS_BIG_ENDIAN_TARGET -#define DEFAULT_ENCAPSULATION CDR_LE +#define DEFAULT_ENCAPSULATION CDR_BE #define PL_DEFAULT_ENCAPSULATION PL_CDR_BE #else #define DEFAULT_ENCAPSULATION CDR_LE From fbddf914d5bd4252ea3d03aa4301118f8bf18c04 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Fri, 4 Aug 2023 11:19:21 +0200 Subject: [PATCH 03/10] Refs #19347: Uncrustify Signed-off-by: Eduardo Ponz --- .../rtps/builtin/liveliness/WLPListener.h | 38 ++++++++++--------- .../rtps/builtin/liveliness/WLPListener.cpp | 15 ++++---- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/include/fastdds/rtps/builtin/liveliness/WLPListener.h b/include/fastdds/rtps/builtin/liveliness/WLPListener.h index f9e54a6d5d0..99907848792 100644 --- a/include/fastdds/rtps/builtin/liveliness/WLPListener.h +++ b/include/fastdds/rtps/builtin/liveliness/WLPListener.h @@ -28,7 +28,7 @@ #include namespace eprosima { -namespace fastrtps{ +namespace fastrtps { namespace rtps { class WLP; @@ -39,14 +39,16 @@ struct CacheChange_t; * Class WLPListener that receives the liveliness messages asserting the liveliness of remote endpoints. * @ingroup LIVELINESS_MODULE */ -class WLPListener: public ReaderListener { +class WLPListener : public ReaderListener +{ public: /** * @brief Constructor * @param pwlp Pointer to the writer liveliness protocol */ - WLPListener(WLP* pwlp); + WLPListener( + WLP* pwlp); /** * @brief Destructor @@ -60,27 +62,28 @@ class WLPListener: public ReaderListener { */ void onNewCacheChangeAdded( RTPSReader* reader, - const CacheChange_t* const change) override; + const CacheChange_t* const change) override; private: /** - * Separate the Key between the GuidPrefix_t and the liveliness Kind - * @param key InstanceHandle_t to separate. - * @param guidP GuidPrefix_t pointer to store the info. - * @param liveliness Liveliness Kind Pointer. - * @return True if correctly separated. - */ + * Separate the Key between the GuidPrefix_t and the liveliness Kind + * @param key InstanceHandle_t to separate. + * @param guidP GuidPrefix_t pointer to store the info. + * @param liveliness Liveliness Kind Pointer. + * @return True if correctly separated. + */ bool separateKey( InstanceHandle_t& key, GuidPrefix_t* guidP, LivelinessQosPolicyKind* liveliness); /** - * Compute the key from a CacheChange_t - * @param change - */ - bool computeKey(CacheChange_t* change); + * Compute the key from a CacheChange_t + * @param change + */ + bool computeKey( + CacheChange_t* change); /** * @brief Check that the ParticipantMessageData kind is a valid one for WLP @@ -90,7 +93,8 @@ class WLPListener: public ReaderListener { * * @return True if the kind corresponds with one for WLP, false otherwise. */ - bool is_wlp_kind(octet* kind); + bool is_wlp_kind( + octet* kind); //! A pointer to the writer liveliness protocol WLP* mp_WLP; @@ -99,6 +103,6 @@ class WLPListener: public ReaderListener { } /* namespace rtps */ } /* namespace eprosima */ -} -#endif +} // namespace eprosima +#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #endif /* _FASTDDS_RTPS_WLPLISTENER_H_ */ diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index ad3b9379a48..686f9f99f99 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -92,7 +92,7 @@ void WLPListener::onNewCacheChangeAdded( } else { - logInfo(RTPS_LIVELINESS,"Ignoring not WLP ParticipantDataMessage"); + logInfo(RTPS_LIVELINESS, "Ignoring not WLP ParticipantDataMessage"); history->remove_change(change); return; } @@ -105,7 +105,7 @@ void WLPListener::onNewCacheChangeAdded( &guidP, &livelinessKind)) { - logInfo(RTPS_LIVELINESS,"Ignoring not WLP ParticipantDataMessage"); + logInfo(RTPS_LIVELINESS, "Ignoring not WLP ParticipantDataMessage"); history->remove_change(change); return; } @@ -167,13 +167,14 @@ bool WLPListener::computeKey( return true; } -bool WLPListener::is_wlp_kind(octet* kind) +bool WLPListener::is_wlp_kind( + octet* kind) { /* - * From RTPS 2.5 9.6.3.1, the ParticipantMessageData kinds for WLP are: - * - PARTICIPANT_MESSAGE_DATA_KIND_AUTOMATIC_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x01} - * - PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x02} - */ + * From RTPS 2.5 9.6.3.1, the ParticipantMessageData kinds for WLP are: + * - PARTICIPANT_MESSAGE_DATA_KIND_AUTOMATIC_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x01} + * - PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x02} + */ bool is_wlp = true; is_wlp &= kind[0] == 0; is_wlp &= kind[1] == 0; From 8cbdaab90bed46f1368a668965864106ffda09d8 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Fri, 4 Aug 2023 11:30:28 +0200 Subject: [PATCH 04/10] Refs #19347: Fix doxygen Signed-off-by: Eduardo Ponz --- include/fastdds/rtps/builtin/liveliness/WLPListener.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/fastdds/rtps/builtin/liveliness/WLPListener.h b/include/fastdds/rtps/builtin/liveliness/WLPListener.h index 99907848792..baea926bc63 100644 --- a/include/fastdds/rtps/builtin/liveliness/WLPListener.h +++ b/include/fastdds/rtps/builtin/liveliness/WLPListener.h @@ -88,8 +88,8 @@ class WLPListener : public ReaderListener /** * @brief Check that the ParticipantMessageData kind is a valid one for WLP * - * @param A pointer to the first octet of the kind array. The function assumes 4 elements in the - * array. + * @param kind A pointer to the first octet of the kind array. The function assumes 4 elements + * in the array. * * @return True if the kind corresponds with one for WLP, false otherwise. */ From f33debf434db07a619461b237cde8f53e30726fe Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Sat, 5 Aug 2023 09:35:56 +0200 Subject: [PATCH 05/10] Refs #19347: Apply suggestions Signed-off-by: Eduardo Ponz --- .../rtps/builtin/liveliness/WLPListener.h | 10 ++- include/fastdds/rtps/messages/CDRMessage.h | 11 +++ include/fastdds/rtps/messages/CDRMessage.hpp | 13 ++++ .../rtps/builtin/liveliness/WLPListener.cpp | 76 ++++++++++++------- 4 files changed, 79 insertions(+), 31 deletions(-) diff --git a/include/fastdds/rtps/builtin/liveliness/WLPListener.h b/include/fastdds/rtps/builtin/liveliness/WLPListener.h index baea926bc63..b17e4afdab4 100644 --- a/include/fastdds/rtps/builtin/liveliness/WLPListener.h +++ b/include/fastdds/rtps/builtin/liveliness/WLPListener.h @@ -86,15 +86,17 @@ class WLPListener : public ReaderListener CacheChange_t* change); /** - * @brief Check that the ParticipantMessageData kind is a valid one for WLP + * @brief Check that the ParticipantMessageData kind is a valid one for WLP and extract the liveliness kind. * - * @param kind A pointer to the first octet of the kind array. The function assumes 4 elements + * @param[in] serialized_kind A pointer to the first octet of the kind array. The function assumes 4 elements * in the array. + * @param[out] liveliness_kind A reference to the LivelinessQosPolicyKind. * * @return True if the kind corresponds with one for WLP, false otherwise. */ - bool is_wlp_kind( - octet* kind); + bool get_wlp_kind( + octet* serialized_kind, + LivelinessQosPolicyKind& liveliness_kind); //! A pointer to the writer liveliness protocol WLP* mp_WLP; diff --git a/include/fastdds/rtps/messages/CDRMessage.h b/include/fastdds/rtps/messages/CDRMessage.h index 7856906f361..20e55b9550f 100644 --- a/include/fastdds/rtps/messages/CDRMessage.h +++ b/include/fastdds/rtps/messages/CDRMessage.h @@ -312,6 +312,17 @@ inline bool addParticipantGenericMessage( ///@} +/** + * @brief Skip bytes in serialized buffer + * + * @param msg The CDR message + * @param length The number of bytes to skip + * @return true if skipped, false otherwise + */ +inline bool skip( + CDRMessage_t* msg, + uint32_t length); + } /* namespace CDRMessage */ } /* namespace rtps */ diff --git a/include/fastdds/rtps/messages/CDRMessage.hpp b/include/fastdds/rtps/messages/CDRMessage.hpp index a39156855d4..d5d67018cd2 100644 --- a/include/fastdds/rtps/messages/CDRMessage.hpp +++ b/include/fastdds/rtps/messages/CDRMessage.hpp @@ -1301,6 +1301,19 @@ inline bool CDRMessage::readParticipantGenericMessage( return true; } +inline bool CDRMessage::skip( + CDRMessage_t* msg, + uint32_t length) +{ + // Validate input + bool ret = (msg != nullptr) && (msg->pos + length <= msg->length); + if (ret) + { + msg->pos += length; + } + return ret; +} + } // namespace rtps } // namespace fastrtps } // namespace eprosima diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index 686f9f99f99..aa7cac6fc99 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -75,28 +75,46 @@ void WLPListener::onNewCacheChangeAdded( } } - // Data should have at least 4 bytes of representation header, 12 of GuidPrefix, and 4 of kind. - if (change->serializedPayload.length >= 20) + // Serialized payload should have at least 4 bytes of representation header, 12 of GuidPrefix, + // 4 of kind, and 4 of length. + const uint32_t min_serialized_length = 4 + 12 + 4 + 4; + if (change->serializedPayload.length >= min_serialized_length) { - // Encapsulation in the second byte of the representation header. - change->serializedPayload.encapsulation = (uint16_t)change->serializedPayload.data[1]; - - // Extract GuidPrefix - memcpy(guidP.value, change->serializedPayload.data + 4, 12); - - // Extract liveliness kind - if (is_wlp_kind(&change->serializedPayload.data[16])) + const uint32_t representation_header_size = 4; + const uint32_t guid_prefix_size = 12; + const uint32_t participant_msg_data_size = 4; + const uint32_t participant_msg_data_pos = 16; + const uint32_t encapsulation_pos = 16; + uint32_t data_length; + + // Create CDR message from buffer to deserialize contents for further validation + CDRMessage_t cdr_message(change->serializedPayload); + + bool message_ok = ( + // Skip representation header + CDRMessage::skip(&cdr_message, representation_header_size) + // Extract GuidPrefix + && CDRMessage::readData(&cdr_message, guidP.value, guid_prefix_size) + // Skip kind, it will be validated later + && CDRMessage::skip(&cdr_message, participant_msg_data_size) + // Extract and validate liveliness kind + && get_wlp_kind(&change->serializedPayload.data[participant_msg_data_pos], livelinessKind) + // Extract data length + && CDRMessage::readUInt32(&cdr_message, &data_length) + // Check that serialized length is correctly set + && (change->serializedPayload.length >= min_serialized_length + data_length)); + + if (message_ok) { - // Adjust and cast to LivelinessQosPolicyKind enum, where AUTOMATIC_LIVELINESS_QOS == 0 - livelinessKind = (LivelinessQosPolicyKind)(change->serializedPayload.data[19] - 0x01); + // Extract encapsulation from the second byte of the representation header. + change->serializedPayload.encapsulation = (uint16_t)change->serializedPayload.data[encapsulation_pos]; } else { - logInfo(RTPS_LIVELINESS, "Ignoring not WLP ParticipantDataMessage"); + logInfo(RTPS_LIVELINESS, "Ignoring incorrect WLP ParticipantDataMessage"); history->remove_change(change); return; } - } else { @@ -138,15 +156,11 @@ bool WLPListener::separateKey( GuidPrefix_t* guidP, LivelinessQosPolicyKind* liveliness) { - bool ret = false; - if (is_wlp_kind(&key.value[12])) + bool ret = get_wlp_kind(&key.value[12], *liveliness); + if (ret) { // Extract GuidPrefix memcpy(guidP->value, key.value, 12); - - // Extract liveliness kind - *liveliness = (LivelinessQosPolicyKind)key.value[15]; - ret = true; } return ret; } @@ -167,19 +181,27 @@ bool WLPListener::computeKey( return true; } -bool WLPListener::is_wlp_kind( - octet* kind) +bool WLPListener::get_wlp_kind( + octet* serialized_kind, + LivelinessQosPolicyKind& liveliness_kind) { /* * From RTPS 2.5 9.6.3.1, the ParticipantMessageData kinds for WLP are: * - PARTICIPANT_MESSAGE_DATA_KIND_AUTOMATIC_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x01} * - PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x02} */ - bool is_wlp = true; - is_wlp &= kind[0] == 0; - is_wlp &= kind[1] == 0; - is_wlp &= kind[2] == 0; - is_wlp &= kind[3] == 0x01 || kind[3] == 0x02; + bool is_wlp = ( + serialized_kind[0] == 0 + && serialized_kind[1] == 0 + && serialized_kind[2] == 0 + && (serialized_kind[3] == 0x01 || serialized_kind[3] == 0x02)); + + if (is_wlp) + { + // Adjust and cast to LivelinessQosPolicyKind enum, where AUTOMATIC_LIVELINESS_QOS == 0 + liveliness_kind = (LivelinessQosPolicyKind)(serialized_kind[3] - 0x01); + } + return is_wlp; } From ed13e8108d951975a0ddbfe46a4e80d4dcd070b2 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Sat, 5 Aug 2023 15:59:03 +0200 Subject: [PATCH 06/10] Refs #19347: Fix Windows warning Signed-off-by: Eduardo Ponz --- include/fastdds/rtps/messages/CDRMessage.hpp | 1 + src/cpp/rtps/builtin/liveliness/WLPListener.cpp | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/fastdds/rtps/messages/CDRMessage.hpp b/include/fastdds/rtps/messages/CDRMessage.hpp index d5d67018cd2..d3161570767 100644 --- a/include/fastdds/rtps/messages/CDRMessage.hpp +++ b/include/fastdds/rtps/messages/CDRMessage.hpp @@ -1309,6 +1309,7 @@ inline bool CDRMessage::skip( bool ret = (msg != nullptr) && (msg->pos + length <= msg->length); if (ret) { + // Advance index the number of specified bytes msg->pos += length; } return ret; diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index aa7cac6fc99..b094b7e8c76 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -57,7 +57,7 @@ void WLPListener::onNewCacheChangeAdded( std::lock_guard guard2(*mp_WLP->mp_builtinProtocols->mp_PDP->getMutex()); GuidPrefix_t guidP; - LivelinessQosPolicyKind livelinessKind; + LivelinessQosPolicyKind livelinessKind = AUTOMATIC_LIVELINESS_QOS; CacheChange_t* change = (CacheChange_t*)changeIN; if (!computeKey(change)) { @@ -77,12 +77,17 @@ void WLPListener::onNewCacheChangeAdded( // Serialized payload should have at least 4 bytes of representation header, 12 of GuidPrefix, // 4 of kind, and 4 of length. - const uint32_t min_serialized_length = 4 + 12 + 4 + 4; + const uint32_t representation_header_size = 4; + const uint32_t guid_prefix_size = 12; + const uint32_t participant_msg_data_kind_size = 4; + const uint32_t participant_msg_data_length_size = 4; + const uint32_t min_serialized_length = representation_header_size + + guid_prefix_size + + participant_msg_data_kind_size + + participant_msg_data_length_size; + if (change->serializedPayload.length >= min_serialized_length) { - const uint32_t representation_header_size = 4; - const uint32_t guid_prefix_size = 12; - const uint32_t participant_msg_data_size = 4; const uint32_t participant_msg_data_pos = 16; const uint32_t encapsulation_pos = 16; uint32_t data_length; @@ -96,7 +101,7 @@ void WLPListener::onNewCacheChangeAdded( // Extract GuidPrefix && CDRMessage::readData(&cdr_message, guidP.value, guid_prefix_size) // Skip kind, it will be validated later - && CDRMessage::skip(&cdr_message, participant_msg_data_size) + && CDRMessage::skip(&cdr_message, participant_msg_data_length_size) // Extract and validate liveliness kind && get_wlp_kind(&change->serializedPayload.data[participant_msg_data_pos], livelinessKind) // Extract data length From 5c1e6537c76f300836b6d1027a77cb90b9568170 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Mon, 7 Aug 2023 12:46:53 +0200 Subject: [PATCH 07/10] Refs #19347: Apply suggestions Signed-off-by: Eduardo Ponz --- .../rtps/builtin/liveliness/WLPListener.h | 8 +-- include/fastdds/rtps/common/CDRMessage_t.h | 6 +- .../rtps/builtin/liveliness/WLPListener.cpp | 70 ++++++++++--------- 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/include/fastdds/rtps/builtin/liveliness/WLPListener.h b/include/fastdds/rtps/builtin/liveliness/WLPListener.h index b17e4afdab4..825fe431933 100644 --- a/include/fastdds/rtps/builtin/liveliness/WLPListener.h +++ b/include/fastdds/rtps/builtin/liveliness/WLPListener.h @@ -21,10 +21,10 @@ #define _FASTDDS_RTPS_WLPLISTENER_H_ #ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC -#include -#include +#include #include - +#include +#include #include namespace eprosima { @@ -95,7 +95,7 @@ class WLPListener : public ReaderListener * @return True if the kind corresponds with one for WLP, false otherwise. */ bool get_wlp_kind( - octet* serialized_kind, + const octet* serialized_kind, LivelinessQosPolicyKind& liveliness_kind); //! A pointer to the writer liveliness protocol diff --git a/include/fastdds/rtps/common/CDRMessage_t.h b/include/fastdds/rtps/common/CDRMessage_t.h index 3c93a564274..aa7e6dc18d7 100644 --- a/include/fastdds/rtps/common/CDRMessage_t.h +++ b/include/fastdds/rtps/common/CDRMessage_t.h @@ -96,7 +96,11 @@ struct RTPS_DllAPI CDRMessage_t final const SerializedPayload_t& payload) : wraps(true) { - msg_endian = payload.encapsulation == PL_CDR_BE ? BIGEND : LITTLEEND; + msg_endian = LITTLEEND; + if (payload.encapsulation == PL_CDR_BE || payload.encapsulation == PL_CDR_BE) + { + msg_endian = BIGEND; + } pos = payload.pos; length = payload.length; buffer = payload.data; diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index b094b7e8c76..c7922b34258 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -16,22 +16,30 @@ * @file WLPListener.cpp * */ - #include -#include -#include +#include +#include +#include +#include -#include +#include #include - -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include -#include - -#include - +#include +#include namespace eprosima { namespace fastrtps { @@ -77,46 +85,44 @@ void WLPListener::onNewCacheChangeAdded( // Serialized payload should have at least 4 bytes of representation header, 12 of GuidPrefix, // 4 of kind, and 4 of length. - const uint32_t representation_header_size = 4; - const uint32_t guid_prefix_size = 12; - const uint32_t participant_msg_data_kind_size = 4; - const uint32_t participant_msg_data_length_size = 4; - const uint32_t min_serialized_length = representation_header_size - + guid_prefix_size + constexpr uint32_t participant_msg_data_kind_size = 4; + constexpr uint32_t participant_msg_data_length_size = 4; + constexpr uint32_t min_serialized_length = SerializedPayload_t::representation_header_size + + GuidPrefix_t::size + participant_msg_data_kind_size + participant_msg_data_length_size; if (change->serializedPayload.length >= min_serialized_length) { - const uint32_t participant_msg_data_pos = 16; - const uint32_t encapsulation_pos = 16; + constexpr uint32_t participant_msg_data_kind_pos = 16; + constexpr uint32_t encapsulation_pos = 1; uint32_t data_length; + // Extract encapsulation from the second byte of the representation header. Done prior to + // creating the CDRMessage_t, as the CDRMessage_t ctor uses it for its own state. + change->serializedPayload.encapsulation = + static_cast(change->serializedPayload.data[encapsulation_pos]); + // Create CDR message from buffer to deserialize contents for further validation CDRMessage_t cdr_message(change->serializedPayload); bool message_ok = ( // Skip representation header - CDRMessage::skip(&cdr_message, representation_header_size) + CDRMessage::skip(&cdr_message, SerializedPayload_t::representation_header_size) // Extract GuidPrefix - && CDRMessage::readData(&cdr_message, guidP.value, guid_prefix_size) + && CDRMessage::readData(&cdr_message, guidP.value, GuidPrefix_t::size) // Skip kind, it will be validated later - && CDRMessage::skip(&cdr_message, participant_msg_data_length_size) + && CDRMessage::skip(&cdr_message, participant_msg_data_kind_size) // Extract and validate liveliness kind - && get_wlp_kind(&change->serializedPayload.data[participant_msg_data_pos], livelinessKind) + && get_wlp_kind(&change->serializedPayload.data[participant_msg_data_kind_pos], livelinessKind) // Extract data length && CDRMessage::readUInt32(&cdr_message, &data_length) // Check that serialized length is correctly set && (change->serializedPayload.length >= min_serialized_length + data_length)); - if (message_ok) - { - // Extract encapsulation from the second byte of the representation header. - change->serializedPayload.encapsulation = (uint16_t)change->serializedPayload.data[encapsulation_pos]; - } - else + if (!message_ok) { - logInfo(RTPS_LIVELINESS, "Ignoring incorrect WLP ParticipantDataMessage"); + EPROSIMA_LOG_INFO(RTPS_LIVELINESS, "Ignoring incorrect WLP ParticipantDataMessage"); history->remove_change(change); return; } @@ -128,7 +134,7 @@ void WLPListener::onNewCacheChangeAdded( &guidP, &livelinessKind)) { - logInfo(RTPS_LIVELINESS, "Ignoring not WLP ParticipantDataMessage"); + EPROSIMA_LOG_INFO(RTPS_LIVELINESS, "Ignoring not WLP ParticipantDataMessage"); history->remove_change(change); return; } @@ -187,7 +193,7 @@ bool WLPListener::computeKey( } bool WLPListener::get_wlp_kind( - octet* serialized_kind, + const octet* serialized_kind, LivelinessQosPolicyKind& liveliness_kind) { /* @@ -204,7 +210,7 @@ bool WLPListener::get_wlp_kind( if (is_wlp) { // Adjust and cast to LivelinessQosPolicyKind enum, where AUTOMATIC_LIVELINESS_QOS == 0 - liveliness_kind = (LivelinessQosPolicyKind)(serialized_kind[3] - 0x01); + liveliness_kind = static_cast(serialized_kind[3] - 0x01); } return is_wlp; From ee6b6cd079a9cc933706b330d5b21c67b96ef486 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Mon, 7 Aug 2023 13:19:48 +0200 Subject: [PATCH 08/10] Refs #19347: Correct condition when setting the payload encapsulation Signed-off-by: Eduardo Ponz --- include/fastdds/rtps/common/CDRMessage_t.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fastdds/rtps/common/CDRMessage_t.h b/include/fastdds/rtps/common/CDRMessage_t.h index aa7e6dc18d7..567f4e314bd 100644 --- a/include/fastdds/rtps/common/CDRMessage_t.h +++ b/include/fastdds/rtps/common/CDRMessage_t.h @@ -97,7 +97,7 @@ struct RTPS_DllAPI CDRMessage_t final : wraps(true) { msg_endian = LITTLEEND; - if (payload.encapsulation == PL_CDR_BE || payload.encapsulation == PL_CDR_BE) + if (payload.encapsulation == PL_CDR_BE || payload.encapsulation == CDR_BE) { msg_endian = BIGEND; } From 4d3f784dd362c1b6a7d395edd940f3cf8c8e8e97 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Mon, 7 Aug 2023 13:23:56 +0200 Subject: [PATCH 09/10] Refs #19347: Remove legacy typedef Signed-off-by: Eduardo Ponz --- src/cpp/rtps/builtin/liveliness/WLPListener.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index c7922b34258..949359109e4 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -39,8 +39,6 @@ #include #include -#include - namespace eprosima { namespace fastrtps { namespace rtps { @@ -56,8 +54,6 @@ WLPListener::~WLPListener() { } -typedef std::vector::iterator WPIT; - void WLPListener::onNewCacheChangeAdded( RTPSReader* reader, const CacheChange_t* const changeIN) From be4e5bb215b5e1bb9aabeae5563864284cf9f653 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Date: Mon, 7 Aug 2023 13:26:05 +0200 Subject: [PATCH 10/10] Refs #19347: Initialize uint32_t variable Signed-off-by: Eduardo Ponz --- src/cpp/rtps/builtin/liveliness/WLPListener.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index 949359109e4..ff5683fe3c4 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -43,7 +43,6 @@ namespace eprosima { namespace fastrtps { namespace rtps { - WLPListener::WLPListener( WLP* plwp) : mp_WLP(plwp) @@ -92,7 +91,7 @@ void WLPListener::onNewCacheChangeAdded( { constexpr uint32_t participant_msg_data_kind_pos = 16; constexpr uint32_t encapsulation_pos = 1; - uint32_t data_length; + uint32_t data_length = 0; // Extract encapsulation from the second byte of the representation header. Done prior to // creating the CDRMessage_t, as the CDRMessage_t ctor uses it for its own state.