Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
smtpd: unify the table(5) parser with makemap
To:
tech@openbsd.org
Date:
Tue, 09 Jan 2024 17:33:49 +0100

Download raw body.

Thread
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?

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  = "<set>";
 	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);
+}