Skip to content

Fix spurious cascading errors on when-false nodes in multi-error validation #2528

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.

Sign up for GitHub

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

Open
szabolcs-szekely wants to merge 4 commits into
base: devel
Choose a base branch
from
+340 −11
Open

Fix spurious cascading errors on when-false nodes in multi-error validation #2528

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f36af5
validation BUGFIX multi-error when LY_EINCOMPLETE crash and spurious …
szabolcs-szekely May 8, 2026
5219b0b
tests UPDATE when-false keyed list via hash-child XPath lookup
szabolcs-szekely May 11, 2026
2b51f7e
validation BUGFIX when-false review fixes
szabolcs-szekely Jun 3, 2026
f78b246
validation REFACTOR when-false handling per review
szabolcs-szekely Jun 4, 2026
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
5 changes: 5 additions & 0 deletions src/tree_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,8 @@ struct lyd_attr {
* 3 LYD_NEW |x|x|x|x|x|x|x|
* +-+-+-+-+-+-+-+
* 4 LYD_EXT |x|x|x|x|x|x|x|
* +-+-+-+-+-+-+-+
* 5 LYD_WHEN_FALSE |x|x|x|x|x| | |
* ---------------------+-+-+-+-+-+-+-+
*
*/
Expand All @@ -798,6 +800,9 @@ struct lyd_attr {
#define LYD_WHEN_TRUE 0x02 /**< all when conditions of this node were evaluated to true */
#define LYD_NEW 0x04 /**< node was created after the last validation, is needed for the next validation */
#define LYD_EXT 0x08 /**< node is the first sibling parsed as extension instance data */
#define LYD_WHEN_FALSE 0x10 /**< when condition of this node was evaluated to false; the node is kept in the
tree so multi-error validation can continue, but XPath evaluation must treat
the node as non-existent */

/** @} */

Expand Down
66 changes: 58 additions & 8 deletions src/validation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,13 +436,52 @@ lyd_validate_unres_when(struct lyd_node **tree, const struct lys_module *mod, st
/* only a warning */
LOGWRN(LYD_CTX(node), "When condition \"%s\" not satisfied.", disabled->cond->expr);
} else {
/* invalid data */
/* invalid data; mark the when as resolved-to-false so subsequent XPath
* evaluations on this node return "no match" instead of LY_EINCOMPLETE */
node->flags |= LYD_WHEN_FALSE;
/* remove descendants from node_types so their leafrefs are not validated
* against the when-false subtree, which would produce spurious cascading
* errors (mirrors the cleanup done by lyd_validate_autodel_node_del) */
if (node_types && node_types->count) {
struct lyd_node *iter;
uint32_t idx;

LYD_TREE_DFS_BEGIN(node, iter) {
if ((iter->schema->nodetype & LYD_NODE_TERM) &&
LYSC_GET_TYPE_PLG(((struct lysc_node_leaf *)iter->schema)->type->plugin_ref)->validate_tree &&
ly_set_contains(node_types, iter, &idx)) {
ly_set_rm_index(node_types, idx, NULL);
}
LYD_TREE_DFS_END(node, iter);
}
}
/* remove descendants from node_when so their own when conditions are not
* evaluated; a when-false subtree is logically non-existent, so child
* whens are moot and would otherwise produce spurious cascading errors */
if (node_when->count > 1) {
struct lyd_node *iter;
uint32_t idx;

count = node_when->count;
LYD_TREE_DFS_BEGIN(node, iter) {
if ((iter != node) && ly_set_contains(node_when, iter, &idx)) {
ly_set_rm_index_ordered(node_when, idx, NULL);
}
LYD_TREE_DFS_END(node, iter);
}
if (count > node_when->count) {
/* descendants were removed, refresh the iteration index */
ly_set_contains(node_when, node, &i);
}
}
Copy link
Copy Markdown
Member

@michalvasko michalvasko Jun 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this code, the function is too long, please put this into a separate function. Also, it may impact performance so why not use the new flag LYD_WHEN_FALSE only for LYD_VALIDATE_MULTI_ERROR validation?

Copy link
Copy Markdown
Author

@szabolcs-szekely szabolcs-szekely Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I separated the function and attached the LYD_WHEN_FALSE flag only for the LYD_VALIDATE_MULTI_ERROR mode with commit f78b246

LOGVAL(LYD_CTX(node), node, LY_VCODE_NOWHEN, disabled->cond->expr);
r = LY_EVALID;
LY_VAL_ERR_GOTO(r, rc = r, val_opts, cleanup);
}
} else {
/* when true */
/* when true; clear any stale when-false mark from a previous validation run
* so XPath no longer treats the node as non-existent */
node->flags &= ~LYD_WHEN_FALSE;
node->flags |= LYD_WHEN_TRUE;
}

Expand Down Expand Up @@ -1774,6 +1813,13 @@ lyd_validate_final_r(struct lyd_node *first, const struct lyd_node *parent, cons
goto next_iter;
}

/* skip further validation for when-false nodes: the node is logically non-existent,
* so its own must constraints and obsolete checks must not run */
if (node->flags & LYD_WHEN_FALSE) {
r = LY_SUCCESS;
goto next_iter;
}

/* obsolete data */
lyd_validate_obsolete(node);

Expand Down Expand Up @@ -1803,13 +1849,17 @@ lyd_validate_final_r(struct lyd_node *first, const struct lyd_node *parent, cons
break;
}

/* validate all children recursively */
r = lyd_validate_final_r(lyd_child(node), node, node->schema, NULL, ext, val_opts,
int_opts & ~LYD_INTOPT_SKIP_SIBLINGS, must_xp_opts, getnext_ht);
LY_VAL_ERR_GOTO(r, rc = r, val_opts, cleanup);
/* skip when-false nodes: they are logically non-existent, so neither their children
* nor their container-default handling must run */
if (!(node->flags & LYD_WHEN_FALSE)) {
/* validate all children recursively */
r = lyd_validate_final_r(lyd_child(node), node, node->schema, NULL, ext, val_opts,
int_opts & ~LYD_INTOPT_SKIP_SIBLINGS, must_xp_opts, getnext_ht);
LY_VAL_ERR_GOTO(r, rc = r, val_opts, cleanup);

/* set default for containers */
lyd_np_cont_dflt_set(node);
/* set default for containers */
lyd_np_cont_dflt_set(node);
}

if (int_opts & LYD_INTOPT_SKIP_SIBLINGS) {
break;
Expand Down
15 changes: 12 additions & 3 deletions src/xpath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5861,10 +5861,14 @@ moveto_node_check(const struct lyd_node *node, enum lyxp_node_type node_type, co
}

/* when check, accept the context node because it should only be the path ".", we have checked the when is valid before */
if (!(options & LYXP_IGNORE_WHEN) && lysc_has_when(schema) && !(node->flags & LYD_WHEN_TRUE) &&
(node != set->cur_node)) {
if (!(options & LYXP_IGNORE_WHEN) && lysc_has_when(schema) &&
!(node->flags & (LYD_WHEN_TRUE | LYD_WHEN_FALSE)) && (node != set->cur_node)) {
return LY_EINCOMPLETE;
}
if ((node->flags & LYD_WHEN_FALSE) && (node != set->cur_node)) {
/* when-false node is treated as non-existent for XPath purposes */
return LY_ENOT;
}

/* match */
return LY_SUCCESS;
Expand Down Expand Up @@ -6372,10 +6376,15 @@ moveto_node_hash_child(struct lyxp_set *set, const struct lysc_node *scnode, con
LY_CHECK_ERR_GOTO(r && (r != LY_ENOTFOUND), ret = r, cleanup);

/* when check */
if (!(options & LYXP_IGNORE_WHEN) && sub && lysc_has_when(sub->schema) && !(sub->flags & LYD_WHEN_TRUE)) {
if (!(options & LYXP_IGNORE_WHEN) && sub && lysc_has_when(sub->schema) &&
!(sub->flags & (LYD_WHEN_TRUE | LYD_WHEN_FALSE))) {
ret = LY_EINCOMPLETE;
goto cleanup;
}
if (sub && (sub->flags & LYD_WHEN_FALSE)) {
/* when-false node is treated as non-existent */
sub = NULL;
}

if (sub) {
/* pos filled later */
Expand Down
Loading
Loading