Did not check for missing list keys in validate. [Key of a list isn't…

… mandatory](#73)

CHANGELOG.md

@@ -152,6 +152,8 @@
 * New config option: CLICON_CLI_MODEL_TREENAME defining name of generated syntax tree if CLIXON_CLI_GENMODEL is set.
 
 ### Corrected Bugs
+* Did not check for missing keys in validate. [Key of a list isn't mandatory](https://github.com/clicon/clixon/issues/73)
+  * Problem here is that you can still add keys via netconf - since validation is a separate step, but in cli or restconf it should not be possible.
 * Partially corrected: [yang type range statement does not support multiple values](https://github.com/clicon/clixon/issues/59).
   * Should work for netconf and restconf, but not for CLI.
 * Fixed: [Range parsing is not RFC 7950 compliant](https://github.com/clicon/clixon/issues/71)

apps/backend/backend_commit.c

@@ -203,13 +203,13 @@ validate_common(clicon_handle       h,
 	xml_flag_set(xn, XML_FLAG_DEL);
 	xml_apply(xn, CX_ELMNT, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_DEL);
 	xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE);
-    }    
+    }
     for (i=0; i<td->td_alen; i++){ /* Also down */
 	xn = td->td_avec[i];
 	xml_flag_set(xn, XML_FLAG_ADD);
 	xml_apply(xn, CX_ELMNT, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_ADD);
 	xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE);
-    }    
+    }
     for (i=0; i<td->td_clen; i++){ /* Also up */
 	xn = td->td_scvec[i];
 	xml_flag_set(xn, XML_FLAG_CHANGE);
@@ -218,7 +218,6 @@ validate_common(clicon_handle       h,
 	xml_flag_set(xn, XML_FLAG_CHANGE);
 	xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE);
     }    
-
     /* 4. Call plugin transaction start callbacks */
     if (plugin_transaction_begin(h, td) < 0)
 	goto done;
@@ -357,7 +356,8 @@ from_client_commit(clicon_handle h,
 		goto done;
         goto ok;
     }
-    cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
+    if (ret == 1)
+	cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
  ok:
     retval = 0;
  done:

lib/src/clixon_xml_map.c

@@ -501,9 +501,27 @@ check_mandatory(cxobj     *xt,
     yang_stmt *y;
     yang_stmt *yc;
     yang_node *yp;
+    cvec      *cvk = NULL; /* vector of index keys */
+    cg_var    *cvi;
+    char      *keyname;
 
     for (i=0; i<yt->ys_len; i++){
 	yc = yt->ys_stmt[i];
+	/* Check if a list does not have mandatory key leafs */
+	if (yt->ys_keyword == Y_LIST &&
+	    yc->ys_keyword == Y_KEY &&
+	    yang_config(yt)){
+	    cvk = yt->ys_cvec; /* Use Y_LIST cache, see ys_populate_list() */
+	    cvi = NULL;
+	    while ((cvi = cvec_each(cvk, cvi)) != NULL) {
+		keyname = cv_string_get(cvi);	    
+		if (xml_find_type(xt, NULL, keyname, CX_ELMNT) == NULL){
+		    if (netconf_missing_element(cbret, "application", keyname, "Mandatory key") < 0)
+			goto done;
+		    goto fail;
+		}
+	    }
+	}
 	if (!yang_mandatory(yc))
 	    continue;
 	switch (yc->ys_keyword){
@@ -793,6 +811,9 @@ xml_yang_validate_all(cxobj   *xt,
 }
 
 /*! Translate a single xml node to a cligen variable vector. Note not recursive 
+ * @retval     1     Validation OK
+ * @retval     0     Validation failed (cbret set)
+ * @retval    -1     Error
  */
 int
 xml_yang_validate_all_top(cxobj   *xt, 

lib/src/clixon_xml_sort.c

@@ -252,11 +252,15 @@ xml_cmp(const void* arg1,
 	cvk = y1->ys_cvec; /* Use Y_LIST cache, see ys_populate_list() */
 	cvi = NULL;
 	while ((cvi = cvec_each(cvk, cvi)) != NULL) {
-	    keyname = cv_string_get(cvi);
-	    b1 = xml_find_body(x1, keyname);
-	    b2 = xml_find_body(x2, keyname);
-	    if ((equal = strcmp(b1,b2)) != 0)
-		goto done;
+	    keyname = cv_string_get(cvi); /* operational data may have NULL keys*/
+	    if ((b1 = xml_find_body(x1, keyname)) == NULL)
+		equal = -1;
+	    else if ((b2 = xml_find_body(x2, keyname)) == NULL)
+		equal = 1;
+	    else{
+		if ((equal = strcmp(b1,b2)) != 0)
+		    goto done;
+	    }
 	}
 	equal = 0;
 	break;

test/test_netconf.sh

@@ -86,6 +86,15 @@ expecteof "$clixon_netconf -qf $cfg" 0 '<rpc message-id="101"><get-config><sourc
 new "Re-Delete eth/0/0 using none should generate error"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces"><interface operation="delete"><name>eth/0/0</name><type>ex:eth</type></interface></interfaces></config><default-operation>none</default-operation> </edit-config></rpc>]]>]]>'  '^<rpc-reply><rpc-error>'
 
+new "Add interface without key"
+expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces"><interface operation="create"><type>ex:eth</type></interface></interfaces></config><default-operation>none</default-operation> </edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+
+new "netconf validate missing key"
+expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>missing-element</error-tag><error-info><bad-element>name</bad-element></error-info><error-severity>error</error-severity><error-message>Mandatory key</error-message></rpc-error></rpc-reply>]]>]]>$'
+
+new "netconf discard-changes"
+expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><discard-changes/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+
 new "netconf edit config"
 expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces"><interface><name>eth/0/0</name></interface><interface><name>eth1</name><enabled>true</enabled><ipv4><address><ip>9.2.3.4</ip><prefix-length>24</prefix-length></address></ipv4></interface></interfaces></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
 

test/test_restconf2.sh

@@ -89,6 +89,9 @@ sleep $RCWAIT
 
 new "restconf tests"
 
+new "restconf POST tree without key"
+expectfn 'curl -s -X POST -d {"example:cont1":{"interface":{"type":"regular"}}} http://localhost/restconf/data' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "name"},"error-severity": "error","error-message": "Mandatory key"}}}'
+
 new "restconf POST initial tree"
 expectfn 'curl -s -X POST -d {"example:cont1":{"interface":{"name":"local0","type":"regular"}}} http://localhost/restconf/data' 0 ""
 
@@ -110,7 +113,10 @@ new "restconf GET if-type"
 expectfn "curl -s -X GET http://localhost/restconf/data/example:cont1/interface=local0/type" 0 '{"example:type": "regular"}'
 
 new "restconf POST interface without mandatory type"
-expectfn 'curl -s -X POST -d {"interface":{"name":"TEST"}} http://localhost/restconf/data/example:cont1' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "type"},"error-severity": "error","error-message": "Mandatory variable"}}}'
+expectfn 'curl -s -X POST http://localhost/restconf/data/example:cont1 -d {"interface":{"name":"TEST"}} http://localhost/restconf/data/example:cont1' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "type"},"error-severity": "error","error-message": "Mandatory variable"}}}'
+
+new "restconf POST interface without mandatory key"
+expectfn 'curl -s -X POST http://localhost/restconf/data/example:cont1 -d {"interface":{"type":"regular"}}' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "name"},"error-severity": "error","error-message": "Mandatory key"}}}'
 
 new "restconf POST interface"
 expectfn 'curl -s -X POST -d {"example:interface":{"name":"TEST","type":"eth0"}} http://localhost/restconf/data/example:cont1' 0 ""