From: Omar Polo Subject: Re: smtpd: unify the table(5) parser with makemap To: Omar Polo Cc: tech@openbsd.org Date: Mon, 05 Feb 2024 00:02:29 +0100 On 2024/01/28 14:04:35 +0100, Omar Polo wrote: > On 2024/01/21 13:59:16 +0100, Omar Polo wrote: > > On 2024/01/09 17:33:49 +0100, Omar Polo wrote: > > > I'd like to unify/converge the two different parsers for the table(5) > > > file format used in smtpd. > > > > > > The smtpd built-in one, used for file table(5)s, is quite simple and > > > straightforward. makemap(8) uses another one built on top of fparseln() > > > and so supports comments in arbitrary positions, escapes with \, and > > > breaking lines over multiple physical lines. > > > > > > table(5) documents that > > > > > > A file table can be converted to a Berkeley database using the > > > makemap(8) utility with no syntax change. > > > > > > which holds true only when there # or \ are present in the file, as the > > > two parser will disagree on the meaning. > > > > > > I think it's valuable to maintain a feature-parity between file and db > > > tables, and to not require syntax changes when converting from one to > > > the other. Unifying the parsers however means a breaking change. > > > > > > I prefer the smtpd built-in one: it's simple and doesn't have escaping > > > or other fancy stuff that it's useless in a table file. It just trims > > > whitespaces and optionally splits the line in a key-value pair. > > > > > > Thoughs/oks? > > > > ping > > anyone? This is mostly moving code around so that we end up with only > one parser for this file-format. > > it's also required to fix ipv6 addresses parsing in file/db tables which > is currently broken (table(5) lies), which i intend to do in a > follow-up. diff /home/op/tmp/smtpd commit - 1f95d9310cff81a3ddd7b95069ea5dea8d1b03de path + /home/op/tmp/smtpd blob - 2144e83959e9c25b24fc0913b0907a5af86a3804 file + makemap.c --- makemap.c +++ makemap.c @@ -37,9 +37,8 @@ static void usage(void); static int parse_map(DB *, int *, char *); -static int parse_entry(DB *, int *, char *, size_t, size_t); -static int parse_mapentry(DB *, int *, char *, size_t, size_t); -static int parse_setentry(DB *, int *, char *, size_t, size_t); +static int add_mapentry(DB *, int *, char *, char *, size_t); +static int add_setentry(DB *, int *, char *, size_t); static int make_plain(DBT *, char *); static int make_aliases(DBT *, char *); static char *conf_aliases(char *); @@ -247,9 +246,10 @@ static int parse_map(DB *db, int *dbputs, char *filename) { FILE *fp; - char *line; - size_t len; + char *key, *val, *line = NULL; + size_t linesize; size_t lineno = 0; + int malformed, table_type, r; if (strcmp(filename, "-") == 0) fp = fdopen(0, "r"); @@ -269,56 +269,47 @@ parse_map(DB *db, int *dbputs, char *filename) return 0; } - while ((line = fparseln(fp, &len, &lineno, - NULL, FPARSELN_UNESCCOMM)) != NULL) { - if (!parse_entry(db, dbputs, line, len, lineno)) { + table_type = (type == T_SET) ? T_LIST : T_HASH; + while (parse_table_line(fp, &line, &linesize, &table_type, + &key, &val, &malformed) != -1) { + lineno++; + if (malformed) { + warnx("%s:%zd: invalid entry", source, lineno); free(line); fclose(fp); return 0; } - free(line); + if (key == NULL) + continue; + + switch (type) { + case T_PLAIN: + case T_ALIASES: + r = add_mapentry(db, dbputs, key, val, lineno); + break; + case T_SET: + r = add_setentry(db, dbputs, key, lineno); + break; + } + + if (!r) { + free(line); + fclose(fp); + return 0; + } } + free(line); fclose(fp); return 1; } static int -parse_entry(DB *db, int *dbputs, char *line, size_t len, size_t lineno) +add_mapentry(DB *db, int *dbputs, char *keyp, char *valp, size_t lineno) { - switch (type) { - case T_PLAIN: - case T_ALIASES: - return parse_mapentry(db, dbputs, line, len, lineno); - case T_SET: - return parse_setentry(db, dbputs, line, len, lineno); - } - return 0; -} - -static int -parse_mapentry(DB *db, int *dbputs, char *line, size_t len, size_t lineno) -{ DBT key; DBT val; - char *keyp; - char *valp; - keyp = line; - while (isspace((unsigned char)*keyp)) - keyp++; - if (*keyp == '\0') - return 1; - - valp = keyp; - strsep(&valp, " \t:"); - if (valp == NULL || valp == keyp) - goto bad; - while (*valp == ':' || isspace((unsigned char)*valp)) - valp++; - if (*valp == '\0') - goto bad; - /* Check for dups. */ key.data = keyp; key.size = strlen(keyp) + 1; @@ -355,18 +346,11 @@ bad: } static int -parse_setentry(DB *db, int *dbputs, char *line, size_t len, size_t lineno) +add_setentry(DB *db, int *dbputs, char *keyp, size_t lineno) { DBT key; DBT val; - char *keyp; - keyp = line; - while (isspace((unsigned char)*keyp)) - keyp++; - if (*keyp == '\0') - return 1; - val.data = ""; val.size = strlen(val.data) + 1; blob - 25090c3acf9980c0fbe22784a5242dae64c8ec53 file + smtpd.h --- smtpd.h +++ smtpd.h @@ -1738,6 +1738,9 @@ void log_trace0(const char *, ...) __attribute__((format (printf, 1, 2))); #define log_trace(m, ...) do { if (tracing & (m)) log_trace0(__VA_ARGS__); } while (0) +int parse_table_line(FILE *, char **, size_t *, int *, + char **, char **, int *); + /* waitq.c */ int waitq_wait(void *, void (*)(void *, void *, void *), void *); void waitq_run(void *, void *); blob - af57038f2313f787ad14ab9156a97a2487df87b4 file + table_static.c --- table_static.c +++ table_static.c @@ -111,80 +111,28 @@ static int table_static_priv_load(struct table_static_priv *priv, const char *path) { FILE *fp; - char *buf = NULL, *p; + char *line = NULL; int lineno = 0; - size_t sz = 0; - ssize_t flen; + size_t linesize = 0; char *keyp; char *valp; - int ret = 0; + int malformed, ret = 0; if ((fp = fopen(path, "r")) == NULL) { log_warn("%s: fopen", path); return 0; } - while ((flen = getline(&buf, &sz, fp)) != -1) { + while (parse_table_line(fp, &line, &linesize, &priv->type, + &keyp, &valp, &malformed) != -1) { lineno++; - if (buf[flen - 1] == '\n') - buf[--flen] = '\0'; - - keyp = buf; - while (isspace((unsigned char)*keyp)) { - ++keyp; - --flen; - } - if (*keyp == '\0') - continue; - while (isspace((unsigned char)keyp[flen - 1])) - keyp[--flen] = '\0'; - if (*keyp == '#') { - if (priv->type == T_NONE) { - keyp++; - while (isspace((unsigned char)*keyp)) - ++keyp; - if (!strcmp(keyp, "@list")) - priv->type = T_LIST; - } - continue; - } - - if (priv->type == T_NONE) { - for (p = keyp; *p; p++) { - if (*p == ' ' || *p == '\t' || *p == ':') { - priv->type = T_HASH; - break; - } - } - if (priv->type == T_NONE) - priv->type = T_LIST; - } - - if (priv->type == T_LIST) { - table_static_priv_add(priv, keyp, NULL); - continue; - } - - /* T_HASH */ - valp = keyp; - strsep(&valp, " \t:"); - if (valp) { - while (*valp) { - if (!isspace((unsigned char)*valp) && - !(*valp == ':' && - isspace((unsigned char)*(valp + 1)))) - break; - ++valp; - } - if (*valp == '\0') - valp = NULL; - } - if (valp == NULL) { - log_warnx("%s: invalid map entry line %d", + if (malformed) { + log_warnx("%s:%d invalid map entry", path, lineno); goto end; } - + if (keyp == NULL) + continue; table_static_priv_add(priv, keyp, valp); } @@ -199,7 +147,7 @@ table_static_priv_load(struct table_static_priv *priv, ret = 1; end: - free(buf); + free(line); fclose(fp); return ret; } blob - feb663cc61ec2572b57741fd694926f0c26967c7 file + util.c --- util.c +++ util.c @@ -826,3 +826,75 @@ log_trace_verbose(int v) /* Set debug logging in log.c */ log_setverbose(v & TRACE_DEBUG ? 2 : foreground_log); } + +int +parse_table_line(FILE *fp, char **line, size_t *linesize, + int *type, char **key, char **val, int *malformed) +{ + char *keyp, *valp, *p; + ssize_t linelen; + + *key = NULL; + *val = NULL; + *malformed = 0; + + if ((linelen = getline(line, linesize, fp)) == -1) + return (-1); + + keyp = *line; + while (isspace((unsigned char)*keyp)) { + ++keyp; + --linelen; + } + if (*keyp == '\0') + return 0; + while (isspace((unsigned char)keyp[linelen - 1])) + keyp[--linelen] = '\0'; + if (*keyp == '#') { + if (*type == T_NONE) { + keyp++; + while (isspace((unsigned char)*keyp)) + ++keyp; + if (!strcmp(keyp, "@list")) + *type = T_LIST; + } + return 0; + } + + if (*type == T_NONE) { + for (p = keyp; *p; p++) { + if (*p == ' ' || *p == '\t' || *p == ':') { + *type = T_HASH; + break; + } + } + if (*type == T_NONE) + *type = T_LIST; + } + + if (*type == T_LIST) { + *key = keyp; + return (0); + } + + /* T_HASH */ + valp = keyp; + strsep(&valp, " \t:"); + if (valp) { + while (*valp) { + if (!isspace((unsigned char)*valp) && + !(*valp == ':' && + isspace((unsigned char)*(valp + 1)))) + break; + ++valp; + } + if (*valp == '\0') + valp = NULL; + } + if (valp == NULL) + *malformed = 1; + + *key = keyp; + *val = valp; + return (0); +}