[moving to tech@, +CC Gilles and Eric]
On 2023/11/13 14:04:37 -0300, Crystal Kolipe <kolip...@exoticsilicon.com> wrote:
> The smtpd.conf manual says:
table(5) actually ;-)
> > When using a `file' table, a list will be written with each value on a
> > line by itself. Comments can be put anywhere in the file using a hash
> > mark (`#'), and extend to the end of the current line.
>
> Unfortunately this is not true :-).
>
> Worse still, it causes silent configuration breakage in some cases, for
> example consider if you have a list of IPs that you want to block stored in a
> table:
>
> $ cat /etc/mail/blocked_ips
> 10.0.0.1
> 10.0.0.50
> 10.1.2.3 # Sends loads of rubbish since last month
> 10.4.5.6
>
> This would be loaded in smtpd.conf via something like:
>
> table ip_reject_list file:/etc/mail/blocked_ips
>
> No error would be reported, and smtpd would start just fine.
>
> But the IP address with the comment would _not_ be included in the list!
My guess is that the intention was to allow only comments at the start
of the line, and the text in table(5) was just a side-effect of
copy-pasting.
In fact, I don't think it should support comments "anywhere in the
file", as they can have unwanted side effects. With your diff, what
would happen if I have a '#' character in one of my passwords?
Instead of adding means of escaping (and escaping the escaping), that
would equally breaking existing confuration, I think we should just
'fix' the manpage. Your use-case would still work, it would only need
to be changed to
$ cat /etc/mail/blocked_ips
10.0.0.1
10.0.0.50
# Sends loads of rubbish since last month
10.1.2.3
or something like that.
While here, I noticed that the parser accepts the magic "@list" comment
to force the file to be considered a list table instead of a mapping.
It was introduced by eric@ in rev. 1.17 back in 2017 but never
documented. Should we document it too?
diff /usr/src
commit - 5310d41c134580317d44a9b61beebad6d69454a4
path + /usr/src
blob - 6da7bd71893c83194159d47614b5bfb0d2f0e138
file + usr.sbin/smtpd/table.5
--- usr.sbin/smtpd/table.5
+++ usr.sbin/smtpd/table.5
@@ -42,9 +42,10 @@ table mymapping { key1 = value1, key2 = value2, key3 =
When using a
.Ql file
table, a list will be written with each value on a line by itself.
-Comments can be put anywhere in the file using a hash mark
+Comments begin with a hash mark
.Pq Sq # ,
and extend to the end of the current line.
+They can be only used at the beginning of a line.
.Bd -literal -offset indent
value1
value2
> The following patch fixes the problem:
>
> --- table_static.c.dist Mon Jun 14 14:58:16 2021
> +++ table_static.c Mon Nov 13 08:28:46 2023
> @@ -118,6 +118,7 @@
> char *keyp;
> char *valp;
> int ret = 0;
> + int i;
>
> if ((fp = fopen(path, "r")) == NULL) {
> log_warn("%s: fopen", path);
> @@ -136,6 +137,14 @@
> }
> if (*keyp == '\0')
> continue;
> +
> + for (i=0; i<flen; i++) {
> + if (keyp[i]=='#') {
> + flen = i;
> + break ;
> + }
> + }
> +
> while (isspace((unsigned char)keyp[flen - 1]))
> keyp[--flen] = '\0';
> if (*keyp == '#') {
