[PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Florian Westphal]

From: Jan Kończak <jan.konczak@cs.put.poznan.pl>

Now, on syntax erros, e.g., 'nft create fable filter', the user sees:
   Error: syntax error, unexpected string
   create fable filter
          ^^^^^
The patch builds an error message that lists what the parser expects
to see, in that case it would print:
   Error: syntax error, unexpected string
   expected any of: synproxy, table, chain, set, element, map,
   flowtable, ct, counter, limit, quota, secmark
   create fable filter
          ^^^^^
The obvious purpose of this is to help people who learn nft syntax.
The messages are still not as explanatory as one wishes, for it may
list parser token names such as 'string', but it's still better
than no hints at all.

Heed that the list of possible items on the parser's side is not
always consistent with expectations.
For instance, lexer/parser recognizes 'l4proto' in this command:
   nft add rule ip F I meta l4proto tcp
as a generic '%token <string> STRING', while 'iifname' in
   nft add rule ip F I meta iifname eth0
is recognized as a '%token IIFNAME'
In such case the parser is only able to say that right after 'meta'
it expects 'iifname' or 'string', rather than 'iifname' and 'l4proto'.

(Notice that the help which is already present in nft is also off,
e.g., 'nft add rule ip F I meta bogus bogus' constructs in meta.c
a list of all possible keywords that does not list all possible
keywords, for 'ibriport' gets recognized, but is not listed there.)

Signed-off-by: Jan Kończak <jan.konczak@cs.put.poznan.pl>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: prefer stdio (fprintf+memopen) vs. manual realloc of a cstring
 buffer, align more with nftables coding style.

 I'll apply this unless there are any objections.

 src/parser_bison.y | 59 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 3ceef79469d7..05f64e6c6cbb 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -221,7 +221,8 @@ int nft_lex(void *, void *, void *);
 %parse-param		{ void *scanner }
 %parse-param		{ struct parser_state *state }
 %lex-param		{ scanner }
-%define parse.error verbose
+%define parse.error custom
+%define parse.lac full
 %locations
 
 %initial-action {
@@ -6603,3 +6604,59 @@ exthdr_key		:	HBH	close_scope_hbh	{ $$ = IPPROTO_HOPOPTS; }
 			;
 
 %%
+
+static int
+yyreport_syntax_error(const yypcontext_t *yyctx, struct nft_ctx *nft,
+                      void *scanner, struct parser_state *state)
+{
+	const char *bad_token = yysymbol_name(yypcontext_token(yyctx));
+	struct location *loc = yypcontext_location(yyctx);
+	yysymbol_kind_t *exp_tokens;
+	int exp_tokens_cnt;
+	size_t errbufsz;
+	FILE *errfp;
+	char *msg;
+
+	errfp = open_memstream(&msg, &errbufsz);
+	if (!errfp)
+		memory_allocation_error();
+
+	exp_tokens_cnt = yypcontext_expected_tokens(yyctx, NULL, 0);
+	exp_tokens = xmalloc_array(exp_tokens_cnt, sizeof(yysymbol_kind_t));
+	yypcontext_expected_tokens(yyctx, exp_tokens, exp_tokens_cnt);
+
+	fprintf(errfp, "syntax error, unexpected %s\nexpected any of: ", bad_token);
+
+	for (int i = 0; i < exp_tokens_cnt; i++) {
+		const char *token_name = yysymbol_name(exp_tokens[i]);
+		bool is_keyword = true;
+
+		/* tokens that name generic things shall be printed as <foo>; detect them */
+		switch (exp_tokens[i]) {
+		case YYSYMBOL_NUM:
+		case YYSYMBOL_STRING:
+		case YYSYMBOL_QUOTED_STRING:
+		case YYSYMBOL_ASTERISK_STRING:
+			is_keyword = false;
+			break;
+		default:
+			break;
+		}
+
+		if (i > 0)
+			fputs(", ", errfp);
+		if (!is_keyword)
+			fputc('<', errfp);
+		fputs(token_name, errfp);
+		if (!is_keyword)
+			fputc('>', errfp);
+	}
+
+	free(exp_tokens);
+	fclose(errfp);
+	/* no newline on the end of the error message; this is intended */
+	yyerror(loc, nft, scanner, state, msg);
+
+	free(msg);
+	return 0;
+}
-- 
2.52.0

Re: [PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Jan Kończak]

>  v2: prefer stdio (fprintf+memopen) vs. manual realloc of a cstring
>  buffer, align more with nftables coding style.
> 
>  I'll apply this unless there are any objections.

None on my side; I should have resubmitted the patch corrected of basis
of your comments, but I simply did not find time yet to look into that.

Re: [PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Florian Westphal]

Jan Kończak <jan.konczak@cs.put.poznan.pl> wrote:
> >  v2: prefer stdio (fprintf+memopen) vs. manual realloc of a cstring
> >  buffer, align more with nftables coding style.
> > 
> >  I'll apply this unless there are any objections.
> 
> None on my side; I should have resubmitted the patch corrected of basis
> of your comments, but I simply did not find time yet to look into that.

Applied, thanks!
If you like you could follow up so that for

'create fable filter'
you get:
Error: syntax error, unexpected string, did you mean 'table'?
expected any of: synproxy, chain, ...

string_misspell_init() + string_misspell_update() should
do the trick.

Re: [PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Phil Sutter]

Hi Jan,

On Tue, Jan 20, 2026 at 06:33:38PM +0100, Jan Kończak wrote:
> >  v2: prefer stdio (fprintf+memopen) vs. manual realloc of a cstring
> >  buffer, align more with nftables coding style.
> > 
> >  I'll apply this unless there are any objections.
> 
> None on my side; I should have resubmitted the patch corrected of basis
> of your comments, but I simply did not find time yet to look into that.

Since parse.error=custom is not supported by bison < 3.6, this patch
breaks nftables compiles on older systems (RHEL8 for instance). Sadly I
haven't found a way to change the define based on configure results
(checking bison version is easy via AX_PROG_BISON_VERSION). On one hand,
bison's declaration section does not support preprocessor macros. On the
other, conditional setting of AM_YFLAGS variable in Makefile.am is
problematic (and breaks tests/compile for me).

Do you perhaps know how to solve this? For reference, what I tried was:

|   AM_YFLAGS = -d -Wno-yacc
|  +if BISON_CUSTOM_ERROR
|  +AM_YFLAGS += -D parse.error=custom -D parse.lac=full
|  +else
|  +AM_YFLAGS += -D parse.error=verbose
|  +endif

And then drop the two defines from parser_bison.y.

Cheers, Phil

Re: [PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Jan Kończak]

> conditional setting of AM_YFLAGS variable in Makefile.am is problematic
> (...)
> what I tried was:
> |   AM_YFLAGS = -d -Wno-yacc
> |  
> |  +if BISON_CUSTOM_ERROR
> |  +AM_YFLAGS += …

Any conditional change in Makefile.am I came up with is put aside by
autotools. The lines "AM_YFLAGS += ..." end up in a ".PRECIOUS" target
in the resulting Makefile and are never applied before bison is called.

I guess moving the %define's from parser_bison.y to bison command line
arguments, and a '#if YYBISON >= 30704' that disables the error
reporting code is a way to go, one only must subdue the autotools.
The YYBISON macro generated by bison has the version encoded starting
from 3.7.4, hence that particular version.

My (working) take on that would be as follows, though I can tell that
mangling the options in configure.ac is a shitty workaround. I never
really wrote autotools configs myself, and I don't have a clue how to
do it better. Feel free to include any part of it in case it helps.

Regards, Jan

====================================================================
diff --git a/configure.ac b/configure.ac
index 0d3ee2ac..9b61cd38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -45,6 +45,13 @@ then
         exit 1
 fi
 
+AC_PATH_PROG([BISON],[bison])
+AX_PROG_BISON_VERSION([3.7.4],[
+	AC_SUBST([YACC], [$(echo "$ac_cv_prog_YACC -D parse.error=custom -D parse.lac=full")])
+],[
+	AC_SUBST([YACC], [$(echo "$ac_cv_prog_YACC -D parse.error=verbose")])
+])
+
 AM_PROG_AR
 LT_INIT([disable-static])
 AC_EXEEXT
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 5a334bf0..25b875ec 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -221,8 +221,14 @@ int nft_lex(void *, void *, void *);
 %parse-param		{ void *scanner }
 %parse-param		{ struct parser_state *state }
 %lex-param		{ scanner }
-%define parse.error custom
-%define parse.lac full
+// Note: configure.ac controls the following defines:
+// if bison version is 3.7.4 or newer:
+//   %define parse.lac full
+//   %define parse.error custom
+// else
+//   %define parse.error verbose
+// Notice that bison 3.6 introduces parse.error custom, but starting from
+// 3.7.4 a numeric macro enablkes checking version
 %locations
 
 %initial-action {
@@ -6537,6 +6543,7 @@ exthdr_key		:	HBH	close_scope_hbh	{ $$ = IPPROTO_HOPOPTS; }
 
 %%
 
+#if YYBISON >= 30704
 static int
 yyreport_syntax_error(const yypcontext_t *yyctx, struct nft_ctx *nft,
                       void *scanner, struct parser_state *state)
@@ -6592,3 +6599,4 @@ yyreport_syntax_error(const yypcontext_t *yyctx, struct nft_ctx *nft,
 	free(msg);
 	return 0;
 }
+#endif

Re: [PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Phil Sutter]

Hi Jan,

On Thu, Jun 04, 2026 at 02:34:55PM +0200, Jan Kończak wrote:
> > conditional setting of AM_YFLAGS variable in Makefile.am is problematic
> > (...)
> > what I tried was:
> > |   AM_YFLAGS = -d -Wno-yacc
> > |  
> > |  +if BISON_CUSTOM_ERROR
> > |  +AM_YFLAGS += …
> 
> Any conditional change in Makefile.am I came up with is put aside by
> autotools. The lines "AM_YFLAGS += ..." end up in a ".PRECIOUS" target
> in the resulting Makefile and are never applied before bison is called.
> 
> I guess moving the %define's from parser_bison.y to bison command line
> arguments, and a '#if YYBISON >= 30704' that disables the error
> reporting code is a way to go, one only must subdue the autotools.
> The YYBISON macro generated by bison has the version encoded starting
> from 3.7.4, hence that particular version.
> 
> My (working) take on that would be as follows, though I can tell that
> mangling the options in configure.ac is a shitty workaround. I never
> really wrote autotools configs myself, and I don't have a clue how to
> do it better. Feel free to include any part of it in case it helps.
> 
> Regards, Jan
> 
> ====================================================================
> diff --git a/configure.ac b/configure.ac
> index 0d3ee2ac..9b61cd38 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -45,6 +45,13 @@ then
>          exit 1
>  fi
>  
> +AC_PATH_PROG([BISON],[bison])
> +AX_PROG_BISON_VERSION([3.7.4],[
> +	AC_SUBST([YACC], [$(echo "$ac_cv_prog_YACC -D parse.error=custom -D parse.lac=full")])
> +],[
> +	AC_SUBST([YACC], [$(echo "$ac_cv_prog_YACC -D parse.error=verbose")])
> +])
> +

Ah, modifying the "command" didn't come to mind. Doing that is
usually error-prone Since scripts may start searching for 'mycmd
--option' in $PATH and what not. I'd go with it for lack of
alternatives, though.

My (uglier) variant is:

| have_bison_3_6=no
| AC_PATH_PROG([BISON], [bison])
| AX_PROG_BISON_VERSION([3.6], [
|                      have_bison_3_6=yes
|                      AC_DEFINE([BISON_CUSTOM_ERROR], [1],
|                                [Define to use parse.error=custom in Bison])
| ])
| AM_CONDITIONAL([BISON_CUSTOM_ERROR], [test "x$have_bison_3_6" != xno])

This is me failing to find a macro which creates both a variable to test
against for Makefile.am and a preprocessor macro. I guess one may join
the two into:

| AC_PATH_PROG([BISON],[bison])
| AX_PROG_BISON_VERSION([3.6], [
| 	AC_SUBST([YACC], [$(echo "$ac_cv_prog_YACC -D parse.error=custom -D parse.lac=full")])
| 	AC_DEFINE([HAVE_BISON_CUSTOM_ERROR], [1], [Define to use parse.error=custom in Bison])
| ], [
| 	AC_SUBST([YACC], [$(echo "$ac_cv_prog_YACC -D parse.error=verbose")])
| ])

Then, one would wrap yyreport_syntax_error into '#ifdef
HAVE_BISON_CUSTOM_ERROR'. Or am I missing some detail about YYBISON?

Thanks, Phil

Re: [PATCH nft v2] parser_bison: on syntax errors, output expected tokens

[Jan Kończak]

> Or am I missing some detail about YYBISON?

No, I don't think you're missing any detail there. I considered this
less intrusive than forcing a define in the configure.ac. Adding a C
macro BISON_CUSTOM_ERROR from configure is actually more versatile.


I just discovered that while autotools forbid conditional AM_YFLAGS,
they allow conditional YACC. It's counterintuitive to me why.
Yet, the following works in Makefile.am:

| if BISON_CUSTOM_ERROR
| YACC += -D parse.error=custom -D parse.lac=full
| AM_CFLAGS += -DBISON_CUSTOM_ERROR
| else
| YACC += -D parse.error=verbose
| endif

Then, the configure.ac has just to set the BISON_CUSTOM_ERROR, without
putting there the flags/define which nobody expects to be there.
To me this looks cleaner.


I guess adding a AC_ARG_ENABLE on that might be worthwhile, too.
Running "nft --debug parser ..." with parse.lac=full yields extra
output which one may or may not prefer if one wishes to debug
parser_bison.y. I mean, something akin to this:

| AC_ARG_ENABLE([custom_parser_errors],
|        AS_HELP_STRING(
|                [--disable-custom-parser-errors],
|                [Disable use of parse.error=custom and LAC in Bison]
|        ),[
|                # keeping the user choice for custom-parser-errors
|        ],[
|                enable_custom_parser_errors=no
|                AC_SUBST([BISON],[$ac_cv_prog_YACC])
|                AX_PROG_BISON_VERSION([3.6],
|                    [enable_custom_parser_errors=yes])
|        ]
| )
| AM_CONDITIONAL([BISON_CUSTOM_ERROR],
|                [test "x$enable_custom_parser_errors" != xno])


Regards, Jan