Skip to content

Commit e48303c

Browse files
committed
data tree CHANGE when validating nested op, do not validate parents
This is a revert of commits ed79390 and e0ffe59, which were incorrectly trying to solve a problem that should not exist to begin with. The problematic use-case added to tests.
1 parent 4563cba commit e48303c

File tree

10 files changed

+235
-141
lines changed

10 files changed

+235
-141
lines changed

src/parser_json.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,7 @@ lyd_parse_json(struct ly_ctx *ctx, const char *data, int options, const struct l
15711571
}
15721572

15731573
/* add/validate default values, unres */
1574-
if (lyd_defaults_add_unres(&result, options, ctx, NULL, 0, data_tree, &act_notif, unres, 1)) {
1574+
if (lyd_defaults_add_unres(&result, options, ctx, NULL, 0, data_tree, act_notif, unres, 1)) {
15751575
goto error;
15761576
}
15771577

src/parser_lyb.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ lyd_parse_lyb(struct ly_ctx *ctx, const char *data, int options, const struct ly
12861286
LY_TREE_DFS_END(node, next, act_notif);
12871287
}
12881288
}
1289-
if (lyd_defaults_add_unres(&node, options, ctx, NULL, 0, data_tree, &act_notif, unres, 0)) {
1289+
if (lyd_defaults_add_unres(&node, options, ctx, NULL, 0, data_tree, act_notif, unres, 0)) {
12901290
lyd_free_withsiblings(node);
12911291
node = NULL;
12921292
goto finish;

src/parser_xml.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ lyd_parse_xml(struct ly_ctx *ctx, struct lyxml_elem **root, int options, ...)
741741
}
742742

743743
/* add default values, resolve unres and check for mandatory nodes in final tree */
744-
if (lyd_defaults_add_unres(&result, options, ctx, NULL, 0, data_tree, &act_notif, unres, 1)) {
744+
if (lyd_defaults_add_unres(&result, options, ctx, NULL, 0, data_tree, act_notif, unres, 1)) {
745745
goto error;
746746
}
747747
if (!(options & (LYD_OPT_TRUSTED | LYD_OPT_NOTIF_FILTER))

src/tree_data.c

Lines changed: 104 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -5123,7 +5123,7 @@ _lyd_validate(struct lyd_node **node, struct lyd_node *data_tree, struct ly_ctx
51235123
}
51245124

51255125
/* add default values, resolve unres and check for mandatory nodes in final tree */
5126-
if (lyd_defaults_add_unres(node, options, ctx, modules, mod_count, data_tree, &act_notif, unres, 1)) {
5126+
if (lyd_defaults_add_unres(node, options, ctx, modules, mod_count, data_tree, act_notif, unres, 1)) {
51275127
goto cleanup;
51285128
}
51295129
if (act_notif) {
@@ -7732,16 +7732,15 @@ lyd_wd_add(struct lyd_node **root, struct ly_ctx *ctx, const struct lys_module *
77327732

77337733
int
77347734
lyd_defaults_add_unres(struct lyd_node **root, int options, struct ly_ctx *ctx, const struct lys_module **modules,
7735-
int mod_count, const struct lyd_node *data_tree, struct lyd_node **act_notif,
7735+
int mod_count, const struct lyd_node *data_tree, struct lyd_node *act_notif,
77367736
struct unres_data *unres, int wd)
77377737
{
7738-
struct lyd_difflist *diff = NULL;
7739-
struct ly_set *set = NULL, *parent_set = NULL;
7740-
char *op_path = NULL;
7741-
int ret = EXIT_SUCCESS, r;
7742-
uint32_t i;
7738+
struct lyd_node *msg_sibling = NULL, *msg_parent = NULL, *data_tree_sibling, *data_tree_parent;
7739+
struct lys_node *msg_op = NULL;
7740+
struct ly_set *set;
7741+
int ret = EXIT_FAILURE;
77437742

7744-
assert(root && (*root || ctx) && act_notif && unres && !(options & LYD_OPT_ACT_NOTIF));
7743+
assert(root && (*root || ctx) && unres && !(options & LYD_OPT_ACT_NOTIF));
77457744

77467745
if (!ctx) {
77477746
ctx = (*root)->schema->module->ctx;
@@ -7757,18 +7756,21 @@ lyd_defaults_add_unres(struct lyd_node **root, int options, struct ly_ctx *ctx,
77577756
LOGERR(ctx, LY_EINVAL, "Cannot add default values to RPC, RPC reply, and notification without at least the empty container.");
77587757
return EXIT_FAILURE;
77597758
}
7760-
if ((options & LYD_OPT_RPC) && !*act_notif && ((*root)->schema->nodetype != LYS_RPC)) {
7759+
if ((options & LYD_OPT_RPC) && !act_notif && ((*root)->schema->nodetype != LYS_RPC)) {
77617760
LOGERR(ctx, LY_EINVAL, "Not valid RPC/action data.");
77627761
return EXIT_FAILURE;
77637762
}
7764-
if ((options & LYD_OPT_RPCREPLY) && !*act_notif && ((*root)->schema->nodetype != LYS_RPC)) {
7763+
if ((options & LYD_OPT_RPCREPLY) && !act_notif && ((*root)->schema->nodetype != LYS_RPC)) {
77657764
LOGERR(ctx, LY_EINVAL, "Not valid reply data.");
77667765
return EXIT_FAILURE;
77677766
}
7768-
if ((options & LYD_OPT_NOTIF) && !*act_notif && ((*root)->schema->nodetype != LYS_NOTIF)) {
7767+
if ((options & LYD_OPT_NOTIF) && !act_notif && ((*root)->schema->nodetype != LYS_NOTIF)) {
77697768
LOGERR(ctx, LY_EINVAL, "Not valid notification data.");
77707769
return EXIT_FAILURE;
77717770
}
7771+
7772+
/* remember the operation/notification schema */
7773+
msg_op = act_notif ? act_notif->schema : (*root)->schema;
77727774
} else if (*root && (*root)->parent) {
77737775
/* we have inner node, so it will be considered as
77747776
* a root of subtree where to add default nodes and
@@ -7777,7 +7779,7 @@ lyd_defaults_add_unres(struct lyd_node **root, int options, struct ly_ctx *ctx,
77777779
}
77787780

77797781
/* add missing default nodes */
7780-
if (wd && lyd_wd_add((*act_notif ? act_notif : root), ctx, modules, mod_count, unres, options)) {
7782+
if (wd && lyd_wd_add((act_notif ? &act_notif : root), ctx, modules, mod_count, unres, options)) {
77817783
return EXIT_FAILURE;
77827784
}
77837785

@@ -7788,118 +7790,118 @@ lyd_defaults_add_unres(struct lyd_node **root, int options, struct ly_ctx *ctx,
77887790
return EXIT_FAILURE;
77897791
}
77907792

7791-
/* create a merge of the additional data tree and the RPC/action/notification */
7793+
/* temporarily link the additional data tree to the RPC/action/notification */
77927794
if (data_tree && (options & (LYD_OPT_RPC | LYD_OPT_RPCREPLY | LYD_OPT_NOTIF))) {
7793-
/* merge data_tree into root but keep information for revert */
7794-
diff = lyd_diff(*root, (struct lyd_node *)data_tree, 0);
7795-
parent_set = ly_set_new();
7796-
if (!diff || !parent_set) {
7797-
LOGINT(ctx);
7798-
return EXIT_FAILURE;
7799-
}
7800-
for (i = 0; diff->type[i] != LYD_DIFF_END; ++i) {
7801-
if (diff->type[i] == LYD_DIFF_DELETED) {
7802-
/* uninteresting, but keep the correct index */
7803-
ly_set_add(parent_set, NULL, LY_SET_OPT_USEASLIST);
7804-
continue;
7805-
}
7795+
/* duplicate the message tree - if it gets deleted we would not be able to positively identify it */
7796+
msg_parent = NULL;
7797+
msg_sibling = *root;
7798+
7799+
if (act_notif) {
7800+
/* fun case */
7801+
data_tree_parent = NULL;
7802+
data_tree_sibling = (struct lyd_node *)data_tree;
7803+
while (data_tree_sibling) {
7804+
while (data_tree_sibling) {
7805+
if ((data_tree_sibling->schema == msg_sibling->schema)
7806+
&& ((msg_sibling->schema->nodetype != LYS_LIST)
7807+
|| lyd_list_equal(data_tree_sibling, msg_sibling, 0))) {
7808+
/* match */
7809+
break;
7810+
}
78067811

7807-
assert(diff->type[i] == LYD_DIFF_CREATED);
7808-
if (diff->second[i]->parent) {
7809-
assert(diff->first[i]);
7810-
/* remember the original parent */
7811-
ly_set_add(parent_set, diff->second[i]->parent, LY_SET_OPT_USEASLIST);
7812-
r = lyd_insert(diff->first[i], diff->second[i]);
7813-
} else {
7814-
assert(!diff->first[i]);
7815-
if (diff->second[i] == data_tree) {
7816-
assert(!diff->second[i]->prev->next);
7817-
if (diff->second[i]->next) {
7818-
/* remember the next sibling */
7819-
ly_set_add(parent_set, diff->second[i]->next, LY_SET_OPT_USEASLIST);
7820-
} else {
7821-
/* remember itself, there are no siblings */
7822-
ly_set_add(parent_set, diff->second[i], LY_SET_OPT_USEASLIST);
7812+
data_tree_sibling = data_tree_sibling->next;
7813+
}
7814+
7815+
if (data_tree_sibling) {
7816+
/* prepare for the new data_tree iteration */
7817+
data_tree_parent = data_tree_sibling;
7818+
data_tree_sibling = data_tree_sibling->child;
7819+
7820+
/* find new action sibling to search for later (skip list keys) */
7821+
msg_parent = msg_sibling;
7822+
assert(msg_sibling->child);
7823+
for (msg_sibling = msg_sibling->child;
7824+
msg_sibling->schema->nodetype == LYS_LEAF;
7825+
msg_sibling = msg_sibling->next) {
7826+
assert(msg_sibling->next);
7827+
}
7828+
if (msg_sibling->schema->nodetype & (LYS_ACTION | LYS_NOTIF)) {
7829+
/* we are done */
7830+
assert(act_notif->parent);
7831+
assert(act_notif->parent->schema == data_tree_parent->schema);
7832+
assert(msg_sibling == act_notif);
7833+
break;
78237834
}
7824-
} else {
7825-
/* remember the previous sibling */
7826-
assert(diff->second[i]->prev->next);
7827-
ly_set_add(parent_set, diff->second[i]->prev, LY_SET_OPT_USEASLIST);
78287835
}
7829-
r = lyd_insert_sibling(root, diff->second[i]);
78307836
}
7831-
if (r) {
7832-
ret = EXIT_FAILURE;
7833-
goto cleanup;
7837+
7838+
/* loop ended after the first iteration, set the values correctly */
7839+
if (!data_tree_parent) {
7840+
data_tree_sibling = (struct lyd_node *)data_tree;
7841+
}
7842+
7843+
} else {
7844+
/* easy case */
7845+
data_tree_parent = NULL;
7846+
data_tree_sibling = (struct lyd_node *)data_tree;
7847+
}
7848+
7849+
/* unlink msg_sibling if needed (won't do anything otherwise) */
7850+
lyd_unlink_internal(msg_sibling, 0);
7851+
7852+
/* now we can insert msg_sibling into data_tree_parent or next to data_tree_sibling */
7853+
assert(data_tree_parent || data_tree_sibling);
7854+
if (data_tree_parent) {
7855+
if (lyd_insert_common(data_tree_parent, NULL, msg_sibling, 0)) {
7856+
goto unlink_datatree;
7857+
}
7858+
} else {
7859+
assert(!data_tree_sibling->parent);
7860+
if (lyd_insert_nextto(data_tree_sibling->prev, msg_sibling, 0, 0)) {
7861+
goto unlink_datatree;
78347862
}
78357863
}
78367864
}
78377865

78387866
if (resolve_unres_data(ctx, unres, root, options)) {
7839-
ret = EXIT_FAILURE;
7867+
goto unlink_datatree;
78407868
}
78417869

7870+
/* we are done */
7871+
ret = EXIT_SUCCESS;
7872+
7873+
/* check that the operation/notification tree was not removed */
78427874
if (options & (LYD_OPT_RPC | LYD_OPT_RPCREPLY | LYD_OPT_NOTIF)) {
7875+
set = NULL;
78437876
if (data_tree) {
7844-
/* first unlink all data tree subtrees */
7845-
assert(diff->type[parent_set->number] == LYD_DIFF_END);
7846-
for (i = 0; i < parent_set->number; ++i) {
7847-
if (!parent_set->set.d[i]) {
7848-
continue;
7849-
}
7850-
lyd_unlink(diff->second[i]);
7851-
}
7852-
7853-
/* put the operation tree back how it was */
7854-
i = parent_set->number;
7855-
do {
7856-
--i;
7857-
if (!parent_set->set.d[i]) {
7858-
continue;
7859-
}
7860-
7861-
/* it was unlinked, so we cannot check the parent directly */
7862-
if (diff->first[i]) {
7863-
r = lyd_insert(parent_set->set.d[i], diff->second[i]);
7864-
} else {
7865-
if (diff->second[i] == data_tree) {
7866-
if (parent_set->set.d[i] == diff->second[i]) {
7867-
/* already unlinked */
7868-
r = 0;
7869-
} else {
7870-
r = lyd_insert_before(parent_set->set.d[i], diff->second[i]);
7871-
}
7872-
} else {
7873-
r = lyd_insert_after(parent_set->set.d[i], diff->second[i]);
7874-
}
7875-
}
7876-
if (r) {
7877-
ret = EXIT_FAILURE;
7878-
goto cleanup;
7879-
}
7880-
} while (i);
7881-
}
7882-
7883-
/* check that the operation/notification tree was not removed */
7884-
op_path = lys_data_path(*act_notif ? (*act_notif)->schema : (*root)->schema);
7885-
if (*root) {
7886-
set = lyd_find_path(*root, op_path);
7877+
set = lyd_find_instance(data_tree_parent ? data_tree_parent : data_tree_sibling, msg_op);
7878+
assert(set && ((set->number == 0) || (set->number == 1)));
7879+
} else if (*root) {
7880+
set = lyd_find_instance(*root, msg_op);
78877881
assert(set && ((set->number == 0) || (set->number == 1)));
78887882
}
78897883
if (!set || !set->number) {
7890-
/* it was removed */
7891-
LOGVAL(ctx, LYE_SPEC, LY_VLOG_LYS, op_path, "Operation/notification not supported because of the current configuration.");
7884+
/* it was removed, handle specially */
7885+
LOGVAL(ctx, LYE_SPEC, LY_VLOG_LYS, msg_op, "Operation/notification not supported because of the current configuration.");
78927886
ret = EXIT_FAILURE;
7893-
goto cleanup;
7887+
}
7888+
ly_set_free(set);
7889+
}
7890+
7891+
unlink_datatree:
7892+
/* put the trees back in order */
7893+
if (data_tree && (options & (LYD_OPT_RPC | LYD_OPT_RPCREPLY | LYD_OPT_NOTIF))) {
7894+
/* unlink and insert it back, if there is a parent */
7895+
lyd_unlink_internal(msg_sibling, 0);
7896+
if (msg_parent) {
7897+
lyd_insert_common(msg_parent, NULL, msg_sibling, 0);
78947898
}
78957899
}
7900+
} else {
7901+
/* we are done */
7902+
ret = EXIT_SUCCESS;
78967903
}
78977904

7898-
cleanup:
7899-
free(op_path);
7900-
ly_set_free(set);
7901-
lyd_free_diff(diff);
7902-
ly_set_free(parent_set);
79037905
return ret;
79047906
}
79057907

0 commit comments

Comments
 (0)