Fix deserialization of maps into non-empty maps

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;
     }
 

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<std::string, double> 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<std::string, double> 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<uint16_t, std::string>;
+
+        const map_type first_map {{static_cast<uint16_t>(1u), "first"}, {static_cast<uint16_t>(2u), "second"}};
+        const map_type second_map {{static_cast<uint16_t>(1u), "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<std::string, double>;
+
+        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<uint16_t, std::string>;
+
+        const map_type input_map {{static_cast<uint16_t>(1u), "first"}, {static_cast<uint16_t>(2u), "second"}};
+        const map_type initial_content {{static_cast<uint16_t>(3u), "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<size_t>(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<std::string, double>;
+
+        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<size_t>(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.