From cd8b23d76b3ac13691717291f90d0d10286dace1 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 22 Jan 2021 13:38:54 +0100 Subject: [PATCH 1/4] Capturing fastcdr exceptions. Signed-off-by: Miguel Company --- rmw_fastrtps_cpp/src/type_support_common.cpp | 31 ++++++++++++------- .../TypeSupport_impl.hpp | 19 ++++++++---- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/rmw_fastrtps_cpp/src/type_support_common.cpp b/rmw_fastrtps_cpp/src/type_support_common.cpp index 9cc671b29..06aa1f3df 100644 --- a/rmw_fastrtps_cpp/src/type_support_common.cpp +++ b/rmw_fastrtps_cpp/src/type_support_common.cpp @@ -14,6 +14,8 @@ #include +#include + #include "rmw/error_handling.h" #include "type_support_common.hpp" @@ -88,19 +90,24 @@ bool TypeSupport::deserializeROSmessage( assert(ros_message); assert(impl); - // Deserialize encapsulation. - deser.read_encapsulation(); - - // If type is not empty, deserialize message - if (has_data_) { - auto callbacks = static_cast(impl); - return callbacks->cdr_deserialize(deser, ros_message); + try { + // Deserialize encapsulation. + deser.read_encapsulation(); + + // If type is not empty, deserialize message + if (has_data_) { + auto callbacks = static_cast(impl); + return callbacks->cdr_deserialize(deser, ros_message); + } + + // Otherwise, consume dummy byte + uint8_t dump = 0; + deser >> dump; + (void)dump; + } + catch(const eprosima::fastcdr::exception::Exception&) { + return false; } - - // Otherwise, consume dummy byte - uint8_t dump = 0; - deser >> dump; - (void)dump; return true; } diff --git a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp index 398c300fc..58d913c0f 100644 --- a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp @@ -17,6 +17,8 @@ #include #include +#include + #include #include #include @@ -936,17 +938,22 @@ bool TypeSupport::deserializeROSmessage( assert(ros_message); assert(members_); - // Deserialize encapsulation. - deser.read_encapsulation(); + try { + // Deserialize encapsulation. + deser.read_encapsulation(); + + (void)impl; + if (members_->member_count_ != 0) { + return TypeSupport::deserializeROSmessage(deser, members_, ros_message); + } - (void)impl; - if (members_->member_count_ != 0) { - TypeSupport::deserializeROSmessage(deser, members_, ros_message); - } else { uint8_t dump = 0; deser >> dump; (void)dump; } + catch(const eprosima::fastcdr::exception::Exception&) { + return false; + } return true; } From 22024c430e4be636b589b8e9d0c9543d24627989 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 22 Jan 2021 13:40:22 +0100 Subject: [PATCH 2/4] Bubble up deserialization errors. Signed-off-by: Miguel Company --- .../TypeSupport_impl.hpp | 8 ++++-- rmw_fastrtps_shared_cpp/src/rmw_request.cpp | 28 +++++++++---------- rmw_fastrtps_shared_cpp/src/rmw_response.cpp | 20 ++++++------- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp index 58d913c0f..d4fc2160c 100644 --- a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp @@ -773,7 +773,9 @@ bool TypeSupport::deserializeROSmessage( { auto sub_members = static_cast(member->members_->data); if (!member->is_array_) { - deserializeROSmessage(deser, sub_members, field); + if (!deserializeROSmessage(deser, sub_members, field)) { + return false; + } } else { size_t array_size = 0; @@ -796,7 +798,9 @@ bool TypeSupport::deserializeROSmessage( return false; } for (size_t index = 0; index < array_size; ++index) { - deserializeROSmessage(deser, sub_members, member->get_function(field, index)); + if (!deserializeROSmessage(deser, sub_members, member->get_function(field, index))) { + return false; + } } } } diff --git a/rmw_fastrtps_shared_cpp/src/rmw_request.cpp b/rmw_fastrtps_shared_cpp/src/rmw_request.cpp index 2c1ef8a64..144d92481 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_request.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_request.cpp @@ -96,22 +96,22 @@ __rmw_take_request( if (request.buffer_ != nullptr) { eprosima::fastcdr::Cdr deser(*request.buffer_, eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, eprosima::fastcdr::Cdr::DDS_CDR); - info->request_type_support_->deserializeROSmessage( - deser, ros_request, info->request_type_support_impl_); - - // Get header - rmw_fastrtps_shared_cpp::copy_from_fastrtps_guid_to_byte_array( - request.sample_identity_.writer_guid(), - request_header->request_id.writer_guid); - request_header->request_id.sequence_number = - ((int64_t)request.sample_identity_.sequence_number().high) << - 32 | request.sample_identity_.sequence_number().low; - request_header->source_timestamp = request.sample_info_.sourceTimestamp.to_ns(); - request_header->received_timestamp = request.sample_info_.receptionTimestamp.to_ns(); + if (info->request_type_support_->deserializeROSmessage( + deser, ros_request, info->request_type_support_impl_)) + { + // Get header + rmw_fastrtps_shared_cpp::copy_from_fastrtps_guid_to_byte_array( + request.sample_identity_.writer_guid(), + request_header->request_id.writer_guid); + request_header->request_id.sequence_number = + ((int64_t)request.sample_identity_.sequence_number().high) << + 32 | request.sample_identity_.sequence_number().low; + request_header->source_timestamp = request.sample_info_.sourceTimestamp.to_ns(); + request_header->received_timestamp = request.sample_info_.receptionTimestamp.to_ns(); + *taken = true; + } delete request.buffer_; - - *taken = true; } return RMW_RET_OK; diff --git a/rmw_fastrtps_shared_cpp/src/rmw_response.cpp b/rmw_fastrtps_shared_cpp/src/rmw_response.cpp index cb36b27a6..0ea43f57c 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_response.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_response.cpp @@ -59,16 +59,16 @@ __rmw_take_response( *response.buffer_, eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, eprosima::fastcdr::Cdr::DDS_CDR); - info->response_type_support_->deserializeROSmessage( - deser, ros_response, info->response_type_support_impl_); - - request_header->source_timestamp = response.sample_info_.sourceTimestamp.to_ns(); - request_header->received_timestamp = response.sample_info_.receptionTimestamp.to_ns(); - request_header->request_id.sequence_number = - ((int64_t)response.sample_identity_.sequence_number().high) << - 32 | response.sample_identity_.sequence_number().low; - - *taken = true; + if (info->response_type_support_->deserializeROSmessage( + deser, ros_response, info->response_type_support_impl_)) { + request_header->source_timestamp = response.sample_info_.sourceTimestamp.to_ns(); + request_header->received_timestamp = response.sample_info_.receptionTimestamp.to_ns(); + request_header->request_id.sequence_number = + ((int64_t)response.sample_identity_.sequence_number().high) << + 32 | response.sample_identity_.sequence_number().low; + + *taken = true; + } } return RMW_RET_OK; From 8c39a10e665b2d107a8bf37b9fdf345e11a80586 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 22 Jan 2021 14:05:33 +0100 Subject: [PATCH 3/4] Keep linters happy. Signed-off-by: Miguel Company --- rmw_fastrtps_cpp/src/type_support_common.cpp | 13 ++++++------- .../rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp | 5 ++--- rmw_fastrtps_shared_cpp/src/rmw_request.cpp | 2 +- rmw_fastrtps_shared_cpp/src/rmw_response.cpp | 3 ++- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/rmw_fastrtps_cpp/src/type_support_common.cpp b/rmw_fastrtps_cpp/src/type_support_common.cpp index 06aa1f3df..2e938cb9d 100644 --- a/rmw_fastrtps_cpp/src/type_support_common.cpp +++ b/rmw_fastrtps_cpp/src/type_support_common.cpp @@ -12,10 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include - #include +#include + #include "rmw/error_handling.h" #include "type_support_common.hpp" @@ -93,19 +93,18 @@ bool TypeSupport::deserializeROSmessage( try { // Deserialize encapsulation. deser.read_encapsulation(); - + // If type is not empty, deserialize message if (has_data_) { auto callbacks = static_cast(impl); - return callbacks->cdr_deserialize(deser, ros_message); + return callbacks->cdr_deserialize(deser, ros_message); } - + // Otherwise, consume dummy byte uint8_t dump = 0; deser >> dump; (void)dump; - } - catch(const eprosima::fastcdr::exception::Exception&) { + } catch (const eprosima::fastcdr::exception::Exception &) { return false; } diff --git a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp index d4fc2160c..02507b726 100644 --- a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp @@ -945,7 +945,7 @@ bool TypeSupport::deserializeROSmessage( try { // Deserialize encapsulation. deser.read_encapsulation(); - + (void)impl; if (members_->member_count_ != 0) { return TypeSupport::deserializeROSmessage(deser, members_, ros_message); @@ -954,8 +954,7 @@ bool TypeSupport::deserializeROSmessage( uint8_t dump = 0; deser >> dump; (void)dump; - } - catch(const eprosima::fastcdr::exception::Exception&) { + } catch (const eprosima::fastcdr::exception::Exception &) { return false; } diff --git a/rmw_fastrtps_shared_cpp/src/rmw_request.cpp b/rmw_fastrtps_shared_cpp/src/rmw_request.cpp index 144d92481..3d21032f4 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_request.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_request.cpp @@ -97,7 +97,7 @@ __rmw_take_request( eprosima::fastcdr::Cdr deser(*request.buffer_, eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, eprosima::fastcdr::Cdr::DDS_CDR); if (info->request_type_support_->deserializeROSmessage( - deser, ros_request, info->request_type_support_impl_)) + deser, ros_request, info->request_type_support_impl_)) { // Get header rmw_fastrtps_shared_cpp::copy_from_fastrtps_guid_to_byte_array( diff --git a/rmw_fastrtps_shared_cpp/src/rmw_response.cpp b/rmw_fastrtps_shared_cpp/src/rmw_response.cpp index 0ea43f57c..2c700c8f6 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_response.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_response.cpp @@ -60,7 +60,8 @@ __rmw_take_response( eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, eprosima::fastcdr::Cdr::DDS_CDR); if (info->response_type_support_->deserializeROSmessage( - deser, ros_response, info->response_type_support_impl_)) { + deser, ros_response, info->response_type_support_impl_)) + { request_header->source_timestamp = response.sample_info_.sourceTimestamp.to_ns(); request_header->received_timestamp = response.sample_info_.receptionTimestamp.to_ns(); request_header->request_id.sequence_number = From ef9b2b976537b48589f94aec2f5be41cbf5bcac3 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 1 Feb 2021 15:57:25 +0100 Subject: [PATCH 4/4] Added error messages. Signed-off-by: Miguel Company --- rmw_fastrtps_cpp/src/type_support_common.cpp | 3 +++ .../include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp | 3 +++ 2 files changed, 6 insertions(+) diff --git a/rmw_fastrtps_cpp/src/type_support_common.cpp b/rmw_fastrtps_cpp/src/type_support_common.cpp index 2e938cb9d..df8cfa101 100644 --- a/rmw_fastrtps_cpp/src/type_support_common.cpp +++ b/rmw_fastrtps_cpp/src/type_support_common.cpp @@ -105,6 +105,9 @@ bool TypeSupport::deserializeROSmessage( deser >> dump; (void)dump; } catch (const eprosima::fastcdr::exception::Exception &) { + RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Fast CDR exception deserializing message of type %s.", + getName()); return false; } diff --git a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp index 02507b726..52b045c35 100644 --- a/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp @@ -955,6 +955,9 @@ bool TypeSupport::deserializeROSmessage( deser >> dump; (void)dump; } catch (const eprosima::fastcdr::exception::Exception &) { + RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Fast CDR exception deserializing message of type %s.", + getName()); return false; }