diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36269387..937eeac7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -80,9 +80,9 @@
### API changes on existing features (you may need to change your code)
* Stricter YANG choice validation leads to enforcement of structures like: `choice c{ mandatory true; leaf x` statements. `x` was not previously enforced.
-* CLICON_XML_SORT option (in clixon-config.yang) has been removed and set to true permanently since setting it to false is obsolete.
+* Many hand-crafted validation messages have been removed and replaced with generic validations, which may lead to changed rpc-error messages.
+* CLICON_XML_SORT option (in clixon-config.yang) has been removed and set to true permanently. Unsorted XML lists leads to slower performance and old obsolete code can be removed.
* Strict namespace setting can be a problem when upgrading existing database files, such as startup-db or persistent running-db, or any other saved XML file.
- * For backward compatibility, load of startup and running set CLICON_XML_NS_STRICT to false temporarily.
* Removed `delete-config` support for candidate db since it is not supported in RFC6241.
* Switched the order of `error-type` and `error-tag` in all netconf and restconf error messages to comply to RFC order.
* Yang parser is stricter (see above) which may break parsing of existing yang specs.
diff --git a/apps/netconf/netconf_rpc.c b/apps/netconf/netconf_rpc.c
index 21a02193..ba41648a 100644
--- a/apps/netconf/netconf_rpc.c
+++ b/apps/netconf/netconf_rpc.c
@@ -135,20 +135,10 @@ netconf_get_config(clicon_handle h,
{
cxobj *xfilter; /* filter */
int retval = -1;
- char *source;
char *ftype = NULL;
cxobj *xfilterconf;
cxobj *xconf;
- if ((source = netconf_get_target(xn, "source")) == NULL){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "source"
- "");
- goto ok;
- }
/* ie ... */
if ((xfilter = xpath_first(xn, "filter")) != NULL)
ftype = xml_find_value(xfilter, "type");
@@ -187,7 +177,6 @@ netconf_get_config(clicon_handle h,
"type"
"");
}
- ok: /* netconf error is not fatal */
retval = 0;
done:
return retval;
@@ -223,11 +212,9 @@ get_edit_opts(cxobj *xn,
if ((optstr = xml_body(x)) != NULL){
if (strcmp(optstr, "test-then-set") == 0)
*testopt = TEST_THEN_SET;
- else
- if (strcmp(optstr, "set") == 0)
+ else if (strcmp(optstr, "set") == 0)
*testopt = SET;
- else
- if (strcmp(optstr, "test-only") == 0)
+ else if (strcmp(optstr, "test-only") == 0)
*testopt = TEST_ONLY;
else
goto parerr;
@@ -311,52 +298,18 @@ netconf_edit_config(clicon_handle h,
{
int retval = -1;
int optret;
- enum operation_type operation = OP_MERGE;
enum test_option testopt = TEST_THEN_SET;/* only supports this */
enum error_option erropt = STOP_ON_ERROR; /* only supports this */
- cxobj *x;
- cxobj *xfilter;
- char *ftype = NULL;
- char *target; /* db */
- /* must have target, and it should be candidate */
- if ((target = netconf_get_target(xn, "target")) == NULL ||
- strcmp(target, "candidate")){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "target"
- "");
- goto ok;
- }
- /* CLICON addition, eg /> */
- if ((xfilter = xpath_first(xn, "filter")) != NULL) {
- if ((ftype = xml_find_value(xfilter, "type")) != NULL)
- if (strcmp(ftype,"restconf")){
- xml_parse_va(xret, NULL, ""
- "invalid-value"
- "protocol"
- "error"
- "");
- goto ok;
- }
- }
- if ((x = xpath_first(xn, "default-operation")) != NULL){
- if (xml_operation(xml_body(x), &operation) < 0){
- xml_parse_va(xret, NULL, ""
- "invalid-value"
- "protocol"
- "error"
- "");
- goto ok;
- }
- }
if ((optret = get_edit_opts(xn, &testopt, &erropt, xret)) < 0)
goto done;
if (optret == 0) /* error in opt parameters */
goto ok;
- /* not supported opts */
+ /* These constraints are clixon-specific since :validate should
+ * support all testopts, and erropts should be supported
+ * And therefore extends the validation
+ * (implement the features before removing these checks)
+ */
if (testopt!=TEST_THEN_SET || erropt!=STOP_ON_ERROR){
xml_parse_va(xret, NULL, ""
"operation-not-supported"
@@ -364,158 +317,15 @@ netconf_edit_config(clicon_handle h,
"error"
"");
goto ok;
- }
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- ok:
- retval = 0;
- done:
- return retval;
-}
-
-/*! Netconf copy configuration
-
-
-
-
-
-
-
-
-
-
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_copy_config(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval = -1;
- char *source;
- char *target; /* filenames */
-
- if ((source = netconf_get_target(xn, "source")) == NULL){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "source"
- "");
- goto ok;
- }
- if ((target = netconf_get_target(xn, "target")) == NULL){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "target"
- "");
- goto ok;
- }
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- ok:
- retval = 0;
- done:
- return retval;
-}
-
-/*! Delete configuration
-
-
-
-
-
- Delete a configuration datastore. The
- configuration datastore cannot be deleted.
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_delete_config(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- char *target; /* filenames */
- int retval = -1;
-
- if ((target = netconf_get_target(xn, "target")) == NULL ||
- strcmp(target, "running")==0){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "target"
- "");
- goto ok;
- }
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- ok:
- retval = 0;
- done:
- return retval;
-}
-
-
-/*! Lock a database
-
-
-
-
-
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_lock(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval = -1;
- char *target;
-
- if ((target = netconf_get_target(xn, "target")) == NULL){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "target"
- "");
- goto ok;
}
if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
goto done;
ok:
retval = 0;
- done:
+ done:
return retval;
}
-/*! Unlock a database
-
-
-
-
-
- XXX
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_unlock(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- return netconf_lock(h, xn, xret);
-}
-
/*! Get running configuration and device state information
*
*
@@ -579,138 +389,11 @@ netconf_get(clicon_handle h,
"type"
"");
}
- // ok: /* netconf error is not fatal */
retval = 0;
done:
return retval;
}
-
-/*! Close a (user) session
-
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
-*/
-static int
-netconf_close_session(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval = -1;
-
- cc_closed++;
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- retval = 0;
- done:
- return retval;
-}
-
-/*! Kill other user sessions
-
- PID
-
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_kill_session(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval=-1;
- cxobj *xs;
-
- if ((xs = xpath_first(xn, "//session-id")) == NULL){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "session-id"
- "");
- goto ok;
- }
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- ok:
- retval = 0;
- done:
- return retval;
-}
-/*! Check the semantic consistency of candidate
-
- :validate
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_validate(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval = -1;
- char *target;
-
- if ((target = netconf_get_target(xn, "source")) == NULL){
- xml_parse_va(xret, NULL, ""
- "missing-element"
- "protocol"
- "error"
- "target"
- "");
- goto ok;
- }
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- ok:
- retval = 0;
- done:
- return retval;
-}
-
-/*! Commit candidate -> running
-
- :candidate
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_commit(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval = -1;
-
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- retval = 0;
- done:
- return retval;
-}
-
-/*! Discard all changes in candidate / revert to running
-
- :candidate
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at ... level.
- * @param[out] xret Return XML, error or OK
- */
-static int
-netconf_discard_changes(clicon_handle h,
- cxobj *xn,
- cxobj **xret)
-{
- int retval = -1;
-
- if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
- goto done;
- retval = 0;
- done:
- return retval;
-}
-
/*! Called when a notification has happened on backend
* and this session has registered for that event.
* Filter it and forward it.
@@ -993,9 +676,25 @@ netconf_rpc_dispatch(clicon_handle h,
if (xml_value_set(xa, username) < 0)
goto done;
}
+ /* Many of these calls are now calling generic clicon_rpc_netconf_xml
+ * directly, since the validation is generic and done before this place
+ * in the call. Some call however need extra validation, such as the
+ * filter parameter to get/get-config and tes- err-opts of edit-config.
+ */
xe = NULL;
while ((xe = xml_child_each(xn, xe, CX_ELMNT)) != NULL) {
- if (strcmp(xml_name(xe), "get-config") == 0){
+ if (strcmp(xml_name(xe), "copy-config") == 0 ||
+ strcmp(xml_name(xe), "delete-config") == 0 ||
+ strcmp(xml_name(xe), "lock") == 0 ||
+ strcmp(xml_name(xe), "unlock") == 0 ||
+ strcmp(xml_name(xe), "kill-session") == 0 ||
+ strcmp(xml_name(xe), "validate") == 0 || /* :validate */
+ strcmp(xml_name(xe), "commit") == 0 || /* :candidate */
+ strcmp(xml_name(xe), "discard-changes") == 0){
+ if (clicon_rpc_netconf_xml(h, xml_parent(xe), xret, NULL) < 0)
+ goto done;
+ }
+ else if (strcmp(xml_name(xe), "get-config") == 0){
if (netconf_get_config(h, xe, xret) < 0)
goto done;
}
@@ -1003,47 +702,14 @@ netconf_rpc_dispatch(clicon_handle h,
if (netconf_edit_config(h, xe, xret) < 0)
goto done;
}
- else if (strcmp(xml_name(xe), "copy-config") == 0){
- if (netconf_copy_config(h, xe, xret) < 0)
- goto done;
- }
- else if (strcmp(xml_name(xe), "delete-config") == 0){
- if (netconf_delete_config(h, xe, xret) < 0)
- goto done;
- }
- else if (strcmp(xml_name(xe), "lock") == 0) {
- if (netconf_lock(h, xe, xret) < 0)
- goto done;
- }
- else if (strcmp(xml_name(xe), "unlock") == 0){
- if (netconf_unlock(h, xe, xret) < 0)
- goto done;
- }
else if (strcmp(xml_name(xe), "get") == 0){
if (netconf_get(h, xe, xret) < 0)
goto done;
}
else if (strcmp(xml_name(xe), "close-session") == 0){
- if (netconf_close_session(h, xe, xret) < 0)
- goto done;
- }
- else if (strcmp(xml_name(xe), "kill-session") == 0) {
- if (netconf_kill_session(h, xe, xret) < 0)
- goto done;
- }
- /* Validate capability :validate */
- else if (strcmp(xml_name(xe), "validate") == 0){
- if (netconf_validate(h, xe, xret) < 0)
- goto done;
- }
- /* Candidate configuration capability :candidate */
- else if (strcmp(xml_name(xe), "commit") == 0){
- if (netconf_commit(h, xe, xret) < 0)
- goto done;
- }
- else if (strcmp(xml_name(xe), "discard-changes") == 0){
- if (netconf_discard_changes(h, xe, xret) < 0)
- goto done;
+ cc_closed++;
+ if (clicon_rpc_netconf_xml(h, xml_parent(xe), xret, NULL) < 0)
+ goto done;
}
/* RFC 5277 :notification */
else if (strcmp(xml_name(xe), "create-subscription") == 0){