From 7a085b2125de4893e05bbe6a8b054efd1499457d Mon Sep 17 00:00:00 2001 From: lijiapei Date: Mon, 7 Sep 2026 12:56:48 +0800 Subject: [PATCH 1/3] Add tests for deserialization of maps into non-empty maps Tests added for both primitive and non-primitive mapped types: - Deserialization into a non-empty map. - Reuse of the same target map in two consecutive deserializations. - Content of the target map preserved when the deserialization fails. These tests show the issues described in #339. Signed-off-by: lijiapei --- test/cdr/SimpleTest.cpp | 168 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/test/cdr/SimpleTest.cpp b/test/cdr/SimpleTest.cpp index 2f746302..716469be 100644 --- a/test/cdr/SimpleTest.cpp +++ b/test/cdr/SimpleTest.cpp @@ -2331,6 +2331,174 @@ TEST(CDRTests, DeserializeIntoANonEmptyMapInXCDRv1) ASSERT_EQ(initialized_map.at(2), "A"); } +// Regression tests for Fast CDR issue #339 +// Deserializing a map into a non-empty map should replace its content, both for +// non-primitive and primitive mapped types. The content of the target map should +// also be preserved when the deserialization fails. + +TEST(CDRTests, DeserializeIntoANonEmptyPrimitiveMapInXCDRv1) +{ + char buffer[BUFFER_LENGTH]; + + const std::map input_map {{"value", 1.0}}; + + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser_map(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1); + cdr_ser_map << input_map; + + std::map initialized_map {{"other", 0.5}}; + + // Deserialization in a non-empty map + Cdr cdr_des_map(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1); + ASSERT_NO_THROW(cdr_des_map >> initialized_map); + ASSERT_EQ(initialized_map.size(), 1u); + EXPECT_DOUBLE_EQ(initialized_map["value"], 1.0); +} + +TEST(CDRTests, ReuseTargetMapInTwoDeserializations) +{ + char buffer[BUFFER_LENGTH]; + + // Case 1: map with non-primitive mapped type + { + using map_type = std::map; + + const map_type first_map {{1, "first"}, {2, "second"}}; + const map_type second_map {{1, "third"}}; + + for (CdrVersion version : {XCDRv1, XCDRv2}) + { + map_type target_map; + + // First deserialization + { + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + cdr_ser << first_map; + Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + ASSERT_NO_THROW(cdr_des >> target_map); + } + + ASSERT_EQ(target_map, first_map); + + // Second deserialization reusing the same (non-empty) map + { + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + cdr_ser << second_map; + Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + ASSERT_NO_THROW(cdr_des >> target_map); + } + + ASSERT_EQ(target_map, second_map); + } + } + + // Case 2: map with primitive mapped type + { + using map_type = std::map; + + const map_type first_map {{"first", 1.0}, {"second", 2.0}}; + const map_type second_map {{"first", 0.0}}; + + for (CdrVersion version : {XCDRv1, XCDRv2}) + { + map_type target_map; + + // First deserialization + { + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + cdr_ser << first_map; + Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + ASSERT_NO_THROW(cdr_des >> target_map); + } + + ASSERT_EQ(target_map, first_map); + + // Second deserialization reusing the same (non-empty) map + { + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + cdr_ser << second_map; + Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version); + ASSERT_NO_THROW(cdr_des >> target_map); + } + + ASSERT_EQ(target_map, second_map); + } + } +} + +TEST(CDRTests, TargetMapPreservedWhenDeserializationFails) +{ + char buffer[BUFFER_LENGTH]; + + // Case 1: map with non-primitive mapped type + { + using map_type = std::map; + + const map_type input_map {{1, "first"}, {2, "second"}}; + const map_type initial_content {{3, "old"}}; + + // XCDRv1: the exception is thrown while deserializing an element + { + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1); + cdr_ser << input_map; + size_t serialized_size = static_cast(cdr_ser.get_current_position() - buffer); + + // Simulate a truncated stream + FastBuffer truncated_buffer(buffer, serialized_size - 1); + Cdr cdr_des(truncated_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1); + + map_type target_map = initial_content; + EXPECT_THROW(cdr_des >> target_map, NotEnoughMemoryException); + EXPECT_EQ(target_map, initial_content); + } + + // XCDRv2: DHEADER announces less bytes than the actual member size + { + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv2); + cdr_ser << input_map; + + // Tamper with DHEADER to make it inconsistent with the member size + uint32_t dheader; + memcpy(&dheader, buffer, sizeof(dheader)); + dheader -= 1; + memcpy(buffer, &dheader, sizeof(dheader)); + + Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv2); + + map_type target_map = initial_content; + EXPECT_THROW(cdr_des >> target_map, BadParamException); + EXPECT_EQ(target_map, initial_content); + } + } + + // Case 2: map with primitive mapped type + { + using map_type = std::map; + + const map_type input_map {{"first", 1.0}, {"second", 2.0}}; + const map_type initial_content {{"other", 0.5}}; + + FastBuffer cdr_buffer(buffer, BUFFER_LENGTH); + Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1); + cdr_ser << input_map; + size_t serialized_size = static_cast(cdr_ser.get_current_position() - buffer); + + // The first element is deserialized before the exception is thrown + FastBuffer truncated_buffer(buffer, serialized_size - sizeof(double)); + Cdr cdr_des(truncated_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1); + + map_type target_map = initial_content; + EXPECT_THROW(cdr_des >> target_map, NotEnoughMemoryException); + EXPECT_EQ(target_map, initial_content); + } +} + TEST(FastCDRTests, Octet) { // Check good case. From e3d2d730d57e224cbe767dc174bd298a0f10491e Mon Sep 17 00:00:00 2001 From: lijiapei Date: Mon, 7 Sep 2026 12:59:48 +0800 Subject: [PATCH 2/3] Fix deserialization of maps into non-empty maps Deserialize into a new map and move it into the output one once the whole deserialization succeeds. This way the output map is not left cleared or partially filled when an exception is thrown inside the deserialization loop, and the previous content of a non-empty output map is always replaced. This applies to both the primitive and non-primitive mapped type overloads. Fixes #339. Signed-off-by: lijiapei --- include/fastcdr/Cdr.h | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/include/fastcdr/Cdr.h b/include/fastcdr/Cdr.h index 5674e360..b85bf12b 100644 --- a/include/fastcdr/Cdr.h +++ b/include/fastcdr/Cdr.h @@ -2034,6 +2034,7 @@ class Cdr /*! * @brief This function template deserializes a map of non-primitive. + * The content of the output map is only replaced when the whole deserialization succeeds. * @param map_t The variable that will store the map read from the buffer. * @return Reference to the eprosima::fastcdr::Cdr object. * @exception exception::NotEnoughMemoryException This exception is thrown when trying to deserialize a position that exceeds the internal memory size. @@ -2044,6 +2045,7 @@ class Cdr std::map<_K, _T>& map_t) { state state_before_error(*this); + std::map<_K, _T> map; if (CdrVersion::XCDRv2 == cdr_version_) { @@ -2063,8 +2065,6 @@ class Cdr uint32_t map_length {0}; deserialize(map_length); - map_t.clear(); - try { uint32_t count {0}; @@ -2074,7 +2074,7 @@ class Cdr _T val; deserialize(key); deserialize(val); - map_t.emplace(std::pair<_K, _T>(std::move(key), std::move(val))); + map.emplace(std::pair<_K, _T>(std::move(key), std::move(val))); ++count; } } @@ -2096,8 +2096,6 @@ class Cdr deserialize(sequence_length); - map_t.clear(); - try { for (uint32_t i = 0; i < sequence_length; ++i) @@ -2106,7 +2104,7 @@ class Cdr _T value; deserialize(key); deserialize(value); - map_t.emplace(std::pair<_K, _T>(std::move(key), std::move(value))); + map.emplace(std::pair<_K, _T>(std::move(key), std::move(value))); } } catch (exception::Exception& ex) @@ -2116,11 +2114,14 @@ class Cdr } } + map_t = std::move(map); + return *this; } /*! * @brief This function template deserializes a map of primitive. + * The content of the output map is only replaced when the whole deserialization succeeds. * @param map_t The variable that will store the map read from the buffer. * @return Reference to the eprosima::fastcdr::Cdr object. * @exception exception::NotEnoughMemoryException This exception is thrown when trying to deserialize a position that exceeds the internal memory size. @@ -2132,6 +2133,7 @@ class Cdr { uint32_t sequence_length = 0; state state_(*this); + std::map<_K, _T> map; deserialize(sequence_length); @@ -2143,7 +2145,7 @@ class Cdr _T value; deserialize(key); deserialize(value); - map_t.emplace(std::pair<_K, _T>(std::move(key), std::move(value))); + map.emplace(std::pair<_K, _T>(std::move(key), std::move(value))); } } catch (exception::Exception& ex) @@ -2152,6 +2154,8 @@ class Cdr ex.raise(); } + map_t = std::move(map); + return *this; } From 97b119c96bf8053fc923c338556ec89a2ddafabb Mon Sep 17 00:00:00 2001 From: lijiapei Date: Mon, 7 Sep 2026 15:41:25 +0800 Subject: [PATCH 3/3] Fix MSVC C4244 warning in map deserialization tests Use explicit static_cast for map keys, as done in DeserializeIntoANonEmptyMapInXCDRv1. int literals converted to uint16_t trigger warning C4244, treated as error by Windows CI. Signed-off-by: lijiapei --- test/cdr/SimpleTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/cdr/SimpleTest.cpp b/test/cdr/SimpleTest.cpp index 716469be..5f75cab6 100644 --- a/test/cdr/SimpleTest.cpp +++ b/test/cdr/SimpleTest.cpp @@ -2363,8 +2363,8 @@ TEST(CDRTests, ReuseTargetMapInTwoDeserializations) { using map_type = std::map; - const map_type first_map {{1, "first"}, {2, "second"}}; - const map_type second_map {{1, "third"}}; + const map_type first_map {{static_cast(1u), "first"}, {static_cast(2u), "second"}}; + const map_type second_map {{static_cast(1u), "third"}}; for (CdrVersion version : {XCDRv1, XCDRv2}) { @@ -2438,8 +2438,8 @@ TEST(CDRTests, TargetMapPreservedWhenDeserializationFails) { using map_type = std::map; - const map_type input_map {{1, "first"}, {2, "second"}}; - const map_type initial_content {{3, "old"}}; + const map_type input_map {{static_cast(1u), "first"}, {static_cast(2u), "second"}}; + const map_type initial_content {{static_cast(3u), "old"}}; // XCDRv1: the exception is thrown while deserializing an element {