diff --git a/CHANGELOG.md b/CHANGELOG.md index 00a27c0e..f2e81145 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,9 +74,10 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: [namespace prefix nc is not supported in full #154](https://github.com/clicon/clixon/issues/154) + * edit-config "config" parameter did not work with prefix other than null * Fixed [YANG: key statement in rpc/notification list #148](https://github.com/clicon/clixon/issues/148) * Do not check uniqueness among lists without keys - * Fixed typo: [False Header Content_type in restconf error #152](https://github.com/clicon/clixon/issues/152) * Added message-id attributes in error and hello replies * See [namespace prefix nc is not supported in full #154](https://github.com/clicon/clixon/issues/154) diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index f974d37a..ff5972ca 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -509,6 +509,7 @@ from_client_get_config(clicon_handle h, goto done; goto ok; } + /* XXX should use prefix cf edit_config */ if ((xfilter = xml_find(xe, "filter")) != NULL){ if ((xpath0 = xml_find_value(xfilter, "select"))==NULL) xpath0="/"; @@ -587,6 +588,8 @@ from_client_edit_config(clicon_handle h, char *attr; int autocommit = 0; char *val = NULL; + cvec *nsc = NULL; + char *prefix = NULL; username = clicon_username_get(h); if ((yspec = clicon_dbspec_yang(h)) == NULL){ @@ -616,15 +619,28 @@ from_client_edit_config(clicon_handle h, goto done; goto ok; } - if ((x = xpath_first(xn, NULL, "default-operation")) != NULL){ + if (xml_nsctx_node(xn, &nsc) < 0) + goto done; + /* Get prefix of netconf base namespace in the incoming message */ + if (xml_nsctx_get_prefix(nsc, NETCONF_BASE_NAMESPACE, &prefix) == 0){ + cprintf(cbx, "No appropriate prefix exists for: %s", NETCONF_BASE_NAMESPACE); + if (netconf_unknown_namespace(cbret, "protocol", xml_name(xn), cbuf_get(cbx)) < 0) + goto done; + goto ok; + } + /* Get default-operation element */ + if ((x = xpath_first(xn, nsc, "%s%sdefault-operation", prefix?prefix:"", prefix?":":"")) != NULL){ if (xml_operation(xml_body(x), &operation) < 0){ if (netconf_invalid_value(cbret, "protocol", "Wrong operation")< 0) goto done; goto ok; } } - if ((xc = xpath_first(xn, NULL, "config")) == NULL){ - if (netconf_missing_element(cbret, "protocol", "config", NULL) < 0) + /* Get config element */ + if ((xc = xpath_first(xn, nsc, "%s%sconfig", prefix?prefix:"", prefix?":":"")) == NULL){ + cprintf(cbx, "Element not found, or mismatching prefix %s for namespace %s", + prefix?prefix:"null", NETCONF_BASE_NAMESPACE); + if (netconf_missing_element(cbret, "protocol", "config", cbuf_get(cbx)) < 0) goto done; goto ok; } @@ -713,6 +729,8 @@ from_client_edit_config(clicon_handle h, ok: retval = 0; done: + if (nsc) + cvec_free(nsc); if (xret) xml_free(xret); if (cbx) @@ -822,6 +840,7 @@ from_client_delete_config(clicon_handle h, uint32_t myid = ce->ce_id; cbuf *cbx = NULL; /* Assist cbuf */ + /* XXX should use prefix cf edit_config */ if ((target = netconf_db_find(xe, "target")) == NULL || strcmp(target, "running")==0){ if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0) @@ -1340,6 +1359,7 @@ from_client_create_subscription(clicon_handle h, struct timeval stop; cvec *nsc = NULL; + /* XXX should use prefix cf edit_config */ if ((nsc = xml_nsctx_init(NULL, EVENT_RFC5277_NAMESPACE)) == NULL) goto done; if ((x = xpath_first(xe, nsc, "//stream")) != NULL) @@ -1654,7 +1674,7 @@ from_client_msg(clicon_handle h, goto reply; } else if (strcmp(namespace, NETCONF_BASE_NAMESPACE) != 0){ - cbuf *cbmsg; + cbuf *cbmsg = NULL; if ((cbmsg = cbuf_new()) == NULL){ clicon_err(OE_UNIX, errno, "cbuf_new"); goto done; diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index 3c9dccc9..966515b9 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -1406,6 +1406,7 @@ netconf_db_find(cxobj *xn, cxobj *xi; char *db = NULL; + /* XXX should use prefix cf edit_config */ if ((xs = xml_find(xn, name)) == NULL) goto done; if ((xi = xml_child_i(xs, 0)) == NULL) diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 18150796..e1f5082d 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -1697,7 +1697,7 @@ xml_find_type_value(cxobj *xt, * * The value can be of an attribute only * @param[in] xt xml tree node - * @param[in] prefix Prefix (namespace local name) or NULL + * @param[in] prefix Prefix (namespace local name) or NULL (any prefix) * @param[in] name name of xml tree node (eg attr name or "body") * @param[in] type Matching type or -1 for any * @retval val Pointer to the name string diff --git a/lib/src/clixon_xpath.c b/lib/src/clixon_xpath.c index 8353ff5f..6e7262c8 100644 --- a/lib/src/clixon_xpath.c +++ b/lib/src/clixon_xpath.c @@ -35,26 +35,24 @@ * Clixon XML XPATH 1.0 according to https://www.w3.org/TR/xpath-10 * * Some notes on namespace extensions in Netconf/Yang - * RFC6241 8.9.1 - * The set of namespace declarations are those in scope on the element. + * 1) The xpath is not "namespace-aware" in the sense that if you look for a path, eg + * "n:a/n:b", those must match the XML, so they need to match prefixes AND name in the xml + * such as . An xml with (or ) will NOT match EVEN IF they have the + * same namespace given by xmlns settings. + * 2) RFC6241 8.9.1 + * In the scope of get-.config, the set of namespace declarations are those in scope on the + * element. * * * * - * We need to add namespace context to the cpath tree, typically in eval. How do - * we do that? * One observation is that the namespace context is static, so it can not be a part * of the xpath-tree, which is context-dependent. * Best is to send it as a (read-only) parameter to the xp_eval family of functions * as an exlicit namespace context. - * For that you need an API to get/set namespaces: clixon_xml_nscache.c? - * Then you need to fix API functions and this is the real work: - * - Replace all existing functions or create new? - * - Expose explicit namespace parameter, or xml object, or default namespace? * - * @see README.md#xml-and-xpath for description of xpath implementation */ #ifdef HAVE_CONFIG_H #include "clixon_config.h" /* generated by config & autoconf */ @@ -587,7 +585,9 @@ xpath_vec_ctx(cxobj *xcur, * * @code * cxobj *x; - * cvec *nsc; // namespace context + * cvec *nsc = NULL; // namespace context + * if (xml_nsctx_node(xtop, &nsc) < 0) + * err; * if ((x = xpath_first(xtop, nsc, "//symbol/foo")) != NULL) { * ... * } diff --git a/test/test_netconf.sh b/test/test_netconf.sh index b8c78ba0..a58b8e1b 100755 --- a/test/test_netconf.sh +++ b/test/test_netconf.sh @@ -1,5 +1,8 @@ #!/usr/bin/env bash # Basic Netconf functionality +# Mainly default/null prefix, but also xx: prefix +# XXX: could add tests for dual prefixes xx and xy with doppelganger names, ie xy:filter that is +# syntactic correct but wrong # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -80,9 +83,12 @@ expecteof "$clixon_netconf -f $cfg" 0 "]]>]]>" "^urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:capability:candidate:1.0urn:ietf:params:netconf:capability:validate:1.1urn:ietf:params:netconf:capability:startup:1.0urn:ietf:params:netconf:capability:xpath:1.0urn:ietf:params:netconf:capability:notification:1.0[0-9]*]]>]]>]]>]]>$" -new "netconf get-config prefix" +new "netconf get-config nc prefix" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +new "netconf get-config xx prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + new "netconf get-config double quotes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -92,6 +98,13 @@ expecteof "$clixon_netconf -qf $cfg" 0 "noneeth/0/0]]>]]>" "^]]>]]>$" +# Trying prefixes +new "Add subtree eth/0/0 using nc prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "noneeth/0/0]]>]]>" "^]]>]]>$" + +new "Add subtree eth/0/0 using xx prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "noneeth/0/0]]>]]>" "^]]>]]>$" + new "Check nothing added" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -124,6 +137,9 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +new "netconf discard-changes using xx prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + new "netconf edit config" expecteof "$clixon_netconf -qf $cfg" 0 "eth/0/0eth1true9.2.3.424]]>]]>" "^]]>]]>$" @@ -146,6 +162,9 @@ expecteof "$clixon_netconf -qf $cfg" 0 "$(cat $tmp)" "^]]>]]>" "^" +new "netconf validate using xx prefix" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^" + new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -167,6 +186,9 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +new "netconf commit using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + new "netconf edit config merge eth2" expecteof "$clixon_netconf -qf $cfg" 0 "eth2ex:ethmerge]]>]]>" "^]]>]]>$" @@ -196,12 +218,21 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^eth1ex:ethtrueup42foo]]>]]>$" +new "netconf get state operation use prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^eth1ex:ethtrueup42foo]]>]]>$" + new "netconf lock" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" new "netconf unlock" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^protocollock-denied0errorUnlock failed, lock is not currently active]]>]]>$" +new "netconf lock using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + +new "netconf unlock using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^protocollock-denied0errorUnlock failed, lock is not currently active]]>]]>$" + new "netconf lock/unlock" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>]]>]]>" "^]]>]]>]]>]]>$" @@ -214,10 +245,16 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +new "close-session using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + # XXX NOTE that this does not actually kill a running session - and may even kill some random process,... new "kill-session" expecteof "$clixon_netconf -qf $cfg" 0 "44]]>]]>" "^]]>]]>$" +new "kill-session using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "44]]>]]>" "^]]>]]>$" + # modify candidate, then lock, should fail. new "netconf edit config" expecteof "$clixon_netconf -qf $cfg" 0 "a
]]>]]>" "^]]>]]>$" @@ -234,18 +271,27 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +new "copy startup to candidate using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + new "netconf get startup" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^eth1ex:ethtrue]]>]]>$" new "netconf delete startup" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" +new "netconf delete startup using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" + new "netconf check empty startup" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" new "netconf example rpc" expecteof "$clixon_netconf -qf $cfg" 0 "42]]>]]>" "^4242]]>]]>$" +new "netconf example rpc using prefix xx" +expecteof "$clixon_netconf -qf $cfg" 0 "42]]>]]>" "^4242]]>]]>$" + new "netconf empty rpc" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$"