On Mon, Aug 28, 2017 at 05:27:03PM +0200, Eric Faurot wrote:
> Hi,
>
> The current static (file) table parser is a bit clumsy. It tries to
> determine the type (mapping or list entry) of each line independently
> by splitting on allowed separators (" \t:") without using any context,
> then fails if the type is not what's expected. It's impossible to define
> a list of ipv6 addresses for instance, since ':' is a valid separator.
>
> This diff changes the parser logic. If the table is untyped, determine
> its type by examining the first entry: if it contains a separator, type
> is "mapping", otherwise type is "list". All entries are then parsed
> according to the table type. The "list" type can also be forced by using
> the "@list" directive in a comment. This allows to define list of entries
> containing a separator.
>
> Existing table files should still be working as expected.
> As a bonus, parse errors are now logged with line number.
>
as discussed, i think it's a neat idea
the diff is ok gilles@ too
> Index: table_static.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/table_static.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 table_static.c
> --- table_static.c 14 Aug 2017 08:01:14 -0000 1.16
> +++ table_static.c 16 Aug 2017 15:27:22 -0000
> @@ -73,7 +73,8 @@ static int
> table_static_config(struct table *t)
> {
> FILE *fp;
> - char *buf = NULL;
> + char *buf = NULL, *p;
> + int lineno = 0;
> size_t sz = 0;
> ssize_t flen;
> char *keyp;
> @@ -90,14 +91,47 @@ table_static_config(struct table *t)
> }
>
> while ((flen = getline(&buf, &sz, fp)) != -1) {
> + lineno++;
> if (buf[flen - 1] == '\n')
> - buf[flen - 1] = '\0';
> + buf[--flen] = '\0';
>
> keyp = buf;
> - while (isspace((unsigned char)*keyp))
> + while (isspace((unsigned char)*keyp)) {
> ++keyp;
> - if (*keyp == '\0' || *keyp == '#')
> + --flen;
> + }
> + if (*keyp == '\0')
> + continue;
> + while (isspace((unsigned char)keyp[flen - 1]))
> + keyp[--flen] = '\0';
> + if (*keyp == '#') {
> + if (t->t_type == T_NONE) {
> + keyp++;
> + while (isspace((unsigned char)*keyp))
> + ++keyp;
> + if (!strcmp(keyp, "@list"))
> + t->t_type = T_LIST;
> + }
> continue;
> + }
> +
> + if (t->t_type == T_NONE) {
> + for (p = keyp; *p; p++) {
> + if (*p == ' ' || *p == '\t' || *p == ':') {
> + t->t_type = T_HASH;
> + break;
> + }
> + }
> + if (t->t_type == T_NONE)
> + t->t_type = T_LIST;
> + }
> +
> + if (t->t_type == T_LIST) {
> + table_add(t, keyp, NULL);
> + continue;
> + }
> +
> + /* T_HASH */
> valp = keyp;
> strsep(&valp, " \t:");
> if (valp) {
> @@ -111,18 +145,20 @@ table_static_config(struct table *t)
> if (*valp == '\0')
> valp = NULL;
> }
> + if (valp == NULL) {
> + log_warnx("%s: invalid map entry line %d", t->t_config,
> + lineno);
> + goto end;
> + }
>
> - if (t->t_type == 0)
> - t->t_type = (valp == keyp || valp == NULL) ? T_LIST :
> - T_HASH;
> + table_add(t, keyp, valp);
> + }
>
> - if ((valp == keyp || valp == NULL) && t->t_type == T_LIST)
> - table_add(t, keyp, NULL);
> - else if ((valp != keyp && valp != NULL) && t->t_type == T_HASH)
> - table_add(t, keyp, valp);
> - else
> - goto end;
> + if (ferror(fp)) {
> + log_warn("%s: getline", t->t_config);
> + goto end;
> }
> +
> /* Accept empty alias files; treat them as hashes */
> if (t->t_type == T_NONE && t->t_backend->services & K_ALIAS)
> t->t_type = T_HASH;
>
--
Gilles Chehade
https://www.poolp.org @poolpOrg
