Henning Brauer([email protected]) on 2016.06.21 13:11:16 +0200:
> * Stefan Sperling <[email protected]> [2016-06-21 11:15]:
> > Generally, I would appreciate more detailed error messages from the pf.conf
> > parser. I recall several occasions where pfctl threw "syntax error" and more
> > specific error reporting would have saved me some time with finding the
> > silly mistake I made. And in this case the ruleset loads successfully even
> > though, while parsing, we already know it's not going to work as intended...
>
> true, that's shared by all yacc-style parsers, if the grammar doesn't
> match you just get syntax error without much of a hint what's wrong.
the quality of the error message depends both on the language (some make
it harder to error out early) and on the parser.
fail early is better.
> however, the ruleset in this case does NOT load.
>
> <brahe@quigon> $ echo '"a macro with spaces"="foo"\npass from $a\ macro\
> with\ spaces"' | pfctl -nvf -
> a macro with spaces = "foo"
> stdin:2: macro 'a' not defined
> stdin:2: syntax error
>
sure, thats what the op reported
>
> > Only as long as it doesn't make the parser code overly complex, of course.
> > But currently the balance is tilted too much towards terse error messages
> > for my taste. So I liked benno's first diff.
>
> it's just a tiny check indeed, which swings the "cost" (not in
> financial terms) vs benefit pendulum towards the benefit side, yes.
thx.
here is a diff for all daemons.
ok for this?
diff --git sbin/iked/parse.y sbin/iked/parse.y
index 958e51a..6455a00 100644
--- sbin/iked/parse.y
+++ sbin/iked/parse.y
@@ -73,6 +73,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -1006,6 +1007,10 @@ string : string STRING
varset : STRING '=' string
{
log_debug("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
err(1, "cannot store variable");
free($1);
@@ -1234,6 +1239,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
unsigned char buf[8096];
diff --git sbin/ipsecctl/parse.y sbin/ipsecctl/parse.y
index fe9cee0..67fbd72 100644
--- sbin/ipsecctl/parse.y
+++ sbin/ipsecctl/parse.y
@@ -71,6 +71,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -883,6 +884,10 @@ varset : STRING '=' string
{
if (ipsec->opts & IPSECCTL_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
err(1, "cannot store variable");
free($1);
@@ -1092,6 +1097,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index 934438c..9846063 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -85,6 +85,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -714,6 +715,10 @@ numberstring : NUMBER
{
varset : STRING '=' varstring {
if (pf->opts & PF_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
err(1, "cannot store variable %s", $1);
free($1);
@@ -5172,6 +5177,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
index e7fd5e1..93c847c 100644
--- usr.sbin/bgpd/parse.y
+++ usr.sbin/bgpd/parse.y
@@ -64,6 +64,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -339,6 +340,10 @@ yesno : STRING {
varset : STRING '=' string {
if (cmd_opts & BGPD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -2406,6 +2411,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/dvmrpd/parse.y usr.sbin/dvmrpd/parse.y
index 66b7b73..66aa47f 100644
--- usr.sbin/dvmrpd/parse.y
+++ usr.sbin/dvmrpd/parse.y
@@ -66,6 +66,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -165,6 +166,10 @@ yesno : STRING {
varset : STRING '=' string {
if (conf->opts & DVMRPD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -504,6 +509,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/eigrpd/parse.y usr.sbin/eigrpd/parse.y
index 02e010f..09067fb 100644
--- usr.sbin/eigrpd/parse.y
+++ usr.sbin/eigrpd/parse.y
@@ -65,6 +65,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -199,6 +200,10 @@ eigrp_af : IPV4 { $$ = AF_INET; }
varset : STRING '=' string {
if (global.cmd_opts & EIGRPD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -731,6 +736,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
unsigned char buf[8096];
diff --git usr.sbin/hostapd/parse.y usr.sbin/hostapd/parse.y
index c6a8beb..56c6bda 100644
--- usr.sbin/hostapd/parse.y
+++ usr.sbin/hostapd/parse.y
@@ -78,6 +78,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -971,6 +972,10 @@ string : string STRING
varset : STRING '=' string
{
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
hostapd_fatal("cannot store variable");
free($1);
@@ -1420,6 +1425,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/httpd/parse.y usr.sbin/httpd/parse.y
index 080f319..bce7741 100644
--- usr.sbin/httpd/parse.y
+++ usr.sbin/httpd/parse.y
@@ -75,6 +75,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -171,6 +172,10 @@ include : INCLUDE STRING {
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -1305,6 +1310,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
unsigned char buf[8096];
diff --git usr.sbin/ifstated/parse.y usr.sbin/ifstated/parse.y
index 5679164..a669149 100644
--- usr.sbin/ifstated/parse.y
+++ usr.sbin/ifstated/parse.y
@@ -63,6 +63,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -144,6 +145,10 @@ string : string STRING
{
varset : STRING '=' string {
if (conf->opts & IFSD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1) {
free($1);
free($3);
@@ -502,6 +507,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/iscsictl/parse.y usr.sbin/iscsictl/parse.y
index b39b2e1..789f6ad 100644
--- usr.sbin/iscsictl/parse.y
+++ usr.sbin/iscsictl/parse.y
@@ -65,6 +65,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
void clear_config(struct iscsi_config *);
@@ -155,6 +156,10 @@ string : string STRING {
;
varset : STRING '=' string {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
err(1, "cannot store variable");
free($1);
@@ -478,6 +483,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/ldapd/parse.y usr.sbin/ldapd/parse.y
index aec1d12..eda1c1b 100644
--- usr.sbin/ldapd/parse.y
+++ usr.sbin/ldapd/parse.y
@@ -67,6 +67,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
struct listener *host_unix(const char *path);
struct listener *host_v4(const char *, in_port_t);
@@ -359,6 +360,10 @@ include : INCLUDE STRING {
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -556,6 +561,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[4096];
diff --git usr.sbin/ldpd/parse.y usr.sbin/ldpd/parse.y
index d332b9e..d94640b 100644
--- usr.sbin/ldpd/parse.y
+++ usr.sbin/ldpd/parse.y
@@ -86,6 +86,7 @@ static int lookup(char *);
static int lgetc(int);
static int lungetc(int);
static int findeol(void);
+static int has_whitespace(const char *);
static int yylex(void);
static int check_file_secrecy(int, const char *);
static struct file *pushfile(const char *, int);
@@ -210,6 +211,10 @@ pw_type : ETHERNET { $$ =
PW_TYPE_ETHERNET; }
varset : STRING '=' string {
if (global.cmd_opts & LDPD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -927,6 +932,16 @@ findeol(void)
}
static int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+static int
yylex(void)
{
unsigned char buf[8096];
diff --git usr.sbin/ospf6d/parse.y usr.sbin/ospf6d/parse.y
index 70e7d3e..7a1aeed 100644
--- usr.sbin/ospf6d/parse.y
+++ usr.sbin/ospf6d/parse.y
@@ -66,6 +66,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -181,6 +182,10 @@ no : /* empty */ { $$ = 0; }
varset : STRING '=' string {
if (conf->opts & OSPFD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -676,6 +681,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/ospfd/parse.y usr.sbin/ospfd/parse.y
index 9b886ad..3c0c8a1 100644
--- usr.sbin/ospfd/parse.y
+++ usr.sbin/ospfd/parse.y
@@ -64,6 +64,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -197,6 +198,10 @@ msec : MSEC NUMBER {
varset : STRING '=' string {
if (conf->opts & OSPFD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -862,6 +867,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
index a6bf390..dbd5cf2 100644
--- usr.sbin/relayd/parse.y
+++ usr.sbin/relayd/parse.y
@@ -77,6 +77,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -349,6 +350,10 @@ port : PORT STRING {
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -2367,6 +2372,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/ripd/parse.y usr.sbin/ripd/parse.y
index ddaca13..b4e4ec9 100644
--- usr.sbin/ripd/parse.y
+++ usr.sbin/ripd/parse.y
@@ -64,6 +64,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -149,6 +150,10 @@ no : /* empty */ { $$ = 0; }
varset : STRING '=' string {
if (conf->opts & RIPD_OPT_VERBOSE)
printf("%s = \"%s\"\n", $1, $3);
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -529,6 +534,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/smtpd/parse.y usr.sbin/smtpd/parse.y
index ab02719..8603a18 100644
--- usr.sbin/smtpd/parse.y
+++ usr.sbin/smtpd/parse.y
@@ -76,6 +76,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
int yyerror(const char *, ...)
__attribute__((__format__ (printf, 1, 2)))
__attribute__((__nonnull__ (1)));
@@ -213,6 +214,10 @@ include : INCLUDE STRING {
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -1630,6 +1635,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
unsigned char buf[8096];
diff --git usr.sbin/snmpd/parse.y usr.sbin/snmpd/parse.y
index 5e288ee..8e953a0 100644
--- usr.sbin/snmpd/parse.y
+++ usr.sbin/snmpd/parse.y
@@ -77,6 +77,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -165,6 +166,10 @@ include : INCLUDE STRING {
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -746,6 +751,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index 23cea73..21fa932 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -63,6 +63,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -129,6 +130,10 @@ include : INCLUDE string {
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatalx("cannot store variable");
free($1);
@@ -423,6 +428,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
diff --git usr.sbin/ypldap/parse.y usr.sbin/ypldap/parse.y
index 39be696..8652441 100644
--- usr.sbin/ypldap/parse.y
+++ usr.sbin/ypldap/parse.y
@@ -72,6 +72,7 @@ int lookup(char *);
int lgetc(int);
int lungetc(int);
int findeol(void);
+int has_whitespace(const char *);
TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
struct sym {
@@ -142,6 +143,10 @@ include : INCLUDE STRING
{
;
varset : STRING '=' STRING {
+ if (has_whitespace($1)) {
+ yyerror("macro name cannot contain whitespace");
+ YYERROR;
+ }
if (symset($1, $3, 0) == -1)
fatal("cannot store variable");
free($1);
@@ -497,6 +502,16 @@ findeol(void)
}
int
+has_whitespace(const char *s) {
+ while (*s != '\0') {
+ if (isspace((unsigned char)*s))
+ return 1;
+ s++;
+ }
+ return 0;
+}
+
+int
yylex(void)
{
u_char buf[8096];
