From 6d46087109160080e41688d11fe3dd13459943c0 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 11 Jul 2019 12:11:45 +0200 Subject: [PATCH] Documented bug [Yang identityref XML encoding is not general #90](https://github.com/clicon/clixon/issues/90) --- CHANGELOG.md | 1 + apps/cli/cli_generate.c | 51 ++++++++++++++++++++++++---------- include/clixon_custom.h | 14 ++++++---- lib/src/clixon_xml_map.c | 43 ++++++++++++++++++++-------- lib/src/clixon_yang.c | 19 ++++--------- lib/src/clixon_yang_internal.h | 2 +- 6 files changed, 84 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab2e4a37..22304f06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -217,6 +217,7 @@ ### Minor changes +* Documented bug [Yang identityref XML encoding is not general #90](https://github.com/clicon/clixon/issues/90) * Added new API function `xpath_parse()` to split parsing and xml evaluation. * Rewrote `api_path2xpath` to handle namespaces. * `api_path2xml_vec` strict mode check added if list key length mismatch diff --git a/apps/cli/cli_generate.c b/apps/cli/cli_generate.c index 1d9400d3..ee89b2e9 100644 --- a/apps/cli/cli_generate.c +++ b/apps/cli/cli_generate.c @@ -181,33 +181,56 @@ yang2cli_var_identityref(yang_stmt *ys, yang_stmt *ybaseref; yang_stmt *ybaseid; cg_var *cv = NULL; - char *name; - char *id; + char *prefix = NULL; + char *id = NULL; int i; + cvec *idrefvec; + yang_stmt *ymod; + yang_stmt *yprefix; + yang_stmt *yspec; if ((ybaseref = yang_find(ytype, Y_BASE, NULL)) != NULL && (ybaseid = yang_find_identity(ys, yang_argument_get(ybaseref))) != NULL){ - if (cvec_len(yang_cvec_get(ybaseid)) > 0){ + idrefvec = yang_cvec_get(ybaseid); + if (cvec_len(idrefvec) > 0){ /* Add a wildchar string first -let validate take it for default prefix */ cprintf(cb, ">"); if (helptext) cprintf(cb, "(\"%s\")", helptext); cprintf(cb, "|<%s:%s choice:", yang_argument_get(ys), cvtypestr); + yspec = ys_spec(ys); i = 0; - while ((cv = cvec_each(yang_cvec_get(ybaseid), cv)) != NULL){ - if (i++) - cprintf(cb, "|"); - name = strdup(cv_name_get(cv)); - if ((id=strchr(name, ':')) != NULL) - *id = '\0'; - cprintf(cb, "%s:%s", name, id+1); - if (name) - free(name); - } + while ((cv = cvec_each(idrefvec, cv)) != NULL){ + if (nodeid_split(cv_name_get(cv), &prefix, &id) < 0) + goto done; + /* Translate from module-name(prefix) to global prefix + * This is really a kludge for true identityref prefix handling + * IDENTITYREF_KLUDGE + * This is actually quite complicated: the cli needs to generate + * a netconf statement with correct xmlns binding + */ + if ((ymod = yang_find_module_by_name(yspec, prefix)) != NULL && + (yprefix = yang_find(ymod, Y_PREFIX, NULL)) != NULL) + if (i++) + cprintf(cb, "|"); + cprintf(cb, "%s:%s", yang_argument_get(yprefix), id); + } + if (prefix){ + free(prefix); + prefix = NULL; + } + if (id){ + free(id); + id = NULL; + } } } retval = 0; - // done: + done: + if (prefix) + free(prefix); + if (id) + free(id); return retval; } diff --git a/include/clixon_custom.h b/include/clixon_custom.h index b36a54ab..424790fb 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -48,9 +48,13 @@ */ #define USE_NETCONF_NS_AS_DEFAULT -/* Use modulename:id instead of prefix:id in derived identityref list - * Modulenames are global/canonical but prefixes are not. - * Experimental since there is some mapping between prefixes and module names - * that needs to be done +/*! Tag for wrong handling of identityref prefixes (XML encoding) + * See https://github.com/clicon/clixon/issues/90 + * Instead of using generic xmlns prefix bindings, the module's own prefix + * is used. + * In the CLI generation case, this is actually quite complicated: the cli + * needs to generate a netconf statement with correct xmlns binding. + * The easy way to do this is to always generate all prefix/namespace bindings + * on the top-level for the modules involved in the netconf operation. */ -#undef USE_IDREF_LIST_MODULE +#undef IDENTITYREF_KLUDGE diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 04d8c020..40b17970 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -330,6 +330,25 @@ validate_leafref(cxobj *xt, goto done; } +/* Get module from its own prefix + * This is really not a valid usecase, a kludge for the identityref derived + * list workaround (IDENTITYREF_KLUDGE) + */ +static yang_stmt * +yang_find_module_by_prefix_yspec(yang_stmt *yspec, + char *prefix) +{ + yang_stmt *ymod = NULL; + yang_stmt *yprefix; + + while ((ymod = yn_each(yspec, ymod)) != NULL) + if (ymod->ys_keyword == Y_MODULE && + (yprefix = yang_find(ymod, Y_PREFIX, NULL)) != NULL && + strcmp(yang_argument_get(yprefix), prefix) == 0) + return ymod; + return NULL; +} + /*! Validate xml node of type identityref, ensure value is a defined identity * Check if a given node has value derived from base identity. This is * a run-time check necessary when validating eg netconf. @@ -367,6 +386,7 @@ validate_identityref(cxobj *xt, char *id = NULL; cbuf *cberr = NULL; cbuf *cb = NULL; + cvec *idrefvec; /* Derived identityref list: (module:id)**/ if ((cb = cbuf_new()) == NULL){ clicon_err(OE_UNIX, errno, "cbuf_new"); @@ -395,13 +415,15 @@ validate_identityref(cxobj *xt, goto fail; } -#if 0 /* Assume proper namespace, otherwise we assume module prefixes */ +#if 0 /* Assume proper namespace, otherwise we assume module prefixes, + * see IDENTITYREF_KLUDGE + */ { char *namespace; yang_stmt *ymod; yang_stmt *yspec; - /* Create an idref as : which is the format of the derived + /* Create an idref as : which is the format of the derived * identityref list associated with the base identities. */ /* Get namespace (of idref) from xml */ @@ -416,14 +438,16 @@ validate_identityref(cxobj *xt, cprintf(cb, "%s:%s", yang_argument_get(ymod), id); } #else -#ifdef USE_IDREF_LIST_MODULE { yang_stmt *ymod; /* idref from prefix:id to module:id */ if (prefix == NULL) ymod = ys_module(ys); - else /* from prefix to name */ - ymod = yang_find_module_by_prefix(ys, prefix); + else{ /* from prefix to name */ +#if 1 /* IDENTITYREF_KLUDGE */ + ymod = yang_find_module_by_prefix_yspec(ys_spec(ys), prefix); +#endif + } if (ymod == NULL){ cprintf(cberr, "Identityref validation failed, %s not derived from %s", node, yang_argument_get(ybaseid)); @@ -433,18 +457,13 @@ validate_identityref(cxobj *xt, } cprintf(cb, "%s:%s", yang_argument_get(ymod), id); } -#else - if (prefix == NULL) - cprintf(cb, "%s:%s", yang_find_myprefix(ys), id); - else - cprintf(cb, "%s:%s", prefix, id); -#endif #endif idref = cbuf_get(cb); /* Here check if node is in the derived node list of the base identity * The derived node list is a cvec computed XXX */ - if (cvec_find(yang_cvec_get(ybaseid), idref) == NULL){ + idrefvec = yang_cvec_get(ybaseid); + if (cvec_find(idrefvec, idref) == NULL){ cprintf(cberr, "Identityref validation failed, %s not derived from %s", node, yang_argument_get(ybaseid)); if (netconf_operation_failed_xml(xret, "application", cbuf_get(cberr)) < 0) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 656c889e..e31a2a72 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -1616,12 +1616,13 @@ ys_populate_identity(clicon_handle h, int retval = -1; yang_stmt *yc = NULL; yang_stmt *ybaseid; - // yang_stmt *ymod; cg_var *cv; char *baseid; char *prefix = NULL; char *id = NULL; cbuf *cb = NULL; + yang_stmt *ymod; + cvec *idrefvec; /* Derived identityref list: (module:id)**/ /* Top-call (no recursion) create idref * The idref is (here) in "canonical form": : @@ -1632,7 +1633,6 @@ ys_populate_identity(clicon_handle h, clicon_err(OE_UNIX, errno, "cbuf_new"); goto done; } -#if 0 /* Use module:id instead of prefix:id in derived list */ if (nodeid_split(yang_argument_get(ys), &prefix, &id) < 0) goto done; if ((ymod = ys_module(ys)) == NULL){ @@ -1640,16 +1640,6 @@ ys_populate_identity(clicon_handle h, goto done; } cprintf(cb, "%s:%s", yang_argument_get(ymod), id); -#else - { - if (nodeid_split(yang_argument_get(ys), &prefix, &id) < 0) - goto done; - if (prefix) - cprintf(cb, "%s:%s", prefix, id); - else - cprintf(cb, "%s:%s", yang_find_myprefix(ys), id); - } -#endif idref = cbuf_get(cb); } /* Iterate through all base statements and check the base identity exists @@ -1666,7 +1656,8 @@ ys_populate_identity(clicon_handle h, } // continue; /* root identity */ /* Check if derived id is already in base identifier */ - if (cvec_find(ybaseid->ys_cvec, idref) != NULL) + idrefvec = yang_cvec_get(ybaseid); + if (cvec_find(idrefvec, idref) != NULL) continue; /* Add derived id to ybaseid */ if ((cv = cv_new(CGV_STRING)) == NULL){ @@ -1675,7 +1666,7 @@ ys_populate_identity(clicon_handle h, } /* add prefix */ cv_name_set(cv, idref); - cvec_append_var(ybaseid->ys_cvec, cv); /* cv copied */ + cvec_append_var(idrefvec, cv); /* cv copied */ if (cv){ cv_free(cv); cv = NULL; diff --git a/lib/src/clixon_yang_internal.h b/lib/src/clixon_yang_internal.h index 665fb512..fe7b2c49 100644 --- a/lib/src/clixon_yang_internal.h +++ b/lib/src/clixon_yang_internal.h @@ -97,7 +97,7 @@ struct yang_stmt{ Y_RANGE: range_min, range_max Y_LIST: vector of keys Y_TYPE & identity: store all derived - types as : list + types as : list */ yang_type_cache *ys_typecache; /* If ys_keyword==Y_TYPE, cache all typedef data except unions */ int _ys_vector_i; /* internal use: yn_each */