diff --git a/CHANGELOG.md b/CHANGELOG.md index 26c595f5..2d533e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,11 +99,16 @@ Developers may need to change their code ### Minor features +* Full RFC 7950 if-feature-expr support (Section 7.20.2) + * Previous implementation did not handle nested if-feature expressions + * As part of fixing: [YANG if-feature does not support nested boolean expression](https://github.com/clicon/clixon/issues/341) + * Added new yacc/lex parser for if-feature-expr string * [Feature Request: Log SSL events](https://github.com/clicon/clixon/issues/331) * Added syslog NOTICE on failed user certs ### Corrected Bugs +* Fixed: [YANG if-feature does not support nested boolean expression](https://github.com/clicon/clixon/issues/341) * Fixed: [RPC edit-config payloads are not fully validated](https://github.com/clicon/clixon/issues/337) ## 5.7.0 diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index c51cd979..78dae6ce 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -262,6 +262,7 @@ int yang_spec_dump(yang_stmt *yspec, int debuglevel); int if_feature(yang_stmt *yspec, char *module, char *feature); int ys_populate(yang_stmt *ys, void *arg); int ys_populate2(yang_stmt *ys, void *arg); +int yang_if_feature_parse(char *str, yang_stmt *ys, int linenum, int *enabled); int yang_apply(yang_stmt *yn, enum rfc_6020 key, yang_applyfn_t fn, int from, void *arg); int yang_datanode(yang_stmt *ys); int yang_abs_schema_nodeid(yang_stmt *ys, char *schema_nodeid, yang_stmt **yres); diff --git a/lib/src/Makefile.in b/lib/src/Makefile.in index 3d577404..10d32f3a 100644 --- a/lib/src/Makefile.in +++ b/lib/src/Makefile.in @@ -93,7 +93,8 @@ YACCOBJS = lex.clixon_xml_parse.o clixon_xml_parse.tab.o \ lex.clixon_xpath_parse.o clixon_xpath_parse.tab.o \ lex.clixon_api_path_parse.o clixon_api_path_parse.tab.o \ lex.clixon_instance_id_parse.o clixon_instance_id_parse.tab.o \ - lex.clixon_text_syntax_parse.o clixon_text_syntax_parse.tab.o + lex.clixon_text_syntax_parse.o clixon_text_syntax_parse.tab.o \ + lex.clixon_if_feature_parse.o clixon_if_feature_parse.tab.o # Generated src GENSRC = build.c @@ -125,12 +126,15 @@ clean: rm -f clixon_api_path_parse.tab.[ch] clixon_api_path_parse.[co] rm -f clixon_instance_id_parse.tab.[ch] clixon_instance_id_parse.[co] rm -f clixon_text_syntax_parse.tab.[ch] clixon_text_syntax_parse.[co] + rm -f clixon_if_feature_parse.tab.[ch] clixon_if_feature_parse.[co] rm -f lex.clixon_xml_parse.c rm -f lex.clixon_yang_parse.c rm -f lex.clixon_json_parse.c rm -f lex.clixon_xpath_parse.c rm -f lex.clixon_api_path_parse.c rm -f lex.clixon_instance_id_parse.c + rm -f lex.clixon_text_syntax_parse.c + rm -f lex.clixon_if_feature_parse.c rm -f *.gcda *.gcno *.gcov # coverage ############################################################################# @@ -236,6 +240,19 @@ clixon_text_syntax_parse.tab.c: clixon_text_syntax_parse.tab.h lex.clixon_text_syntax_parse.o : lex.clixon_text_syntax_parse.c clixon_text_syntax_parse.tab.h $(CC) $(INCLUDES) $(CPPFLAGS) $(CFLAGS) -Wno-error -c $< +# if-feature-expr parser +lex.clixon_if_feature_parse.c : clixon_if_feature_parse.l clixon_if_feature_parse.tab.h + $(LEX) -Pclixon_if_feature_parse clixon_if_feature_parse.l # -d is debug + +clixon_if_feature_parse.tab.h: clixon_if_feature_parse.y + $(YACC) -l -d -b clixon_if_feature_parse -p clixon_if_feature_parse clixon_if_feature_parse.y # -t is debug + +# extra rule to avoid parallell yaccs +clixon_if_feature_parse.tab.c: clixon_if_feature_parse.tab.h + +lex.clixon_if_feature_parse.o : lex.clixon_if_feature_parse.c clixon_if_feature_parse.tab.h + $(CC) $(INCLUDES) $(CPPFLAGS) $(CFLAGS) -Wno-error -c $< + distclean: clean rm -f Makefile *~ .depend diff --git a/lib/src/clixon_if_feature_parse.h b/lib/src/clixon_if_feature_parse.h new file mode 100644 index 00000000..5b9905b9 --- /dev/null +++ b/lib/src/clixon_if_feature_parse.h @@ -0,0 +1,66 @@ +/* + * + ***** BEGIN LICENSE BLOCK ***** + + Copyright (C) 2022 Olof Hagsand and Rubicon Communications, LLC(Netgate) + + This file is part of CLIXON. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + Alternatively, the contents of this file may be used under the terms of + the GNU General Public License Version 3 or later (the "GPL"), + in which case the provisions of the GPL are applicable instead + of those above. If you wish to allow use of your version of this file only + under the terms of the GPL, and not to allow others to + use your version of this file under the terms of Apache License version 2, + indicate your decision by deleting the provisions above and replace them with + the notice and other provisions required by the GPL. If you do not delete + the provisions above, a recipient may use your version of this file under + the terms of any one of the Apache License version 2 or the GPL. + + ***** END LICENSE BLOCK ***** + + * if-feature-expr RFC7950 14 + */ +#ifndef _CLIXON_IF_FEATURE_PARSE_H_ +#define _CLIXON_IF_FEATURE_PARSE_H_ + +/* + * Types + */ +/*! XML parser yacc handler struct */ +struct clixon_if_feature_parse_yacc { + char *if_parse_string; /* original (copy of) parse string */ + int if_linenum; /* Number of \n in parsed buffer */ + void *if_lexbuf; /* Internal parse buffer from lex */ + yang_stmt *if_ys; /* Yang statement, NULL if no check */ + int if_enabled; /* Result: 0: feature disabled, 1: enabled */ +}; +typedef struct clixon_if_feature_parse_yacc clixon_if_feature_yacc; + +/* + * Variables + */ +extern char *clixon_if_feature_parsetext; + +/* + * Prototypes + */ +int clixon_if_feature_parsel_init(clixon_if_feature_yacc *ya); +int clixon_if_feature_parsel_exit(clixon_if_feature_yacc *ya); +int clixon_if_feature_parsel_linenr(void); +int clixon_if_feature_parselex(void *); +int clixon_if_feature_parseparse(void *); + +#endif /* _CLIXON_IF_FEATURE_PARSE_H_ */ diff --git a/lib/src/clixon_if_feature_parse.l b/lib/src/clixon_if_feature_parse.l new file mode 100644 index 00000000..bfa413e4 --- /dev/null +++ b/lib/src/clixon_if_feature_parse.l @@ -0,0 +1,118 @@ +/* + * + ***** BEGIN LICENSE BLOCK ***** + + Copyright (C) 2022 Olof Hagsand and Rubicon Communications, LLC(Netgate) + + This file is part of CLIXON. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + Alternatively, the contents of this file may be used under the terms of + the GNU General Public License Version 3 or later (the "GPL"), + in which case the provisions of the GPL are applicable instead + of those above. If you wish to allow use of your version of this file only + under the terms of the GPL, and not to allow others to + use your version of this file under the terms of Apache License version 2, + indicate your decision by deleting the provisions above and replace them with + the notice and other provisions required by the GPL. If you do not delete + the provisions above, a recipient may use your version of this file under + the terms of any one of the Apache License version 2 or the GPL. + + ***** END LICENSE BLOCK ***** + + * if-feature-expr RFC7950 14 + */ + +%{ + +#include "clixon_config.h" + +#include +#include +#include + +#include "clixon_if_feature_parse.tab.h" /* generated file */ + +/* cligen */ +#include + +/* clicon */ +#include "clixon_queue.h" +#include "clixon_hash.h" +#include "clixon_handle.h" +#include "clixon_yang.h" +#include "clixon_xml.h" +#include "clixon_if_feature_parse.h" + +/* Redefine main lex function so that you can send arguments to it: _if is added to arg list */ +#define YY_DECL int clixon_if_feature_parselex(void *_if) + +/* Dont use input function (use user-buffer) */ +#define YY_NO_INPUT + +/* typecast macro */ +#define _IF ((clixon_if_feature_yacc *)_if) + +#undef clixon_xml_parsewrap +int clixon_if_feature_parsewrap(void) +{ + return 1; +} + +/* + */ + +%} + +%x COMMENT +%x STRING + +%% + +[ \t\r]* { return SEP; } +\n { _IF->if_linenum++; return SEP; } +\( { return *yytext; } +\) { return *yytext; } +not { return NOT; } +and { return AND; } +or { return OR; } +<> { return MY_EOF; } +[^\n\r \t\(\)]+ { + clixon_if_feature_parselval.string = strdup(yytext); + return TOKEN; } +. { return -1; } + +%% + +/*! Initialize XML scanner. + */ +int +clixon_if_feature_parsel_init(clixon_if_feature_yacc *ife) +{ + BEGIN(INITIAL); + ife->if_lexbuf = yy_scan_string (ife->if_parse_string); + if (0) + yyunput(0, ""); /* XXX: just to use unput to avoid warning */ + return 0; +} + +/*! Exit xml scanner */ +int +clixon_if_feature_parsel_exit(clixon_if_feature_yacc *ife) +{ + yy_delete_buffer(ife->if_lexbuf); + clixon_if_feature_parselex_destroy(); /* modern */ + + return 0; +} diff --git a/lib/src/clixon_if_feature_parse.y b/lib/src/clixon_if_feature_parse.y new file mode 100644 index 00000000..6ed149ac --- /dev/null +++ b/lib/src/clixon_if_feature_parse.y @@ -0,0 +1,214 @@ +/* + * + ***** BEGIN LICENSE BLOCK ***** + + Copyright (C) 2022 Olof Hagsand and Rubicon Communications, LLC(Netgate) + + This file is part of CLIXON. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + Alternatively, the contents of this file may be used under the terms of + the GNU General Public License Version 3 or later (the "GPL"), + in which case the provisions of the GPL are applicable instead + of those above. If you wish to allow use of your version of this file only + under the terms of the GPL, and not to allow others to + use your version of this file under the terms of Apache License version 2, + indicate your decision by deleting the provisions above and replace them with + the notice and other provisions required by the GPL. If you do not delete + the provisions above, a recipient may use your version of this file under + the terms of any one of the Apache License version 2 or the GPL. + + ***** END LICENSE BLOCK ***** + + * if-feature-expr RFC7950 14 + if-feature-expr = if-feature-term + [sep or-keyword sep if-feature-expr] + + if-feature-term = if-feature-factor + [sep and-keyword sep if-feature-term] + + if-feature-factor = not-keyword sep if-feature-factor / + "(" optsep if-feature-expr optsep ")" / + identifier-ref-arg + Called twice: + 1. In parsing just for syntac checks (dont do feature-check) + 2. In populate when all yangs are resolved, then do semantic feature-check + */ +%union { + char *string; + void *stack; + int number; +} + +%token MY_EOF +%token AND +%token NOT +%token OR +%token TOKEN +%token SEP + +%type iffactor ifexpr + +%start top + +%lex-param {void *_if} /* Add this argument to parse() and lex() function */ +%parse-param {void *_if} + +%{ + +/* typecast macro */ +#define _IF ((clixon_if_feature_yacc *)_if) + +#include +#include +#include +#include +#include +#include + +/* cligen */ +#include + +/* clicon */ +#include "clixon_err.h" +#include "clixon_log.h" +#include "clixon_queue.h" +#include "clixon_hash.h" +#include "clixon_string.h" +#include "clixon_handle.h" +#include "clixon_yang.h" +#include "clixon_xml.h" +#include "clixon_yang_module.h" +#include "clixon_xml_vec.h" +#include "clixon_data.h" +#include "clixon_if_feature_parse.h" + +/* Enable for debugging, steals some cycles otherwise */ +#if 0 +#define _PARSE_DEBUG(s) clicon_debug(1,(s)) +#else +#define _PARSE_DEBUG(s) +#endif + +void +clixon_if_feature_parseerror(void *arg, + char *s) +{ + clixon_if_feature_yacc *ife = (clixon_if_feature_yacc *)arg; + + clicon_err(OE_XML, XMLPARSE_ERRNO, "if_feature_parse: line %d: %s: at or before: %s", + ife->if_linenum, + s, + clixon_if_feature_parsetext); + return; +} + +/*! Check if feature "str" is enabled or not in context of yang node ys + */ +static int +if_feature_check(char *str, + yang_stmt *ys) +{ + int retval = -1; + char *prefix = NULL; + char *feature = NULL; + yang_stmt *ymod; /* module yang node */ + yang_stmt *yfeat; /* feature yang node */ + cg_var *cv; + + if (ys == NULL) + return 0; + if (nodeid_split(str, &prefix, &feature) < 0) + goto done; + /* Specifically need to handle? strcmp(prefix, myprefix)) */ + if (prefix == NULL) + ymod = ys_module(ys); + else + ymod = yang_find_module_by_prefix(ys, prefix); + if (ymod == NULL) + goto done; + /* Check if feature exists, and is set, otherwise remove */ + if ((yfeat = yang_find(ymod, Y_FEATURE, feature)) == NULL){ + clicon_err(OE_YANG, EINVAL, "Yang module %s has IF_FEATURE %s, but no such FEATURE statement exists", + ymod?yang_argument_get(ymod):"none", + feature); + goto done; + } + /* Check if this feature is enabled or not + * Continue loop to catch unbound features and make verdict at end + */ + cv = yang_cv_get(yfeat); + if (cv == NULL || !cv_bool_get(cv)) /* disabled */ + retval = 0; + else /* enabled */ + retval = 1; + done: + if (prefix) + free(prefix); + if (feature) + free(feature); + return retval; +} + +%} + +%% + +top : ifexpr MY_EOF + { + _PARSE_DEBUG("top->expr"); + _IF->if_enabled = $1; + YYACCEPT; + } + ; + +ifexpr : iffactor sep1 OR sep1 ifexpr + { + _PARSE_DEBUG("expr->factor sep OR sep expr"); + $$ = $1 || $5; + } + | iffactor sep1 AND sep1 ifexpr + { + _PARSE_DEBUG("expr->factor sep AND sep expr"); + $$ = $1 && $5; + } + | iffactor + { + _PARSE_DEBUG("expr->factor"); + $$ = $1; + } + ; + +iffactor : NOT sep1 iffactor { _PARSE_DEBUG("factor-> NOT sep factor"); + $$ = !$3; } + | '(' optsep ifexpr optsep ')' + { _PARSE_DEBUG("factor-> ( optsep expr optsep )"); + $$ = $3; } + | TOKEN { + _PARSE_DEBUG("factor->TOKEN"); + if (_IF->if_ys == NULL) $$ = 0; + else if (($$ = if_feature_check($1, _IF->if_ys)) < 0) YYERROR; + } + ; + +optsep : sep1 { _PARSE_DEBUG("optsep->sep"); } + | { _PARSE_DEBUG("optsep->"); } + ; + +sep1 : sep1 SEP { _PARSE_DEBUG("sep->sep SEP"); } + | SEP { _PARSE_DEBUG("sep->SEP"); } + ; + +%% + diff --git a/lib/src/clixon_text_syntax_parse.y b/lib/src/clixon_text_syntax_parse.y index cf22980c..c099a6e2 100644 --- a/lib/src/clixon_text_syntax_parse.y +++ b/lib/src/clixon_text_syntax_parse.y @@ -82,7 +82,7 @@ #include "clixon_text_syntax_parse.h" /* Enable for debugging, steals some cycles otherwise */ -#if 1 +#if 0 #define _PARSE_DEBUG(s) clicon_debug(1,(s)) #else #define _PARSE_DEBUG(s) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 03ba247a..e8f6d5c9 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -80,6 +80,7 @@ #include "clixon_plugin.h" #include "clixon_data.h" #include "clixon_options.h" +#include "clixon_if_feature_parse.h" #include "clixon_yang_parse.h" #include "clixon_yang_parse_lib.h" #include "clixon_yang_cardinality.h" @@ -2782,164 +2783,42 @@ ys_populate2(yang_stmt *ys, return retval; } -/*! Handle complexity of if-feature node - * @param[in] h Clixon handle - * @param[in] ys Yang if-feature statement - * @retval -1 Error - * @retval 0 Feature not enabled: remove yt - * @retval 1 OK - * @note if-feature syntax is restricted to single, and, or, syntax, such as "a or b" - * but RFC7950 allows for nested expr/term/factor syntax. - * XXX This should really be parsed in yang/lex. - * XXX: work-around for https://github.com/clicon/clixon/issues/341 +/*! Invoke yang if-feature sub-parser on string + * + * @param[in] str If-feature-expr string + * @param[in] ys Yang if-feature statement + * @param[in] linenum Line number context + * @param[out] enabled 0: Disabled, 1: Enabled + * @retval 0 OK + * @retval -1 Error */ -static int -yang_if_feature(clicon_handle h, - yang_stmt *ys) +int +yang_if_feature_parse(char *str, + yang_stmt *ys, + int linenum, + int *enabled) { - int retval = -1; - char **vec = NULL; - int nvec; - char *f; - int i; - int j; - char *prefix = NULL; - char *feature = NULL; - yang_stmt *ymod; /* module yang node */ - yang_stmt *yfeat; /* feature yang node */ - int opand = -1; /* -1:not set, 0:or, 1:and */ - int enabled = 0; - cg_var *cv; + int retval = -1; + clixon_if_feature_yacc ife = {0,}; - if ((vec = clicon_strsep(ys->ys_argument, " \t\r\n", &nvec)) == NULL) + clicon_debug(2, "%s %s", __FUNCTION__, str); + ife.if_parse_string = str; + ife.if_linenum = linenum; + ife.if_ys = ys; + if (clixon_if_feature_parsel_init(&ife) < 0) goto done; - /* Two steps: first detect operators - * Step 1: support "a" or "a or b or c" or "a and b and c " - */ - j = 0; - for (i=0; iys_argument); - goto done; - } - opand = 0; - break; - case 0: - break; - case 1: - goto ok; - clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument); - goto done; - break; - } - } - else if (strcmp(f, "and") == 0){ - switch (opand){ - case -1: - if (i != 1){ - goto ok; - clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument); - goto done; - } - opand = 1; - break; - case 0: - goto ok; - clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument); - goto done; - break; - case 1: - break; - } - } - else{ - goto ok; - clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument); - goto done; - } - } /* for step 1 */ - if (j%2 == 0){ /* Must be odd: eg a / "a or b" etc */ - goto ok; - clicon_err(OE_YANG, EINVAL, "Syntax error IF_FEATURE \"%s\" (only single if-feature-expr and/or lists allowed)", ys->ys_argument); + if (clixon_if_feature_parseparse(&ife) != 0) { /* yacc returns 1 on error */ + clicon_log(LOG_NOTICE, "If-feature error: line %d", ife.if_linenum); + if (clicon_errno == 0) + clicon_err(OE_JSON, 0, "If-feature parser error with no error code (should not happen)"); goto done; } - - if (opand == -1) /* Uninitialized means single operand */ - opand = 1; - if (opand) /* if AND, start as enabled, if OR start as disabled */ - enabled = 1; - else - enabled = 0; - /* Step 2: Boolean operations on operands */ - j = 0; - for (i=0; iys_len){ ys = yt->ys_stmt[i]; if (ys->ys_keyword == Y_IF_FEATURE){ - if ((ret = yang_if_feature(h, ys)) < 0) + /* Parse the if-feature-expr string */ + ret = 0; + if (yang_if_feature_parse(yang_argument_get(ys), ys, 1, &ret) < 0) goto done; + clicon_debug(1, "%s %s %d", __FUNCTION__, yang_argument_get(ys), ret); if (ret == 0) goto disabled; } diff --git a/lib/src/clixon_yang_parse.y b/lib/src/clixon_yang_parse.y index c3a6b587..3a387b07 100644 --- a/lib/src/clixon_yang_parse.y +++ b/lib/src/clixon_yang_parse.y @@ -316,10 +316,9 @@ ysp_add(clixon_yang_yacc *yy, yang_argument_set(ys, argument); if (yn_insert(yn, ys) < 0) /* Insert into hierarchy */ goto err; + yang_linenum_set(ys, yy->yy_linenum); /* For error/debugging */ if (ys_parse_sub(ys, extra) < 0) /* Check statement-specific syntax */ goto err2; /* dont free since part of tree */ - yang_linenum_set(ys, yy->yy_linenum); /* For error/debugging */ -// done: return ys; err: if (ys) diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index e4ae0e23..1eb9b3ae 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -1956,6 +1956,13 @@ ys_parse_sub(yang_stmt *ys, goto done; } break; + case Y_IF_FEATURE: + /* Invoke next level parser on if-feature-expr string. Note do not send ys since + * pass 1 is not yet resolved, only check syntax, actual feature check made in next pass + */ + if (yang_if_feature_parse(yang_argument_get(ys), NULL, yang_linenum_get(ys), NULL) < 0) + goto done; + break; case Y_UNKNOWN:{ /* save (optional) argument in ys_cv */ if (extra == NULL) break; diff --git a/test/test_feature.sh b/test/test_feature.sh index dcd8e0d4..accdb743 100755 --- a/test/test_feature.sh +++ b/test/test_feature.sh @@ -117,6 +117,23 @@ module example{ description "Not enabled"; type "string"; } + leaf m9{ + if-feature "(A or B " + + "or B1) and A1"; + description "Enabled"; + type "string"; + } + leaf m10{ + if-feature "(A and A1 " + + "and B1) or not A"; + description "Disabled"; + type "string"; + } + leaf m11{ + if-feature "not (A or B)"; + description "Disabled"; + type "string"; + } } EOF @@ -196,6 +213,9 @@ testrun m5 false testrun m6 false testrun m7 true testrun m8 false +testrun m9 true +testrun m10 false +testrun m11 false # This test has been broken up into all different modules instead of one large # reply since the modules change so often