Re: [nft PATCH] evaluate: Reject set references in mapping LHS

[Phil Sutter <phil@nwl.cc>]

On Tue, Nov 12, 2019 at 11:18:27PM +0100, Phil Sutter wrote:
> On Tue, Nov 12, 2019 at 10:45:18PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Oct 31, 2019 at 07:21:24PM +0100, Phil Sutter wrote:
> > > This wasn't explicitly caught before causing a program abort:
> > > 
> > > | BUG: invalid range expression type set reference
> > > | nft: expression.c:1162: range_expr_value_low: Assertion `0' failed.
> > > | zsh: abort      sudo ./install/sbin/nft add rule t c meta mark set tcp dport map '{ @s : 23 }
> > > 
> > > With this patch in place, the error message is way more descriptive:
> > > 
> > > | Error: Key can't be set reference
> > > | add rule t c meta mark set tcp dport map { @s : 23 }
> > > |                                            ^^
> > 
> > I wanted to check why the parser allow for this...
> 
> For set elements or LHS parts of map elements, there is set_lhs_expr.
> The latter may be concat_rhs_expr or multiton_rhs_expr. concat_rhs_expr
> eventually resolves into primary_rhs_expr which may be symbol_expr.
> 
> BTW, it seems like from parser side, set references on map element's
> RHS are allowed as well.
> 
> IMHO, parser_bison.y slowly but steadily turns into a can of worms. :(

On a second look, I start wondering whether symbol_expr was a wise
choice: This thing combines variables ('$' identifier), "unidentified"
strings and set references (AT identifier).

With symbol_expr being listed in both primary_expr and primary_rhs_expr,
set references are allowed about anywhere - even in concatenations or
any binary operation.

Cheers, Phil