Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(config_compiler): "/" mistaken as path separator in merged map key #192

Merged
merged 2 commits into from
Mar 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 24 additions & 18 deletions src/rime/config/config_compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,33 +155,39 @@ inline static string StripOperator(const string& key, bool adding) {
}

// defined in config_data.cc
bool TraverseCopyOnWrite(an<ConfigItemRef> root, const string& path,
function<bool (an<ConfigItemRef> target)> writer);
an<ConfigItemRef> TypeCheckedCopyOnWrite(an<ConfigItemRef> parent,
const string& key);
an<ConfigItemRef> TraverseCopyOnWrite(an<ConfigItemRef> head,
const string& path);

static bool EditNode(an<ConfigItemRef> target,
static bool EditNode(an<ConfigItemRef> head,
const string& key,
const an<ConfigItem>& value,
bool merge_tree) {
DLOG(INFO) << "EditNode(" << key << "," << merge_tree << ")";
DLOG(INFO) << "edit node: " << key << ", merge_tree: " << merge_tree;
bool appending = IsAppending(key);
bool merging = IsMerging(key, value, merge_tree);
auto writer = [=](an<ConfigItemRef> target) {
if ((appending || merging) && **target) {
DLOG(INFO) << "writer: editing node";
return !value ||
(appending && (AppendToString(target, As<ConfigValue>(value)) ||
AppendToList(target, As<ConfigList>(value)))) ||
(merging && MergeTree(target, As<ConfigMap>(value)));
} else {
DLOG(INFO) << "writer: overwriting node";
*target = value;
return true;
}
};
string path = StripOperator(key, appending || merging);
DLOG(INFO) << "appending: " << appending << ", merging: " << merging
<< ", path: " << path;
return TraverseCopyOnWrite(target, path, writer);
auto find_target_node =
merge_tree ? &TypeCheckedCopyOnWrite : &TraverseCopyOnWrite;
auto target = find_target_node(head, path);
if (!target) {
// error finding target node; cannot write
return false;
}
if ((appending || merging) && **target) {
DLOG(INFO) << "writer: editing node";
return !value || // no-op
(appending && (AppendToString(target, As<ConfigValue>(value)) ||
AppendToList(target, As<ConfigList>(value)))) ||
(merging && MergeTree(target, As<ConfigMap>(value)));
} else {
DLOG(INFO) << "writer: overwriting node";
*target = value;
return true;
}
}

bool PatchLiteral::Resolve(ConfigCompiler* compiler) {
Expand Down
43 changes: 29 additions & 14 deletions src/rime/config/config_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,37 +159,52 @@ class ConfigDataRootRef : public ConfigItemRef {
ConfigData* data_;
};

bool TraverseCopyOnWrite(an<ConfigItemRef> root, const string& path,
function<bool (an<ConfigItemRef> target)> writer) {
an<ConfigItemRef> TypeCheckedCopyOnWrite(an<ConfigItemRef> parent,
const string& key) {
// special case to allow editing current node by __append: __merge: /+: /=:
if (key.empty()) {
return parent;
}
bool is_list = ConfigData::IsListItemReference(key);
auto expected_node_type = is_list ? ConfigItem::kList : ConfigItem::kMap;
an<ConfigItem> existing_node = *parent;
if (existing_node && existing_node->type() != expected_node_type) {
LOG(ERROR) << "copy on write failed; incompatible node type: " << key;
return nullptr;
}
return Cow(parent, key);
}

an<ConfigItemRef> TraverseCopyOnWrite(an<ConfigItemRef> head,
const string& path) {
DLOG(INFO) << "TraverseCopyOnWrite(" << path << ")";
if (path.empty() || path == "/") {
return writer(root);
return head;
}
an<ConfigItemRef> head = root;
vector<string> keys = ConfigData::SplitPath(path);
size_t n = keys.size();
for (size_t i = 0; i < n; ++i) {
const auto& key = keys[i];
bool is_list = ConfigData::IsListItemReference(key);
auto expected_node_type = is_list ? ConfigItem::kList : ConfigItem::kMap;
an<ConfigItem> existing_node = *head;
if (existing_node && existing_node->type() != expected_node_type) {
LOG(ERROR) << "copy on write failed; incompatible node type: " << key;
return false;
if (auto child = TypeCheckedCopyOnWrite(head, key)) {
head = child;
} else {
LOG(ERROR) << "while writing to " << path;
return nullptr;
}
head = Cow(head, key);
}
return writer(head);
return head;
}

bool ConfigData::TraverseWrite(const string& path, an<ConfigItem> item) {
LOG(INFO) << "write: " << path;
auto root = New<ConfigDataRootRef>(this);
return TraverseCopyOnWrite(root, path, [=](an<ConfigItemRef> target) {
if (auto target = TraverseCopyOnWrite(root, path)) {
*target = item;
set_modified();
return true;
});
} else {
return false;
}
}

vector<string> ConfigData::SplitPath(const string& path) {
Expand Down