Hello, Thanks for your reply and merge.
Now I'll go on a work of fixing "peers". Thanks, Hyeonggeun. On Thu, Jan 8, 2026 at 2:33 AM William Lallemand <[email protected]> wrote: > Hello, > > Thank you, looks good to me. I just changed a little bit your commit > message and wrap it to 80 columns. I also added a > fix before your commit on the error message which was mentioning "users" > instead of "userlist" when a keyword was not > found ! > > Merged in master! > > Regards, > > On Tue, Jan 06, 2026 at 01:22:12PM +0900, Hyeonggeun Oh wrote: > > Subject: [PATCH] MINOR: cfgparse: Refactor "userlist" parser to print it > in -dKall operation > > Hello haproxy team. > > This patch covers issue https://github.com/haproxy/haproxy/issues/3221. > > > > Before working keyword "peers", which has bigger loggic than "userlist", > I've fixed "userlist" keyword first to check and make consensus with > maintainers. > > > > The parser for the "userlist" section did not use the standard keyword > registration mechanism. Instead, it relied on a series of strcmp() > comparisons to identify keywords such as "group" and "user". > > > > This had two main drawbacks: > > 1. The keywords were not discoverable by the "-dKall" dump option, > making it difficult for users to see all available keywords for the section. > > 2. The implementation was inconsistent with the parsers for other > sections, which have been progressively refactored to use the standard > cfg_kw_list infrastructure. > > > > This patch refactors the userlist parser to align it with the project's > standard conventions. > > > > The parsing logic for the "group" and "user" keywords has been extracted > from the if/else block in cfg_parse_users() into two new dedicated > functions: > > - cfg_parse_users_group() > > - cfg_parse_users_user() > > > > These two keywords are now registered via a dedicated cfg_kw_list, > making them visible to the rest of the HAPorxy ecosystem, including the > -dKall dump. > > > > I hope that this change makes the userlist parser cleaner, and > furthermore, provides a clear pattern for refactoring other sections like > "peers" or the other keywords. > > --- > > src/cfgparse.c | 259 ++++++++++++++++++++++++++++--------------------- > > 1 file changed, 146 insertions(+), 113 deletions(-) > > > > diff --git a/src/cfgparse.c b/src/cfgparse.c > > index 5a7ec7405..08cd3e0d1 100644 > > --- a/src/cfgparse.c > > +++ b/src/cfgparse.c > > @@ -1389,153 +1389,184 @@ cfg_parse_users(const char *file, int linenum, > char **args, int kwm) > > newul->next = userlist; > > userlist = newul; > > > > - } else if (strcmp(args[0], "group") == 0) { /* new group */ > > - int cur_arg; > > - const char *err; > > - struct auth_groups *ag; > > + } else { > > + const struct cfg_kw_list *kwl; > > + char *errmsg = NULL; > > + int index; > > > > - if (!*args[1]) { > > - ha_alert("parsing [%s:%d]: '%s' expects <name> as > arguments.\n", > > - file, linenum, args[0]); > > - err_code |= ERR_ALERT | ERR_FATAL; > > - goto out; > > + list_for_each_entry(kwl, &cfg_keywords.list, list) { > > + for (index = 0; kwl->kw[index].kw; index++) { > > + if ((kwl->kw[index].section & > CFG_USERLIST) && > > + (strcmp(kwl->kw[index].kw, > args[0]) == 0)) { > > + err_code |= > kwl->kw[index].parse(args, CFG_USERLIST, NULL, NULL, file, linenum, > &errmsg); > > + if (errmsg) { > > + ha_alert("parsing > [%s:%d] : %s\n", file, linenum, errmsg); > > + ha_free(&errmsg); > > + } > > + goto out; > > + } > > + } > > } > > > > - err = invalid_char(args[1]); > > - if (err) { > > - ha_alert("parsing [%s:%d]: character '%c' is not > permitted in '%s' name '%s'.\n", > > - file, linenum, *err, args[0], args[1]); > > - err_code |= ERR_ALERT | ERR_FATAL; > > - goto out; > > - } > > + ha_alert("parsing [%s:%d]: unknown keyword '%s' in '%s' > section\n", file, linenum, args[0], "users"); > > + err_code |= ERR_ALERT | ERR_FATAL; > > + } > > > > - if (!userlist) > > - goto out; > > +out: > > + return err_code; > > +} > > > > - for (ag = userlist->groups; ag; ag = ag->next) > > - if (strcmp(ag->name, args[1]) == 0) { > > - ha_warning("parsing [%s:%d]: ignoring > duplicated group '%s' in userlist '%s'.\n", > > - file, linenum, args[1], > userlist->name); > > - err_code |= ERR_ALERT; > > - goto out; > > - } > > +int cfg_parse_users_group(char **args, int section_type, struct proxy > *curproxy, const struct proxy *defproxy, const char *file, int linenum, > char **err) > > +{ > > + int cur_arg; > > + const char *err_str; > > + struct auth_groups *ag; > > + int err_code = 0; > > > > - ag = calloc(1, sizeof(*ag)); > > - if (!ag) { > > - ha_alert("parsing [%s:%d]: out of memory.\n", > file, linenum); > > - err_code |= ERR_ALERT | ERR_ABORT; > > - goto out; > > - } > > + if (!*args[1]) { > > + ha_alert("parsing [%s:%d]: '%s' expects <name> as > arguments.\n", > > + file, linenum, args[0]); > > + err_code |= ERR_ALERT | ERR_FATAL; > > + goto out; > > + } > > > > - ag->name = strdup(args[1]); > > - if (!ag->name) { > > - ha_alert("parsing [%s:%d]: out of memory.\n", > file, linenum); > > - err_code |= ERR_ALERT | ERR_ABORT; > > - free(ag); > > + err_str = invalid_char(args[1]); > > + if (err_str) { > > + ha_alert("parsing [%s:%d]: character '%c' is not permitted > in '%s' name '%s'.\n", > > + file, linenum, *err_str, args[0], args[1]); > > + err_code |= ERR_ALERT | ERR_FATAL; > > + goto out; > > + } > > + > > + if (!userlist) > > + goto out; > > + > > + for (ag = userlist->groups; ag; ag = ag->next) > > + if (strcmp(ag->name, args[1]) == 0) { > > + ha_warning("parsing [%s:%d]: ignoring duplicated > group '%s' in userlist '%s'.\n", > > + file, linenum, args[1], > userlist->name); > > + err_code |= ERR_ALERT; > > goto out; > > } > > > > - cur_arg = 2; > > + ag = calloc(1, sizeof(*ag)); > > + if (!ag) { > > + ha_alert("parsing [%s:%d]: out of memory.\n", file, > linenum); > > + err_code |= ERR_ALERT | ERR_ABORT; > > + goto out; > > + } > > > > - while (*args[cur_arg]) { > > - if (strcmp(args[cur_arg], "users") == 0) { > > - if (ag->groupusers) { > > - ha_alert("parsing [%s:%d]: 'users' > option already defined in '%s' name '%s'.\n", > > - file, linenum, args[0], > args[1]); > > - err_code |= ERR_ALERT | ERR_FATAL; > > - free(ag->groupusers); > > - free(ag->name); > > - free(ag); > > - goto out; > > - } > > - ag->groupusers = strdup(args[cur_arg + 1]); > > - cur_arg += 2; > > - continue; > > - } else { > > - ha_alert("parsing [%s:%d]: '%s' only > supports 'users' option.\n", > > - file, linenum, args[0]); > > + ag->name = strdup(args[1]); > > + if (!ag->name) { > > + ha_alert("parsing [%s:%d]: out of memory.\n", file, > linenum); > > + err_code |= ERR_ALERT | ERR_ABORT; > > + free(ag); > > + goto out; > > + } > > + > > + cur_arg = 2; > > + > > + while (*args[cur_arg]) { > > + if (strcmp(args[cur_arg], "users") == 0) { > > + if (ag->groupusers) { > > + ha_alert("parsing [%s:%d]: 'users' option > already defined in '%s' name '%s'.\n", > > + file, linenum, args[0], > args[1]); > > err_code |= ERR_ALERT | ERR_FATAL; > > free(ag->groupusers); > > free(ag->name); > > free(ag); > > goto out; > > } > > + ag->groupusers = strdup(args[cur_arg + 1]); > > + cur_arg += 2; > > + continue; > > + } else { > > + ha_alert("parsing [%s:%d]: '%s' only supports > 'users' option.\n", > > + file, linenum, args[0]); > > + err_code |= ERR_ALERT | ERR_FATAL; > > + free(ag->groupusers); > > + free(ag->name); > > + free(ag); > > + goto out; > > } > > + } > > > > - ag->next = userlist->groups; > > - userlist->groups = ag; > > + ag->next = userlist->groups; > > + userlist->groups = ag; > > > > - } else if (strcmp(args[0], "user") == 0) { /* new > user */ > > - struct auth_users *newuser; > > - int cur_arg; > > +out: > > + return err_code; > > +} > > > > - if (!*args[1]) { > > - ha_alert("parsing [%s:%d]: '%s' expects <name> as > arguments.\n", > > - file, linenum, args[0]); > > - err_code |= ERR_ALERT | ERR_FATAL; > > - goto out; > > - } > > - if (!userlist) > > - goto out; > > +int cfg_parse_users_user(char **args, int section_type, struct proxy > *curproxy, const struct proxy *defproxy, const char *file, int linenum, > char **err) > > +{ > > + struct auth_users *newuser; > > + int cur_arg; > > + int err_code = 0; > > > > - for (newuser = userlist->users; newuser; newuser = > newuser->next) > > - if (strcmp(newuser->user, args[1]) == 0) { > > - ha_warning("parsing [%s:%d]: ignoring > duplicated user '%s' in userlist '%s'.\n", > > - file, linenum, args[1], > userlist->name); > > - err_code |= ERR_ALERT; > > - goto out; > > - } > > + if (!*args[1]) { > > + ha_alert("parsing [%s:%d]: '%s' expects <name> as > arguments.\n", > > + file, linenum, args[0]); > > + err_code |= ERR_ALERT | ERR_FATAL; > > + goto out; > > + } > > + if (!userlist) > > + goto out; > > > > - newuser = calloc(1, sizeof(*newuser)); > > - if (!newuser) { > > - ha_alert("parsing [%s:%d]: out of memory.\n", > file, linenum); > > - err_code |= ERR_ALERT | ERR_ABORT; > > + for (newuser = userlist->users; newuser; newuser = newuser->next) > > + if (strcmp(newuser->user, args[1]) == 0) { > > + ha_warning("parsing [%s:%d]: ignoring duplicated > user '%s' in userlist '%s'.\n", > > + file, linenum, args[1], userlist->name); > > + err_code |= ERR_ALERT; > > goto out; > > } > > > > - newuser->user = strdup(args[1]); > > + newuser = calloc(1, sizeof(*newuser)); > > + if (!newuser) { > > + ha_alert("parsing [%s:%d]: out of memory.\n", file, > linenum); > > + err_code |= ERR_ALERT | ERR_ABORT; > > + goto out; > > + } > > + > > + newuser->user = strdup(args[1]); > > > > - newuser->next = userlist->users; > > - userlist->users = newuser; > > + newuser->next = userlist->users; > > + userlist->users = newuser; > > > > - cur_arg = 2; > > + cur_arg = 2; > > > > - while (*args[cur_arg]) { > > - if (strcmp(args[cur_arg], "password") == 0) { > > + while (*args[cur_arg]) { > > + if (strcmp(args[cur_arg], "password") == 0) { > > #ifdef USE_LIBCRYPT > > - if (!crypt("", args[cur_arg + 1])) { > > - ha_alert("parsing [%s:%d]: the > encrypted password used for user '%s' is not supported by crypt(3).\n", > > - file, linenum, > newuser->user); > > - err_code |= ERR_ALERT | ERR_FATAL; > > - goto out; > > - } > > -#else > > - ha_warning("parsing [%s:%d]: no crypt(3) > support compiled, encrypted passwords will not work.\n", > > - file, linenum); > > - err_code |= ERR_ALERT; > > -#endif > > - newuser->pass = strdup(args[cur_arg + 1]); > > - cur_arg += 2; > > - continue; > > - } else if (strcmp(args[cur_arg], > "insecure-password") == 0) { > > - newuser->pass = strdup(args[cur_arg + 1]); > > - newuser->flags |= AU_O_INSECURE; > > - cur_arg += 2; > > - continue; > > - } else if (strcmp(args[cur_arg], "groups") == 0) { > > - newuser->u.groups_names = > strdup(args[cur_arg + 1]); > > - cur_arg += 2; > > - continue; > > - } else { > > - ha_alert("parsing [%s:%d]: '%s' only > supports 'password', 'insecure-password' and 'groups' options.\n", > > - file, linenum, args[0]); > > + if (!crypt("", args[cur_arg + 1])) { > > + ha_alert("parsing [%s:%d]: the encrypted > password used for user '%s' is not supported by crypt(3).\n", > > + file, linenum, newuser->user); > > err_code |= ERR_ALERT | ERR_FATAL; > > goto out; > > } > > +#else > > + ha_warning("parsing [%s:%d]: no crypt(3) support > compiled, encrypted passwords will not work.\n", > > + file, linenum); > > + err_code |= ERR_ALERT; > > +#endif > > + newuser->pass = strdup(args[cur_arg + 1]); > > + cur_arg += 2; > > + continue; > > + } else if (strcmp(args[cur_arg], "insecure-password") == > 0) { > > + newuser->pass = strdup(args[cur_arg + 1]); > > + newuser->flags |= AU_O_INSECURE; > > + cur_arg += 2; > > + continue; > > + } else if (strcmp(args[cur_arg], "groups") == 0) { > > + newuser->u.groups_names = strdup(args[cur_arg + > 1]); > > + cur_arg += 2; > > + continue; > > + } else { > > + ha_alert("parsing [%s:%d]: '%s' only supports > 'password', 'insecure-password' and 'groups' options.\n", > > + file, linenum, args[0]); > > + err_code |= ERR_ALERT | ERR_FATAL; > > + goto out; > > } > > - } else { > > - ha_alert("parsing [%s:%d]: unknown keyword '%s' in '%s' > section\n", file, linenum, args[0], "users"); > > - err_code |= ERR_ALERT | ERR_FATAL; > > } > > > > out: > > @@ -4921,6 +4952,8 @@ REGISTER_CONFIG_SECTION("traces", > cfg_parse_traces, NULL); > > > > static struct cfg_kw_list cfg_kws = {{ },{ > > { CFG_GLOBAL, "default-path", cfg_parse_global_def_path }, > > + { CFG_USERLIST, "group", cfg_parse_users_group }, > > + { CFG_USERLIST, "user", cfg_parse_users_user }, > > { /* END */ } > > }}; > > > > -- > > 2.48.1 > > > > > > > > -- > William Lallemand > >
