Skip to content

Commit 757d5e4

Browse files
mergify[bot]shashenli
mergify[bot]
and
shashenli
authored
Fix deserialization of maps into non-empty maps (#340) (#342)
* 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. * 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. * Fix MSVC C4244 warning in map deserialization tests Use explicit static_cast<uint16_t> for map keys, as done in DeserializeIntoANonEmptyMapInXCDRv1. int literals converted to uint16_t trigger warning C4244, treated as error by Windows CI. --------- (cherry picked from commit b2457c4) Signed-off-by: lijiapei <l2249712344@gmail.com> Co-authored-by: lijiapei <lilingjie@cvte.com>
1 parent e93b83d commit 757d5e4

2 files changed

Lines changed: 179 additions & 7 deletions

File tree

‎include/fastcdr/Cdr.h‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,6 +2011,7 @@ class Cdr
20112011

20122012
/*!
20132013
* @brief This function template deserializes a map of non-primitive.
2014+
* The content of the output map is only replaced when the whole deserialization succeeds.
20142015
* @param map_t The variable that will store the map read from the buffer.
20152016
* @return Reference to the eprosima::fastcdr::Cdr object.
20162017
* @exception exception::NotEnoughMemoryException This exception is thrown when trying to deserialize a position that exceeds the internal memory size.
@@ -2021,6 +2022,7 @@ class Cdr
20212022
std::map<_K, _T>& map_t)
20222023
{
20232024
state state_before_error(*this);
2025+
std::map<_K, _T> map;
20242026

20252027
if (CdrVersion::XCDRv2 == cdr_version_)
20262028
{
@@ -2040,8 +2042,6 @@ class Cdr
20402042
uint32_t map_length {0};
20412043
deserialize(map_length);
20422044

2043-
map_t.clear();
2044-
20452045
try
20462046
{
20472047
uint32_t count {0};
@@ -2051,7 +2051,7 @@ class Cdr
20512051
_T val;
20522052
deserialize(key);
20532053
deserialize(val);
2054-
map_t.emplace(std::pair<_K, _T>(std::move(key), std::move(val)));
2054+
map.emplace(std::pair<_K, _T>(std::move(key), std::move(val)));
20552055
++count;
20562056
}
20572057
}
@@ -2073,8 +2073,6 @@ class Cdr
20732073

20742074
deserialize(sequence_length);
20752075

2076-
map_t.clear();
2077-
20782076
try
20792077
{
20802078
for (uint32_t i = 0; i < sequence_length; ++i)
@@ -2083,7 +2081,7 @@ class Cdr
20832081
_T value;
20842082
deserialize(key);
20852083
deserialize(value);
2086-
map_t.emplace(std::pair<_K, _T>(std::move(key), std::move(value)));
2084+
map.emplace(std::pair<_K, _T>(std::move(key), std::move(value)));
20872085
}
20882086
}
20892087
catch (exception::Exception& ex)
@@ -2093,11 +2091,14 @@ class Cdr
20932091
}
20942092
}
20952093

2094+
map_t = std::move(map);
2095+
20962096
return *this;
20972097
}
20982098

20992099
/*!
21002100
* @brief This function template deserializes a map of primitive.
2101+
* The content of the output map is only replaced when the whole deserialization succeeds.
21012102
* @param map_t The variable that will store the map read from the buffer.
21022103
* @return Reference to the eprosima::fastcdr::Cdr object.
21032104
* @exception exception::NotEnoughMemoryException This exception is thrown when trying to deserialize a position that exceeds the internal memory size.
@@ -2109,6 +2110,7 @@ class Cdr
21092110
{
21102111
uint32_t sequence_length = 0;
21112112
state state_(*this);
2113+
std::map<_K, _T> map;
21122114

21132115
deserialize(sequence_length);
21142116

@@ -2120,7 +2122,7 @@ class Cdr
21202122
_T value;
21212123
deserialize(key);
21222124
deserialize(value);
2123-
map_t.emplace(std::pair<_K, _T>(std::move(key), std::move(value)));
2125+
map.emplace(std::pair<_K, _T>(std::move(key), std::move(value)));
21242126
}
21252127
}
21262128
catch (exception::Exception& ex)
@@ -2129,6 +2131,8 @@ class Cdr
21292131
ex.raise();
21302132
}
21312133

2134+
map_t = std::move(map);
2135+
21322136
return *this;
21332137
}
21342138

‎test/cdr/SimpleTest.cpp‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2331,6 +2331,174 @@ TEST(CDRTests, DeserializeIntoANonEmptyMapInXCDRv1)
23312331
ASSERT_EQ(initialized_map.at(2), "A");
23322332
}
23332333

2334+
// Regression tests for Fast CDR issue #339
2335+
// Deserializing a map into a non-empty map should replace its content, both for
2336+
// non-primitive and primitive mapped types. The content of the target map should
2337+
// also be preserved when the deserialization fails.
2338+
2339+
TEST(CDRTests, DeserializeIntoANonEmptyPrimitiveMapInXCDRv1)
2340+
{
2341+
char buffer[BUFFER_LENGTH];
2342+
2343+
const std::map<std::string, double> input_map {{"value", 1.0}};
2344+
2345+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2346+
Cdr cdr_ser_map(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1);
2347+
cdr_ser_map << input_map;
2348+
2349+
std::map<std::string, double> initialized_map {{"other", 0.5}};
2350+
2351+
// Deserialization in a non-empty map
2352+
Cdr cdr_des_map(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1);
2353+
ASSERT_NO_THROW(cdr_des_map >> initialized_map);
2354+
ASSERT_EQ(initialized_map.size(), 1u);
2355+
EXPECT_DOUBLE_EQ(initialized_map["value"], 1.0);
2356+
}
2357+
2358+
TEST(CDRTests, ReuseTargetMapInTwoDeserializations)
2359+
{
2360+
char buffer[BUFFER_LENGTH];
2361+
2362+
// Case 1: map with non-primitive mapped type
2363+
{
2364+
using map_type = std::map<uint16_t, std::string>;
2365+
2366+
const map_type first_map {{static_cast<uint16_t>(1u), "first"}, {static_cast<uint16_t>(2u), "second"}};
2367+
const map_type second_map {{static_cast<uint16_t>(1u), "third"}};
2368+
2369+
for (CdrVersion version : {XCDRv1, XCDRv2})
2370+
{
2371+
map_type target_map;
2372+
2373+
// First deserialization
2374+
{
2375+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2376+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2377+
cdr_ser << first_map;
2378+
Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2379+
ASSERT_NO_THROW(cdr_des >> target_map);
2380+
}
2381+
2382+
ASSERT_EQ(target_map, first_map);
2383+
2384+
// Second deserialization reusing the same (non-empty) map
2385+
{
2386+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2387+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2388+
cdr_ser << second_map;
2389+
Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2390+
ASSERT_NO_THROW(cdr_des >> target_map);
2391+
}
2392+
2393+
ASSERT_EQ(target_map, second_map);
2394+
}
2395+
}
2396+
2397+
// Case 2: map with primitive mapped type
2398+
{
2399+
using map_type = std::map<std::string, double>;
2400+
2401+
const map_type first_map {{"first", 1.0}, {"second", 2.0}};
2402+
const map_type second_map {{"first", 0.0}};
2403+
2404+
for (CdrVersion version : {XCDRv1, XCDRv2})
2405+
{
2406+
map_type target_map;
2407+
2408+
// First deserialization
2409+
{
2410+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2411+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2412+
cdr_ser << first_map;
2413+
Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2414+
ASSERT_NO_THROW(cdr_des >> target_map);
2415+
}
2416+
2417+
ASSERT_EQ(target_map, first_map);
2418+
2419+
// Second deserialization reusing the same (non-empty) map
2420+
{
2421+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2422+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2423+
cdr_ser << second_map;
2424+
Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, version);
2425+
ASSERT_NO_THROW(cdr_des >> target_map);
2426+
}
2427+
2428+
ASSERT_EQ(target_map, second_map);
2429+
}
2430+
}
2431+
}
2432+
2433+
TEST(CDRTests, TargetMapPreservedWhenDeserializationFails)
2434+
{
2435+
char buffer[BUFFER_LENGTH];
2436+
2437+
// Case 1: map with non-primitive mapped type
2438+
{
2439+
using map_type = std::map<uint16_t, std::string>;
2440+
2441+
const map_type input_map {{static_cast<uint16_t>(1u), "first"}, {static_cast<uint16_t>(2u), "second"}};
2442+
const map_type initial_content {{static_cast<uint16_t>(3u), "old"}};
2443+
2444+
// XCDRv1: the exception is thrown while deserializing an element
2445+
{
2446+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2447+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1);
2448+
cdr_ser << input_map;
2449+
size_t serialized_size = static_cast<size_t>(cdr_ser.get_current_position() - buffer);
2450+
2451+
// Simulate a truncated stream
2452+
FastBuffer truncated_buffer(buffer, serialized_size - 1);
2453+
Cdr cdr_des(truncated_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1);
2454+
2455+
map_type target_map = initial_content;
2456+
EXPECT_THROW(cdr_des >> target_map, NotEnoughMemoryException);
2457+
EXPECT_EQ(target_map, initial_content);
2458+
}
2459+
2460+
// XCDRv2: DHEADER announces less bytes than the actual member size
2461+
{
2462+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2463+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv2);
2464+
cdr_ser << input_map;
2465+
2466+
// Tamper with DHEADER to make it inconsistent with the member size
2467+
uint32_t dheader;
2468+
memcpy(&dheader, buffer, sizeof(dheader));
2469+
dheader -= 1;
2470+
memcpy(buffer, &dheader, sizeof(dheader));
2471+
2472+
Cdr cdr_des(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv2);
2473+
2474+
map_type target_map = initial_content;
2475+
EXPECT_THROW(cdr_des >> target_map, BadParamException);
2476+
EXPECT_EQ(target_map, initial_content);
2477+
}
2478+
}
2479+
2480+
// Case 2: map with primitive mapped type
2481+
{
2482+
using map_type = std::map<std::string, double>;
2483+
2484+
const map_type input_map {{"first", 1.0}, {"second", 2.0}};
2485+
const map_type initial_content {{"other", 0.5}};
2486+
2487+
FastBuffer cdr_buffer(buffer, BUFFER_LENGTH);
2488+
Cdr cdr_ser(cdr_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1);
2489+
cdr_ser << input_map;
2490+
size_t serialized_size = static_cast<size_t>(cdr_ser.get_current_position() - buffer);
2491+
2492+
// The first element is deserialized before the exception is thrown
2493+
FastBuffer truncated_buffer(buffer, serialized_size - sizeof(double));
2494+
Cdr cdr_des(truncated_buffer, Cdr::DEFAULT_ENDIAN, XCDRv1);
2495+
2496+
map_type target_map = initial_content;
2497+
EXPECT_THROW(cdr_des >> target_map, NotEnoughMemoryException);
2498+
EXPECT_EQ(target_map, initial_content);
2499+
}
2500+
}
2501+
23342502
TEST(FastCDRTests, Octet)
23352503
{
23362504
// Check good case.

0 commit comments

Comments
 (0)