diff --git a/src/tree_data.h b/src/tree_data.h index f2dd0f307..badbd77e8 100644 --- a/src/tree_data.h +++ b/src/tree_data.h @@ -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| | | * ---------------------+-+-+-+-+-+-+-+ * */ @@ -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 */ /** @} */ diff --git a/src/validation.c b/src/validation.c index 67cabe23f..c0ab51214 100644 --- a/src/validation.c +++ b/src/validation.c @@ -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); + } + } 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; } @@ -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); @@ -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; diff --git a/src/xpath.c b/src/xpath.c index 1869eefec..5c2874c93 100644 --- a/src/xpath.c +++ b/src/xpath.c @@ -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; @@ -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 */ diff --git a/tests/utests/data/test_validation.c b/tests/utests/data/test_validation.c index 6ea851a90..7ecaeb5a4 100644 --- a/tests/utests/data/test_validation.c +++ b/tests/utests/data/test_validation.c @@ -1632,6 +1632,248 @@ test_store_only(void **state) lyd_free_siblings(tree); } +static void +test_when_must_cross_ref(void **state) +{ + /* Regression: when one node's "when" evaluates to false, references to it + * from a sibling's "must" (or another "when") must NOT bubble up + * LY_EINCOMPLETE -- the when-false node should be treated as non-existent + * by XPath. Before the LYD_WHEN_FALSE flag was introduced, this scenario + * either suppressed the second error (must skipped under multi-error) or + * crashed with "Internal error (validation.c:1058)" when a dummy-when + * evaluation hit the same chain. */ + struct lyd_node *tree; + const char *schema = + "module twm {\n" + " namespace urn:tests:twm;\n" + " prefix twm;\n" + " yang-version 1.1;\n" + "\n" + " container top {\n" + " leaf flag {\n" + " type boolean;\n" + " default \"false\";\n" + " }\n" + " leaf gated {\n" + " when \"../flag = 'true'\";\n" + " type string;\n" + " }\n" + " leaf checker {\n" + " must \"/twm:top/twm:gated = 'expected'\" {\n" + " error-app-tag \"checker-bad\";\n" + " error-message \"checker requires gated=expected\";\n" + " }\n" + " type string;\n" + " }\n" + " leaf dep-when {\n" + " when \"/twm:top/twm:gated = 'expected'\";\n" + " mandatory true;\n" + " type string;\n" + " }\n" + " }\n" + "}"; + const char *data = + "\n" + " x\n" + " val\n" + "\n"; + + UTEST_ADD_MODULE(schema, LYS_IN_YANG, NULL, NULL); + + /* with multi-error: both the when failure and the cross-referencing must + * failure must be reported, and validation must NOT crash with an + * internal error from lyd_validate_dummy_when (the mandatory dep-when + * leaf would otherwise trigger LY_EINCOMPLETE there) */ + CHECK_PARSE_LYD_PARAM(data, LYD_XML, 0, + LYD_VALIDATE_PRESENT | LYD_VALIDATE_MULTI_ERROR, LY_EVALID, tree); + CHECK_LOG_CTX_APPTAG("checker requires gated=expected", "/twm:top/checker", 0, "checker-bad"); + CHECK_LOG_CTX("When condition \"../flag = 'true'\" not satisfied.", "/twm:top/gated", 0); + + /* Regression: a leafref inside a when-false container must NOT produce a + * spurious "Invalid leafref value" error. Before the node_types cleanup in + * lyd_validate_unres_when was added, XPath navigation from the leafref leaf + * upward through the LYD_WHEN_FALSE parent returned LY_ENOT, making the + * target instance unreachable and triggering a false leafref failure. */ + const char *schema2 = + "module twm2 {\n" + " namespace urn:tests:twm2;\n" + " prefix twm2;\n" + " yang-version 1.1;\n" + "\n" + " container top {\n" + " leaf flag {\n" + " type boolean;\n" + " }\n" + " list item {\n" + " key \"name\";\n" + " leaf name { type string; }\n" + " }\n" + " container gated {\n" + " when \"../flag = 'true'\";\n" + " leaf ref {\n" + " type leafref {\n" + " path \"../../item/name\";\n" + " }\n" + " }\n" + " }\n" + " }\n" + "}"; + const char *data2 = + "\n" + " false\n" + " x\n" + " x\n" + "\n"; + + UTEST_ADD_MODULE(schema2, LYS_IN_YANG, NULL, NULL); + + /* only the when error must be reported; no spurious leafref error */ + CHECK_PARSE_LYD_PARAM(data2, LYD_XML, 0, + LYD_VALIDATE_PRESENT | LYD_VALIDATE_MULTI_ERROR, LY_EVALID, tree); + CHECK_LOG_CTX("When condition \"../flag = 'true'\" not satisfied.", "/twm2:top/gated", 0); + + /* Regression: a leaf that itself carries both a when condition and a + * leafref type must NOT produce a spurious leafref error when its own + * when evaluates to false. The leaf is the LYD_WHEN_FALSE node, not a + * descendant of one, so the node_types cleanup must include the node + * itself (not only its descendants). Mirrors the imsdc application-stream + * pattern where current()/../ in the path also becomes + * non-existent once that sibling's when is false. */ + const char *schema3 = + "module twm3 {\n" + " namespace urn:tests:twm3;\n" + " prefix twm3;\n" + " yang-version 1.1;\n" + "\n" + " container top {\n" + " leaf flag {\n" + " type boolean;\n" + " }\n" + " list item {\n" + " key \"name\";\n" + " leaf name { type string; }\n" + " }\n" + " leaf selector {\n" + " when \"../flag = 'true'\";\n" + " type string;\n" + " }\n" + " leaf ref {\n" + " when \"../flag = 'true'\";\n" + " type leafref {\n" + " path \"/twm3:top/twm3:item[twm3:name = current()/../selector]/twm3:name\";\n" + " }\n" + " }\n" + " }\n" + "}"; + const char *data3 = + "\n" + " false\n" + " x\n" + " x\n" + " x\n" + "\n"; + + UTEST_ADD_MODULE(schema3, LYS_IN_YANG, NULL, NULL); + + /* only the when errors must be reported; no spurious leafref error for ref */ + CHECK_PARSE_LYD_PARAM(data3, LYD_XML, 0, + LYD_VALIDATE_PRESENT | LYD_VALIDATE_MULTI_ERROR, LY_EVALID, tree); + CHECK_LOG_CTX("When condition \"../flag = 'true'\" not satisfied.", "/twm3:top/selector", 0); + CHECK_LOG_CTX("When condition \"../flag = 'true'\" not satisfied.", "/twm3:top/ref", 0); + + /* Regression: explicitly exercise the hash-based child lookup path + * (moveto_node_hash_child in src/xpath.c) for a when-false KEYED LIST. The + * other sub-cases reach moveto_node_check via plain name steps; this one + * forces XPath through the optimized [key='...'] lookup. The data + * deliberately sets item[name='a']/value to 'expected' so that, if the + * when-false flag were NOT honored by the hash lookup, the must would + * incorrectly pass. With LYD_WHEN_FALSE the list instance is invisible to + * XPath, the value comparison is against an empty node-set, and the must + * must fail. */ + const char *schema4 = + "module twm4 {\n" + " namespace urn:tests:twm4;\n" + " prefix twm4;\n" + " yang-version 1.1;\n" + "\n" + " container top {\n" + " leaf flag {\n" + " type boolean;\n" + " }\n" + " list item {\n" + " when \"../flag = 'true'\";\n" + " key \"name\";\n" + " leaf name { type string; }\n" + " leaf value { type string; }\n" + " }\n" + " leaf checker {\n" + " must \"/twm4:top/twm4:item[twm4:name = 'a']/twm4:value = 'expected'\" {\n" + " error-app-tag \"checker-bad\";\n" + " error-message \"checker requires item[a]/value=expected\";\n" + " }\n" + " type string;\n" + " }\n" + " }\n" + "}"; + const char *data4 = + "\n" + " false\n" + " aexpected\n" + " val\n" + "\n"; + + UTEST_ADD_MODULE(schema4, LYS_IN_YANG, NULL, NULL); + + /* both errors must be reported: the when-false on the list and the must + * failure on checker (the hash lookup must treat item[name='a'] as + * non-existent despite value='expected' being physically present). Note: + * CHECK_LOG_CTX pops via ly_err_last, so assertions are in reverse order. */ + CHECK_PARSE_LYD_PARAM(data4, LYD_XML, 0, + LYD_VALIDATE_PRESENT | LYD_VALIDATE_MULTI_ERROR, LY_EVALID, tree); + CHECK_LOG_CTX_APPTAG("checker requires item[a]/value=expected", "/twm4:top/checker", 0, "checker-bad"); + CHECK_LOG_CTX("When condition \"../flag = 'true'\" not satisfied.", "/twm4:top/item[name='a']", 0); + + /* Regression: a when-false node with a descendant that also carries a when + * must NOT emit a spurious when error for the descendant. The descendant's + * when references its when-gated ancestor, so its evaluation returns + * LY_EINCOMPLETE and it stays queued in node_when until the ancestor is + * resolved to false. The node_when descendant cleanup in + * lyd_validate_unres_when then removes it before it can be (re-)evaluated. + * Without that cleanup, the descendant's own when is evaluated against the + * logically non-existent subtree and a second, spurious when error is + * reported. Only the ancestor's when error is expected. */ + const char *schema5 = + "module twm5 {\n" + " namespace urn:tests:twm5;\n" + " prefix twm5;\n" + " yang-version 1.1;\n" + "\n" + " container top {\n" + " leaf flag {\n" + " type boolean;\n" + " }\n" + " container outer {\n" + " when \"../flag = 'true'\";\n" + " leaf inner {\n" + " when \"../../flag = 'true'\";\n" + " type string;\n" + " }\n" + " }\n" + " }\n" + "}"; + const char *data5 = + "\n" + " false\n" + " x\n" + "\n"; + + UTEST_ADD_MODULE(schema5, LYS_IN_YANG, NULL, NULL); + + CHECK_PARSE_LYD_PARAM(data5, LYD_XML, 0, + LYD_VALIDATE_PRESENT | LYD_VALIDATE_MULTI_ERROR, LY_EVALID, tree); + CHECK_LOG_CTX("When condition \"../flag = 'true'\" not satisfied.", "/twm5:top/outer", 0); +} + int main(void) { @@ -1650,6 +1892,7 @@ main(void) UTEST(test_state), UTEST(test_must), UTEST(test_multi_error), + UTEST(test_when_must_cross_ref), UTEST(test_action), UTEST(test_rpc), UTEST(test_reply),