Skip to content

Commit bcf7604

Browse files
committed
Fix merge operator support (that can be visible by iterating through the node)
The problem is that const_map_to.get(*j->first) compares only the shared_ptr's, while we need to compare the key itself. v2: remove const iterator v3: fix use-after-free due to const reference
1 parent 6292971 commit bcf7604

File tree

2 files changed

+29
-5
lines changed

2 files changed

+29
-5
lines changed

src/nodebuilder.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <algorithm>
12
#include <cassert>
23

34
#include "nodebuilder.h"
@@ -75,12 +76,20 @@ void NodeBuilder::OnMapStart(const Mark& mark, const std::string& tag,
7576

7677
void MergeMapCollection(detail::node& map_to, detail::node& map_from,
7778
detail::shared_memory_holder& pMemory) {
78-
const detail::node& const_map_to = map_to;
7979
for (auto j = map_from.begin(); j != map_from.end(); j++) {
80-
detail::node* s = const_map_to.get(*j->first, pMemory);
81-
if (s == nullptr) {
82-
map_to.insert(*j->first, *j->second, pMemory);
83-
}
80+
const auto from_key = j->first;
81+
/// NOTE: const_map_to.get(*j->first) cannot be used here, since it
82+
/// compares only the shared_ptr's, while we need to compare the key
83+
/// itself.
84+
///
85+
/// NOTE: get() also iterates over elements
86+
bool found = std::any_of(map_to.begin(), map_to.end(), [&](const detail::node_iterator_value<detail::node> & kv)
87+
{
88+
const auto key_node = kv.first;
89+
return key_node->scalar() == from_key->scalar();
90+
});
91+
if (!found)
92+
map_to.insert(*from_key, *j->second, pMemory);
8493
}
8594
}
8695

test/integration/load_node_test.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "yaml-cpp/yaml.h" // IWYU pragma: keep
22

33
#include "gtest/gtest.h"
4+
#include <algorithm>
45

56
namespace YAML {
67
namespace {
@@ -184,6 +185,20 @@ TEST(LoadNodeTest, MergeKeyA) {
184185
EXPECT_EQ(1, node["z"]["c"].as<int>());
185186
}
186187

188+
TEST(LoadNodeTest, MergeKeyAIterator) {
189+
Node node = Load(
190+
"{x: &foo {a : 1,b : 1,c : 1}, y: &bar {d: 2, e : 2, f : 2, a : 2}, z: "
191+
"&stuff { << : *foo, b : 3} }");
192+
EXPECT_EQ(NodeType::Map, node["z"].Type());
193+
194+
const auto& z = node["z"];
195+
size_t z_b_keys = std::count_if(z.begin(), z.end(), [&](const detail::iterator_value & kv)
196+
{
197+
return kv.first.as<std::string>() == "b";
198+
});
199+
ASSERT_EQ(z_b_keys, 1);
200+
}
201+
187202
TEST(LoadNodeTest, MergeKeyB) {
188203
Node node = Load(
189204
"{x: &foo {a : 1,b : 1,c : 1}, y: &bar {d: 2, e : 2, f : 2, a : 2}, z: "

0 commit comments

Comments
 (0)