Skip to content

Commit

Permalink
* Error messages for invalid number ranges and string lengths have be…
Browse files Browse the repository at this point in the history 
…en uniformed and changed.

  * Error messages for invalid ranges are now on the form:
  ```
    Number 23 out of range: 1-10
    String length 23 out of range: 1-10
  ```
  • Loading branch information
@olofhagsand
olofhagsand committed Jun 20, 2019
1 parent c683bc9 commit 97529a2
Show file tree
Hide file tree
Showing 8 changed files with 388 additions and 33 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@

### API changes on existing features (you may need to change your code)

* Error messages for invalid number ranges and string lengths have been uniformed and changed.
* Error messages for invalid ranges are now on the form:
```
Number 23 out of range: 1-10
String length 23 out of range: 1-10
```
* On validation callbacks, XML_FLAG_ADD is added to all nodes at startup validation, not just the top-level. This is the same behaviour as for steady-state validation.
* All hash_ functions have been prefixed with `clicon_` to avoid name collision with other packages (frr)
* All calls to the following functions must be changed: `hash_init`, `hash_free`, `hash_lookup`, `hash_value`, `hash_add`, `hash_del`, `hash_dump`, and `hash_keys`.
Expand Down
4 changes: 0 additions & 4 deletions lib/src/clixon_datastore_read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,10 +589,6 @@ xmldb_get_zerocopy(clicon_handle h,
db_elmnt *de = NULL;
db_elmnt de0 = {0,};

if (!clicon_option_bool(h, "CLICON_XMLDB_CACHE")){
clicon_err(OE_CFG, 0, "CLICON_XMLDB_CACHE must be set");
goto done;
}
if ((yspec = clicon_dbspec_yang(h)) == NULL){
clicon_err(OE_YANG, ENOENT, "No yang spec");
goto done;
Expand Down
113 changes: 106 additions & 7 deletions lib/src/clixon_yang_type.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,105 @@ cv_validate_pattern(clicon_handle h,
(rmax && (i) > cv_##type##_get(rmax)))


/*! Error messsage for int violating ranges
* @note contains kludge - duplicate loop
*/
static int
outofrange(cg_var *cv0,
cvec *cvv,
char **reason)
{
int retval = -1;
cbuf *cb = NULL;
cg_var *cv1;
cg_var *cv2;
int i;

if ((cb = cbuf_new()) == NULL)
goto done;
cprintf(cb, "Number ");
cv2cbuf(cv0, cb);
cprintf(cb, " out of range: ");
/* Kludge: need to repeat the same loop as in the main function in
cv_validate1 */
i = 0;
while (i<cvec_len(cvv)){
cv1 = cvec_i(cvv, i++); /* Increment to check for max pair */
if (strcmp(cv_name_get(cv1),"range_min") != 0){
clicon_err(OE_YANG, EINVAL, "Internal error, expected range_min");
goto done;
}
if (i<cvec_len(cvv) &&
(cv2 = cvec_i(cvv, i)) != NULL &&
strcmp(cv_name_get(cv2),"range_max") == 0){
i++;
}
else
cv2 = cv1;
if (i>2)
cprintf(cb, ", ");
cv2cbuf(cv1, cb);
cprintf(cb, "-");
cv2cbuf(cv2, cb);
}
if (reason && (*reason = strdup(cbuf_get(cb))) == NULL)
goto done;
if (cb)
cbuf_free(cb);
retval = 0;
done:
return retval;
}

/*! Error messsage for string violating string limits
* @note contains kludge - duplicate loop
*/
static int
outoflength(uint64_t u64,
cvec *cvv,
char **reason)
{
int retval = -1;
cbuf *cb = NULL;
cg_var *cv1;
cg_var *cv2;
int i;

if ((cb = cbuf_new()) == NULL)
goto done;
cprintf(cb, "String length %" PRIu64 " out of range: ", u64);

/* Kludge: need to repeat the same loop as in the main function in
cv_validate1 */
i = 0;
while (i<cvec_len(cvv)){
cv1 = cvec_i(cvv, i++); /* Increment to check for max pair */
if (strcmp(cv_name_get(cv1),"range_min") != 0){
clicon_err(OE_YANG, EINVAL, "Internal error, expected range_min");
goto done;
}
if (i<cvec_len(cvv) &&
(cv2 = cvec_i(cvv, i)) != NULL &&
strcmp(cv_name_get(cv2),"range_max") == 0){
i++;
}
else
cv2 = cv1;
if (i>2)
cprintf(cb, ", ");
cv2cbuf(cv1, cb);
cprintf(cb, "-");
cv2cbuf(cv2, cb);
}
if (reason && (*reason = strdup(cbuf_get(cb))) == NULL)
goto done;
if (cb)
cbuf_free(cb);
retval = 0;
done:
return retval;
}

/*! Validate CLIgen variable
* @param[in] h Clicon handle
* @param[in] cv A cligen variable to validate. This is a correctly parsed cv.
Expand All @@ -600,6 +699,7 @@ cv_validate_pattern(clicon_handle h,
* @retval 0 Validation not OK, malloced reason is returned. Free reason with free()
* @retval 1 Validation OK
* @note reason if given must be freed by caller
* @see cv_validate Corresponding type check in cligen
*/
static int
cv_validate1(clicon_handle h,
Expand Down Expand Up @@ -705,14 +805,13 @@ cv_validate1(clicon_handle h,
/* Check fails */
if (i==cvec_len(cvv)){ /* And it is last */
if (reason){
if (reti)
*reason = cligen_reason("Number out of range: %"
PRId64, ii);
else if (retu)
*reason = cligen_reason("Number out of range: %"
PRIu64, uu);
if (reti || retu){
if (outofrange(cv, cvv, reason) < 0)
goto done;
}
else
*reason = cligen_reason("string length out of range: %" PRIu64, uu);
if (outoflength(uu, cvv, reason) < 0)
goto done;
}
goto fail;
}
Expand Down
3 changes: 2 additions & 1 deletion test/all.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
: ${pattern:=test_*.sh}

if [ $# -gt 0 ]; then
echo "usage: $0 # detailed logs and stopon first error"
echo "usage: $0 # detailed logs and stop on first error. Use pattern=\"\" $0 to"
echo " Use pattern=<pattern> $0 to narrow down test cases"
exit -1
fi

Expand Down
2 changes: 0 additions & 2 deletions test/test_insert.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# First a list with 0-5 base elements, insert in different places
# Second varying yangs: container, leaf, list, leaf-list, choice, user-order list


# Magic line must be first in script (see README.md)
s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi

Expand All @@ -27,7 +26,6 @@ cat <<EOF > $cfg
<CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
<CLICON_XMLDB_CACHE>true</CLICON_XMLDB_CACHE>
</clixon-config>
EOF

Expand Down
2 changes: 1 addition & 1 deletion test/test_transaction.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ new "3. Validate system-error config (9999 not in range)"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><edit-config><target><candidate/></target><config><x xmlns='urn:example:clixon'><y><a>$nr</a><b>9999</b></y></x></config></edit-config></rpc>]]>]]>" '^<rpc-reply><ok/></rpc-reply>]]>]]>$'

new "Validate system-error validate (should fail)"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>b</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range: 9999</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>' '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>b</bad-element></error-info><error-severity>error</error-severity><error-message>Number 9999 out of range: 0-100</error-message></rpc-error></rpc-reply>]]>]]>$'

new "Validate system-error discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><discard-changes/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
Expand Down
39 changes: 21 additions & 18 deletions test/test_type.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ EOF

# Type tests.
# Parameters:
# 1: dbcache true/false
# 1: dbcache: cache, nocache, cache-zerocopy
testrun(){
dbcache=$1
new "test params: -f $cfg # dbcache: $dbcache"
Expand All @@ -216,7 +216,7 @@ testrun(){
<CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
<CLICON_CLI_GENMODEL_COMPLETION>1</CLICON_CLI_GENMODEL_COMPLETION>
<CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
<CLICON_XMLDB_CACHE>$dbcache</CLICON_XMLDB_CACHE>
<CLICON_DATASTORE_CACHE>$dbcache</CLICON_DATASTORE_CACHE>
<CLICON_XMLDB_FORMAT>$format</CLICON_XMLDB_FORMAT>
</clixon-config>
EOF
Expand All @@ -231,7 +231,7 @@ EOF
start_backend -s init -f $cfg

new "waiting"
sleep $RCWAIT
wait_backend
fi

new "cli set transitive string. type is alpha followed by number and is defined in three levels of modules"
Expand Down Expand Up @@ -360,15 +360,15 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><num1 xmlns="urn:example:clixon">-1</num1></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf validate num1 -1 wrong"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>num1</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range: -1</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>num1</bad-element></error-info><error-severity>error</error-severity><error-message>Number -1 out of range: 1-1</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>]]>]]>$"

#-------- num2 range and blanks

new "cli range test num2 3 error"
expectfn "$clixon_cli -1f $cfg -l o set num2 3" 255 '^CLI syntax error: "set num2 3": Number out of range: 3$'
expectfn "$clixon_cli -1f $cfg -l o set num2 3" 255 '^CLI syntax error: "set num2 3": Number 3 out of range: 4-4000$'

new "cli range test num2 1000 ok"
expectfn "$clixon_cli -1f $cfg -l o set num2 1000" 0 '^$'
Expand All @@ -380,7 +380,7 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><num2 xmlns="urn:example:clixon">3</num2></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf validate num2 3 fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>num2</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range: 3</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>num2</bad-element></error-info><error-severity>error</error-severity><error-message>Number 3 out of range: 4-4000</error-message></rpc-error></rpc-reply>]]>]]>$'

new "netconf range set num2 1000 ok"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><num2 xmlns="urn:example:clixon">1000</num2></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
Expand All @@ -392,7 +392,7 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><num2 xmlns="urn:example:clixon">5000</num2></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf validate num2 5000 fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>num2</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range: 5000</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>num2</bad-element></error-info><error-severity>error</error-severity><error-message>Number 5000 out of range: 4-4000</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>]]>]]>$"
Expand Down Expand Up @@ -420,7 +420,7 @@ EOF
#-------- num4 multiple ranges 1..2 | 42..50

new "cli range test num4 multiple 0 fail"
expectfn "$clixon_cli -1f $cfg -l o set num4 0" 255 '^CLI syntax error: "set num4 0": Number out of range: 0$'
expectfn "$clixon_cli -1f $cfg -l o set num4 0" 255 '^CLI syntax error: "set num4 0": Number 0 out of range: 1-2, 42-50$'

new "cli range test num4 multiple 2 ok"
expectfn "$clixon_cli -1f $cfg -l e set num4 2" 0 '^$'
Expand Down Expand Up @@ -477,7 +477,7 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><dec xmlns="urn:example:clixon">-3.59</dec></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf range dec64 -3.59 validate fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number -3.590 out of range'

new "netconf range dec64 -3.5"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><dec xmlns="urn:example:clixon">-3.500</dec></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
Expand All @@ -489,13 +489,13 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><dec xmlns="urn:example:clixon">-2</dec></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf range dec64 -2 validate fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number -2.000 out of range'

new "netconf range dec64 -0.001"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><dec xmlns="urn:example:clixon">-0.001</dec></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf range dec64 -0.001 validate fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number -0.001 out of range'

new "netconf range dec64 0.0"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><dec xmlns="urn:example:clixon">0.0</dec></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
Expand All @@ -507,12 +507,12 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><dec xmlns="urn:example:clixon">+0.001</dec></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf range dec64 +0.001 validate fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number out of range'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>dec</bad-element></error-info><error-severity>error</error-severity><error-message>Number 0.001 out of range'

#----------------string ranges---------------------
#-------- len1 single range (2)
new "cli length test len1 1 fail"
expectfn "$clixon_cli -1f $cfg -l o set len1 x" 255 '^CLI syntax error: "set len1 x": String length not within limits: 1$'
expectfn "$clixon_cli -1f $cfg -l o set len1 x" 255 '^CLI syntax error: "set len1 x": String length 1 out of range: 2-2$'

new "cli length test len1 2 OK"
expectfn "$clixon_cli -1f $cfg -l o set len1 xy" 0 '^$'
Expand All @@ -527,12 +527,12 @@ EOF
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><len1 xmlns="urn:example:clixon">x</len1></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"

new "netconf validate len1 1 wrong"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>len1</bad-element></error-info><error-severity>error</error-severity><error-message>string length out of range: 1</error-message></rpc-error></rpc-reply>]]>]]>$'
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><validate><source><candidate/></source></validate></rpc>]]>]]>" '^<rpc-reply><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>len1</bad-element></error-info><error-severity>error</error-severity><error-message>String length 1 out of range: 2-2</error-message></rpc-error></rpc-reply>]]>]]>$'

#-------- len2 range and blanks

new "cli length test len2 3 error"
expectfn "$clixon_cli -1f $cfg -l o set len2 ab" 255 '^CLI syntax error: "set len2 ab": String length not within limits: 2$'
expectfn "$clixon_cli -1f $cfg -l o set len2 ab" 255 '^CLI syntax error: "set len2 ab": String length 2 out of range: 4-4000$'

new "cli length test len2 42 ok"
expectfn "$clixon_cli -1f $cfg -l o set len2 hejhophdsakjhkjsadhkjsahdkjsad" 0 '^$'
Expand All @@ -547,7 +547,7 @@ EOF

#-------- len4 multiple ranges 2..3 | 20-29
new "cli length test len4 1 error"
expectfn "$clixon_cli -1f $cfg -l o set len4 a" 255 '^CLI syntax error: "set len4 a": String length not within limits: 1$'
expectfn "$clixon_cli -1f $cfg -l o set len4 a" 255 '^CLI syntax error: "set len4 a": String length 1 out of range: 2-3, 20-29$'

new "cli length test len4 2 ok"
expectfn "$clixon_cli -1f $cfg -l o set len4 ab" 0 '^$'
Expand Down Expand Up @@ -614,9 +614,12 @@ EOF
}

# Run without db cache
testrun false
testrun nocache

# Run with db cache
testrun true
testrun cache

# Run with zero-copy XXX does not work
#testrun cache-zerocopy

rm -rf $dir
Loading

0 comments on commit 97529a2

Please sign in to comment.