From: Omar Polo <op@omarpolo.com>
Subject: Re: smtpd: allow braces for `listen' options
To: "Kirill A. Korinsky" <kirill@korins.ky>
Cc: tech@openbsd.org
Date: Fri, 18 Oct 2024 00:50:45 +0200

On 2024/10/14 01:18:51 +0200, Kirill A. Korinsky <kirill@korins.ky> wrote:
> On Mon, 14 Oct 2024 00:54:15 +0200,
> Omar Polo <op@omarpolo.com> wrote:
> > 
> > some time ago I was playing with the idea of turning the prox-exec to an
> > argv-style list of string, which would also avoid having to use system()
> > and could be fed directly to exec*(), but I haven't wrote any diff in
> > the end.
> > 
> > Anyway, I think that also
> > 
> > 	filter dnsbl proc-exec {
> > 		filter-dnsbl -m domain1 domain2
> > 			domain3 domain4
> > 	}
> > 
> > would be way nicer.  I can cook an initial diff to turn the words inside
> > {...} into a string, so no larger changes for now, in the next days.
> > 
> 
> what is amazing and I volunteer to at least test it!

it seemed easier to say than to actually do it.  I'm attaching a trivial
diff for it, but there are some issues.  Your example has to be written
as

filter dnsbl proc-exec {
	filter-dnsbl '-m' domain1 domain2
		domain3 domain4
}

because otherwise a bare -m will lead to a syntax error.  Same story for
the usage of keywords inside the braces.

We could add a knob to the lexer so that when we enter the braces we
turn off many things, but I'm not sure if it would be accepted.  (and
before showing that diff, i want to check whether we can really assume a
one token lookahead or if implementations are allowed to do fancier
things.)

(I still like to wonder about using something like this because then we
could switch from calling system() to exec*(), which has less surprises
wrt shell expanding stuff.  It could also apply to the mda command
string.)

> > P.S.: I was mostly testing the waters here, if this looks fine to y'all
> > I'd like to also introduce {...} for `action' as well in a similar
> > manner.
> 
> and after that on the table left "match", isn't it?

Yeah.  These are the places where we can accumulate some important
number of flags, and allowing to group them into braces is a nice
usability improvement.

diff /usr/src
commit - 7e955a16785914c7c42222b2cd5d61af7a395f99
path + /usr/src
blob - d975100507999952f9d8c5404608697687520278
file + usr.sbin/smtpd/parse.y
--- usr.sbin/smtpd/parse.y
+++ usr.sbin/smtpd/parse.y
@@ -191,6 +191,7 @@ typedef struct {
 %token	<v.string>	STRING
 %token  <v.number>	NUMBER
 %type	<v.table>	table
+%type	<v.string>	numberstr cmdline cmdline_l
 %type	<v.number>	size negation
 %type	<v.table>	tables tablenew tableref
 %%
@@ -295,6 +296,29 @@ tableval_list	: string_list			{ }
 		| keyval_list			{ }
 		;
 
+numberstr:
+STRING
+| NUMBER {
+	if (asprintf(&$$, "%lld", (long long)$1) == -1)
+		fatalx("asprintf");
+}
+;
+
+cmdline_l:
+numberstr optnl { $$ = $1; }
+| cmdline_l numberstr optnl {
+	if (asprintf(&$$, "%s %s", $1, $2) == -1)
+		fatalx("asprint");
+	free($1);
+	free($2);
+}
+;
+
+cmdline:
+STRING
+| '{' optnl cmdline_l '}' { $$ = $3; }
+;
+
 bounce:
 BOUNCE WARN_INTERVAL {
 	memset(conf->sc_bounce_warn, 0, sizeof conf->sc_bounce_warn);
@@ -1911,7 +1935,7 @@ FILTER STRING PROC STRING {
 	filter_config = NULL;
 }
 |
-FILTER STRING PROC_EXEC STRING {
+FILTER STRING PROC_EXEC cmdline {
 	if (dict_get(conf->sc_filters_dict, $2)) {
 		yyerror("filter already exists with that name: %s", $2);
 		free($2);