pfctl: Allow a semicolon (;) as a comment

[otis]

Summary

To make parsing of, for example, Spamhaus' drop.txt and similar
files that contains semicolons as comments, allow them also
in file-based tables.

[kp]

I'm not opposed to this, but I believe it's incomplete. This will only accept ';' as a comment in table files. It won't work for lines in the main pf.conf.

I've not tested it, but I think you also need this:

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index d07a3fdc188e..e652c27f1acf 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -6530,7 +6530,7 @@ top:
                ; /* nothing */

        yylval.lineno = file->lineno;
-       if (c == '#')
+       if (c == '#' || c == ';')
                while ((c = lgetc(0)) != '\n' && c != EOF)
                        ; /* nothing */
        if (c == '$' && parsebuf == NULL) {

Can you add a test case or two in /usr/src/sbin/pfctl/tests to validate that this does the right thing?

[otis]

In D46088#1050559, @kp wrote:

I'm not opposed to this, but I believe it's incomplete. This will only accept ';' as a comment in table files. It won't work for lines in the main pf.conf.

I've considered also this possibility (and looked into parser.y) but decided to not do it, eventually. But I might do it as well.

I've not tested it, but I think you also need this:

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index d07a3fdc188e..e652c27f1acf 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -6530,7 +6530,7 @@ top:
                ; /* nothing */

        yylval.lineno = file->lineno;
-       if (c == '#')
+       if (c == '#' || c == ';')
                while ((c = lgetc(0)) != '\n' && c != EOF)
                        ; /* nothing */
        if (c == '$' && parsebuf == NULL) {

Can you add a test case or two in /usr/src/sbin/pfctl/tests to validate that this does the right thing?

Yes, I was already looking into the test suite and will surely add those tests.

[kp]

In D46088#1050560, @otis wrote:

In D46088#1050559, @kp wrote:

I'm not opposed to this, but I believe it's incomplete. This will only accept ';' as a comment in table files. It won't work for lines in the main pf.conf.

I've considered also this possibility (and looked into parser.y) but decided to not do it, eventually. But I might do it as well.

I had assumed that the man page change you made was about the general case, but it appears to be only about address files, so we could actually do this without misleading users. I think the ideal would be to do both, but if there's a test case for this we can do this now and consider extending it to the general case later.

[otis]

Add tests.

[kp]

Thanks for the patch!