On Wed, Nov 13, 2019 at 11:10:50AM +0100, Phil Sutter wrote:
> 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.
It would be probably good to restrict set references to where it makes
sense only. This is good for the grammar and we don't need to validate
all possible invalid combinations from the evaluation step.
Would you have a look or you think it's too complicated to attack this
from the parser?
