Hello The following patch set addresses ticket https://fedorahosted.org/sssd/ticket/2108
Please see commit messages for more details about what each patch does. Please follow the order of the patches in the set. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/
From 7ab8b93be5a6e1aa91b47d06d5bcd8061e29735e Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Sun, 6 Oct 2013 11:31:50 -0400 Subject: [PATCH 2/5] [INI] Extend error set and add parsing error One more parsing error is now needed. --- ini/ini_configobj.h | 11 +++++++---- ini/ini_print.c | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/ini/ini_configobj.h b/ini/ini_configobj.h index 1c04d8a..9a4eb58 100644 --- a/ini/ini_configobj.h +++ b/ini/ini_configobj.h @@ -175,7 +175,8 @@ enum ERR_PARSE { ERR_SPECIAL, /**< Line contains invalid characters (Error). */ ERR_TAB, /**< Line starts with tab when it should not (Error). */ - ERR_MAXPARSE = ERR_TAB /**< Special value. Size of the error array. */ + ERR_BADCOMMENT, /**< End of file while processing comment (Error). */ + ERR_MAXPARSE = ERR_BADCOMMENT /**< Special value. Size of the error array. */ }; /** @@ -332,11 +333,13 @@ enum ERR_PARSE { */ /** @brief Suppress multi line value wrapping */ -#define INI_PARSE_NOWRAP 0x0001 +#define INI_PARSE_NOWRAP 0x0001 /** @brief No spaces are allowed to the left of the key */ -#define INI_PARSE_NOSPACE 0x0002 +#define INI_PARSE_NOSPACE 0x0002 /** @brief No tabs are allowed to the left of the key */ -#define INI_PARSE_NOTAB 0x0004 +#define INI_PARSE_NOTAB 0x0004 +/** @brief Do not allow C-style comments */ +#define INI_PARSE_NO_C_COMMENTS 0x0008 /** * @} diff --git a/ini/ini_print.c b/ini/ini_print.c index 1812cd6..4df934c 100644 --- a/ini/ini_print.c +++ b/ini/ini_print.c @@ -126,7 +126,9 @@ static const char *parsing_error_str(int parsing_error) _("Invalid character at the " "beginning of the line."), _("Invalid tab character at the " - "beginning of the line.") + "beginning of the line."), + _("Incomplete comment at the " + "end of the file.") }; /* Check the range */ -- 1.7.1
From 5de73ecb026b226da7a6b27599921477f3b0ab4a Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Sun, 6 Oct 2013 11:35:23 -0400 Subject: [PATCH 3/5] [INI] Process c-style comments Patch adds parser check for extended set of comment types. It also processes error if last comment is incomplete. --- ini/ini_parse.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 87 insertions(+), 6 deletions(-) diff --git a/ini/ini_parse.c b/ini/ini_parse.c index 0e462a1..6374887 100644 --- a/ini/ini_parse.c +++ b/ini/ini_parse.c @@ -79,6 +79,7 @@ struct parser_obj { struct ini_comment *ic; char *last_read; uint32_t last_read_len; + int inside_comment; char *key; uint32_t key_len; struct ref_array *raw_lines; @@ -257,6 +258,7 @@ static int parser_create(struct ini_cfgobj *co, new_po->seclinenum = 0; new_po->last_read = NULL; new_po->last_read_len = 0; + new_po->inside_comment = 0; new_po->key = NULL; new_po->key_len = 0; new_po->raw_lines = NULL; @@ -321,7 +323,11 @@ static int parser_read(struct parser_obj *po) if (res == -1) { if (feof(po->file)) { TRACE_FLOW_STRING("Read nothing", ""); - action = PARSE_POST; + if (po->inside_comment) { + action = PARSE_ERROR; + po->last_error = ERR_BADCOMMENT; + } + else action = PARSE_POST; } else { TRACE_ERROR_STRING("Error reading", ""); @@ -1294,6 +1300,73 @@ static int handle_section(struct parser_obj *po, uint32_t *action) } +static int check_for_comment(char *buffer, + uint32_t buffer_len, + int allow_c_comments, + int *inside_comment) +{ + int pos; + int is_comment = 0; + + TRACE_FLOW_ENTRY(); + + if (*inside_comment) { + /* We are already inside the comment + * and we are looking for the end of the comment + */ + if (buffer_len) { + pos = buffer_len - 1; + while(isspace(buffer[pos]) && pos > 0) pos--; + + /* Check for comment at the end of the line */ + if ((pos > 1) && + (buffer[pos] == '/') && + (buffer[pos - 1] == '*')) { + *inside_comment = 0; + } + } + is_comment = 1; + } + else { + /* We do not allow spaces in front of comments + * so we expect the comment to start right away. + */ + if ((buffer[0] == '\0') || + (buffer[0] == ';') || + (buffer[0] == '#')) { + is_comment = 1; + } + else if ((allow_c_comments) && (buffer_len > 1)) { + if (buffer[0] == '/') { + if (buffer[1] == '/') is_comment = 1; + else if (buffer[1] == '*') { + + is_comment = 1; + *inside_comment = 1; + + /* Here we need to check whether this comment ends + * on this line or not + */ + pos = buffer_len - 1; + while(isspace(buffer[pos]) && pos > 0) pos--; + + /* Check for comment at the end of the line + * but make sure we have at least two asterisks + */ + if ((pos > 2) && + (buffer[pos] == '/') && + (buffer[pos - 1] == '*')) { + *inside_comment = 0; + } + } + } + } + } + + TRACE_FLOW_EXIT(); + return is_comment; +} + /* Inspect the line */ static int parser_inspect(struct parser_obj *po) { @@ -1302,9 +1375,13 @@ static int parser_inspect(struct parser_obj *po) TRACE_FLOW_ENTRY(); - if ((*(po->last_read) == '\0') || - (*(po->last_read) == ';') || - (*(po->last_read) == '#')) { + TRACE_INFO_STRING("Buffer:", po->last_read); + TRACE_INFO_NUMBER("In comment:", po->inside_comment); + + if (check_for_comment(po->last_read, + po->last_read_len, + !(po->parse_flags & INI_PARSE_NO_C_COMMENTS), + &(po->inside_comment))) { error = handle_comment(po, &action); if (error) { @@ -1453,8 +1530,12 @@ static int parser_error(struct parser_obj *po) return error; } - /* Exit if there was an error parsing file */ - if (po->error_level == INI_STOP_ON_ANY) { + if (po->last_error == ERR_BADCOMMENT) { + /* Avoid endless loop */ + action = PARSE_DONE; + po->ret = EIO; + } + else if (po->error_level == INI_STOP_ON_ANY) { action = PARSE_DONE; if (po->last_error & INI_WARNING) po->ret = EILSEQ; else po->ret = EIO; -- 1.7.1
From f07efe3fff76c4830ffe19509ce2695e763b300b Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Sun, 6 Oct 2013 11:52:48 -0400 Subject: [PATCH 5/5] [INI] Unit test for c-style comments --- ini/ini_parse_ut.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-) diff --git a/ini/ini_parse_ut.c b/ini/ini_parse_ut.c index 7dd7518..3a523af 100644 --- a/ini/ini_parse_ut.c +++ b/ini/ini_parse_ut.c @@ -2944,6 +2944,72 @@ int trim_test(void) INIOUT(printf("\n<==== TRIM TEST END =====>\n\n")); return EOK; } + +int comment_test(void) +{ + int error; + struct ini_cfgfile *file_ctx = NULL; + struct ini_cfgobj *ini_config = NULL; + char **error_list = NULL; + char infile[PATH_MAX]; + char *srcdir = NULL; + int err_count = 0; + + INIOUT(printf("\n\n<==== COMMENT TEST START =====>\n")); + + srcdir = getenv("srcdir"); + snprintf(infile, PATH_MAX, "%s/ini/ini.d/comment.conf", + (srcdir == NULL) ? "." : srcdir); + + + INIOUT(printf("Reading file %s\n", infile)); + error = ini_config_file_open(infile, + 0, + &file_ctx); + if (error) { + printf("Failed to open file for reading. Error %d.\n", error); + return error; + } + + INIOUT(printf("Creating configuration object\n")); + error = ini_config_create(&ini_config); + if (error) { + printf("Failed to create object. Error %d.\n", error); + ini_config_file_destroy(file_ctx); + return error; + } + INIOUT(printf("Parsing\n")); + error = ini_config_parse(file_ctx, + INI_STOP_ON_NONE, + 0, + 0, + ini_config); + if (error) { + INIOUT(printf("Failed to parse configuration. " + "Error %d.\n", error)); + err_count = ini_config_error_count(ini_config); + if (err_count) { + INIOUT(printf("Errors detected while parsing: %s\n", + ini_config_get_filename(file_ctx))); + ini_config_get_errors(ini_config, &error_list); + INIOUT(ini_config_print_errors(stdout, error_list)); + ini_config_free_errors(error_list); + } + } + + INIOUT(col_debug_collection(ini_config->cfg, COL_TRAVERSE_DEFAULT)); + ini_config_file_destroy(file_ctx); + ini_config_destroy(ini_config); + + if(err_count != 4) { + printf("Expected 4 errors got: %d\n", err_count); + return 1; + } + + INIOUT(printf("\n<==== COMMENT TEST END =====>\n\n")); + return EOK; +} + /* Main function of the unit test */ int main(int argc, char *argv[]) { @@ -2958,6 +3024,7 @@ int main(int argc, char *argv[]) get_test, space_test, trim_test, + comment_test, NULL }; test_fn t; int i = 0; -- 1.7.1
From a2d60d9c226ac9de4d58fa2dc1018be89904dd4e Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Sun, 6 Oct 2013 11:38:56 -0400 Subject: [PATCH 4/5] [INI] Test files for unit test Update existing file with c-style comments. Add another one for negative testing. --- ini/ini.d/comment.conf | 31 +++++++++++++++++++++++++++++++ ini/ini.d/real.conf | 6 ++++++ 2 files changed, 37 insertions(+), 0 deletions(-) create mode 100644 ini/ini.d/comment.conf diff --git a/ini/ini.d/comment.conf b/ini/ini.d/comment.conf new file mode 100644 index 0000000..e148d23 --- /dev/null +++ b/ini/ini.d/comment.conf @@ -0,0 +1,31 @@ +/* C-style comment 1 */ +/* + * C-style comment 2 */ + +/* +// + * C-style comment 2 +// +commented_out_key = some_value +*/ + +# The following should be viewed as comment +# so no error is expected +/*/ = /*/ + +# The following should be viewed as key +# so no error is expected +/./ = // + +# The following should be viewed as key +# but it would be error due to spaces + /./* = /./ + + // This should be an error becuase parser thinks it should be a KVP line + + /* Same with this one */ + +/* This comment should procduce an error + + + diff --git a/ini/ini.d/real.conf b/ini/ini.d/real.conf index 78b5874..0458c60 100644 --- a/ini/ini.d/real.conf +++ b/ini/ini.d/real.conf @@ -1,6 +1,9 @@ +// This is a real file with some comments + [config] version = 0.1 +/**/ [monitor] description = Monitor Configuration sbusTimeout = 10 @@ -8,6 +11,9 @@ sbusAddress = unix:path=/var/lib/sss/pipes/private/dbus servicePingTime = 10 bad_number = 5a +/* Service section defines + * which service are allowed. + */ [services] description = Local service configuration activeServices = dp, nss, pam, info -- 1.7.1
From 8d3d072aca6761303d3c5817d530adda5bdd3dfa Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Sun, 6 Oct 2013 11:29:25 -0400 Subject: [PATCH 1/5] [INI] Do not check validity of comments The validity of comment was checked on the low level. Now when we support c-style comments any line can be a part of comment so vality check needs to be removed. Comment object should trust parser to do the right thing. --- ini/ini_comment.c | 47 ----------------------------------------------- 1 files changed, 0 insertions(+), 47 deletions(-) diff --git a/ini/ini_comment.c b/ini/ini_comment.c index 3d25562..53a84f8 100644 --- a/ini/ini_comment.c +++ b/ini/ini_comment.c @@ -200,45 +200,6 @@ int ini_comment_copy(struct ini_comment *ic, return error; } -/* Is the comment valid? */ -static int ini_comment_is_valid(const char *line) -{ - int i; - - TRACE_FLOW_ENTRY(); - - /* Null is ok */ - if (!line) { - TRACE_FLOW_STRING("ini_comment_is_valid", "Exit - NULL str"); - return 1; - } - - /* Empty is Ok or starts with a special symbol */ - if ((line[0] == '\0') || - (line[0] == ';') || - (line[0] == '#')) { - TRACE_FLOW_STRING("ini_comment_is_valid", "Exit - empty or comment"); - return 1; - } - - /* All spaces is Ok too */ - TRACE_INFO_STRING("Line to eval", line); - - i = 0; - while (line[i] != '\0') { - if (!isspace(line[i])) { - TRACE_ERROR_STRING("ini_comment_is_valid", "Invalid comment"); - return 0; - } - i++; - } - - TRACE_FLOW_STRING("ini_comment_is_valid", "Exit - empty str"); - return 1; - -} - - /* * Modify the comment object */ @@ -281,14 +242,6 @@ static int ini_comment_modify(struct ini_comment *ic, memcpy(&input, &line, sizeof(char *)); if (mode != INI_COMMENT_MODE_REMOVE) { - /* - * Check that provided line is a comment or an empty line. - * Can be NULL too. - */ - if (!ini_comment_is_valid(input)) { - TRACE_ERROR_NUMBER("Invalid comment", EINVAL); - return EINVAL; - } error = simplebuffer_alloc(&elem); if (error) { -- 1.7.1
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
