union UPDATE unions with leafrefs returning misleading error codes (#2360)

* validate: unions with leafrefs may return misleading error codes

When a leafref is not found in a union, it should not return an error
indicating the value for the leafref wouldn't match the restrictions
for a target but instead just say the leafref target wasn't found.

The only data that should indicate the error message for data type
restrictions is data associated with the target itself, not leafrefs.

Signed-off-by: Brad House <[email protected]>

* fixes as per @michalvasko

* do not use lyd_value()

* reference RFC in doxygen

* error checking for OOM

---------

Signed-off-by: Brad House <[email protected]>

Author: bradh352

src/plugins_types/union.c

@@ -150,6 +150,50 @@ lyb_parse_union(const void *lyb_data, size_t lyb_data_len, uint32_t *type_idx, c
     }
 }
 
+/**
+ * @brief For leafref failures, ensure the appropriate error is propagated, not a type validation failure.
+ *
+ * RFC7950 Section 15.5 defines the appropriate error app tag of "require-instance".
+ *
+ * @param[in,out] err Error record to be updated
+ * @param[in] type leafref type used to extract target path
+ * @param[in] value value attempting to be stored
+ * @param[in] value_len Length of value that was attempted to be stored.
+ * @return LY_ERR value. Only possible errors are LY_SUCCESS and LY_EMEM.
+ */
+static LY_ERR
+union_update_lref_err(struct ly_err_item *err, const struct lysc_type *type, const void *value, size_t value_len)
+{
+    const struct lysc_type_leafref *lref;
+    char *valstr = NULL;
+    int msg_len;
+
+    if ((err == NULL) || (type->basetype != LY_TYPE_LEAFREF)) {
+        return LY_SUCCESS;
+    }
+
+    lref = (const struct lysc_type_leafref *)type;
+    free(err->apptag);
+    err->apptag = strdup("instance-required");
+    if (err->apptag == NULL) {
+        return LY_EMEM;
+    }
+
+    free(err->msg);
+    err->msg = NULL;
+    valstr = strndup((const char *)value, value_len);
+    if (valstr == NULL) {
+        return LY_EMEM;
+    }
+
+    msg_len = asprintf(&err->msg, LY_ERRMSG_NOLREF_VAL, valstr, lyxp_get_expr(lref->path));
+    free(valstr);
+    if (msg_len == -1) {
+        return LY_EMEM;
+    }
+    return LY_SUCCESS;
+}
+
 /**
  * @brief Store (and validate) subvalue as a specific type.
  *
@@ -170,7 +214,7 @@ union_store_type(const struct ly_ctx *ctx, struct lysc_type_union *type_u, uint3
         uint32_t options, ly_bool validate, const struct lyd_node *ctx_node, const struct lyd_node *tree,
         struct lys_glob_unres *unres, struct ly_err_item **err)
 {
-    LY_ERR rc = LY_SUCCESS;
+    LY_ERR urc, rc = LY_SUCCESS;
     struct lysc_type *type = type_u->types[type_idx];
     const void *value = NULL;
     size_t value_len = 0;
@@ -190,7 +234,13 @@ union_store_type(const struct ly_ctx *ctx, struct lysc_type_union *type_u, uint3
             if ((rc != LY_SUCCESS) && (rc != LY_EINCOMPLETE)) {
                 /* clear any leftover/freed garbage */
                 memset(&subvalue->value, 0, sizeof subvalue->value);
-                return rc;
+
+                /* if this is a leafref, lets make sure we propagate the appropriate error, and not a type validation failure */
+                urc = union_update_lref_err(*err, type_u->types[ti], value, value_len);
+                if (urc != LY_SUCCESS) {
+                    rc = urc;
+                }
+                goto cleanup;
             }
 
             assert(subvalue->value.realtype);
@@ -219,16 +269,19 @@ union_store_type(const struct ly_ctx *ctx, struct lysc_type_union *type_u, uint3
     if (options & LYPLG_TYPE_STORE_ONLY) {
         opts |= LYPLG_TYPE_STORE_ONLY;
     }
-    if (dynamic) {
-        opts |= LYPLG_TYPE_STORE_DYNAMIC;
-    }
 
     rc = type->plugin->store(ctx, type, value, value_len, opts, format, prefix_data, subvalue->hints,
             subvalue->ctx_node, &subvalue->value, unres, err);
     if ((rc != LY_SUCCESS) && (rc != LY_EINCOMPLETE)) {
         /* clear any leftover/freed garbage */
         memset(&subvalue->value, 0, sizeof subvalue->value);
-        return rc;
+
+        /* if this is a leafref, lets make sure we propagate the appropriate error, and not a type validation failure */
+        urc = union_update_lref_err(*err, type, value, value_len);
+        if (urc != LY_SUCCESS) {
+            rc = urc;
+        }
+        goto cleanup;
     }
 
     if (validate && (rc == LY_EINCOMPLETE)) {
@@ -240,6 +293,10 @@ union_store_type(const struct ly_ctx *ctx, struct lysc_type_union *type_u, uint3
         }
     }
 
+cleanup:
+    if (dynamic) {
+        free((void *)value);
+    }
     return rc;
 }
 

tests/utests/types/union.c

@@ -105,8 +105,8 @@ test_data_xml(void **state)
     TEST_ERROR_XML2("",
             "defs", "", "un1", "123456789012345678901", LY_EVALID);
     CHECK_LOG_CTX("Invalid union value \"123456789012345678901\" - no matching subtype found:\n"
-            "    libyang 2 - leafref, version 1: Invalid type int8 value \"123456789012345678901\".\n"
-            "    libyang 2 - leafref, version 1: Invalid type int64 value \"123456789012345678901\".\n"
+            "    libyang 2 - leafref, version 1: Invalid leafref value \"123456789012345678901\" - no target instance \"/int8\" with the same value.\n"
+            "    libyang 2 - leafref, version 1: Invalid leafref value \"123456789012345678901\" - no target instance \"/int64\" with the same value.\n"
             "    libyang 2 - identityref, version 1: Invalid identityref \"123456789012345678901\" value - identity not found in module \"defs\".\n"
             "    libyang 2 - instance-identifier, version 1: Invalid instance-identifier \"123456789012345678901\" value - syntax error.\n"
             "    libyang 2 - string, version 1: Unsatisfied length - string \"123456789012345678901\" length is not allowed.\n",
@@ -295,7 +295,8 @@ test_validation(void **state)
     assert_int_equal(LY_EVALID, lyd_validate_all(&tree, NULL, LYD_VALIDATE_PRESENT, NULL));
     CHECK_LOG_CTX("Invalid LYB union value - no matching subtype found:\n"
             "    libyang 2 - leafref, version 1: Invalid leafref value \"one\" - no target instance \"../../a/name\" with the same value.\n"
-            "    libyang 2 - leafref, version 1: Invalid type uint32 value \"one\".\n", "/lref:test/community[name='test']/view", 0);
+            "    libyang 2 - leafref, version 1: Invalid leafref value \"one\" - no target instance \"../../b/name\" with the same value.\n",
+            "/lref:test/community[name='test']/view", 0);
 
     lyd_free_all(tree);
 }