On Fri, Feb 12, 2021 at 06:32:01PM +0100, Phil Sutter wrote:
> Hi,
>
> On Fri, Feb 12, 2021 at 06:09:21PM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Feb 12, 2021 at 01:20:07PM +0100, Florian Westphal wrote:
> > > Phil Sutter <phil@xxxxxx> wrote:
> > > > I didn't find a better way to conditionally parse two following args as
> > > > strings instead of just a single one. Basically I miss an explicit end
> > > > condition from which to call BEGIN(0).
> > >
> > > Yes, thats part of the problem.
> > >
> > > > > Seems we need allow "{" for "*" and then count the {} nests so
> > > > > we can pop off a scanner state stack once we make it back to the
> > > > > same } level that we had at the last state switch.
> > > >
> > > > What is the problem?
> > >
> > > Detect when we need to exit the current start condition.
> > >
> > > We may not even be able to do BEGIN(0) if we have multiple, nested
> > > start conditionals. flex supports start condition stacks, but that
> > > still leaves the exit/closure issue.
> > >
> > > Example:
> > >
> > > table chain {
> > > chain bla { /* should start to recognize rules, but
> > > we did not see 'rule' keyword */
> > > ip saddr { ... } /* can't exit rule start condition on } ... */
> > > ip daddr { ... }
> > > } /* should disable rule keywords again */
> > >
> > > chain dynamic { /* so 'dynamic' is a string here ... */
> > > }
> > > }
> > >
> > > I don't see a solution, perhaps add dummy bison rule(s)
> > > to explicitly signal closure of e.g. a rule context?
> >
> > It should also be possible to add an explicit rule to allow for
> > keywords to be used as table/chain/... identifier.
>
> Which means we have to collect and maintain a list of all known keywords
> which is at least error-prone.
You mean, someone might forget to update the list of keywords.
That's right.
> > It should be possible to add a test script in the infrastructure to
> > create table/chain/... using keywords, to make sure this does not
> > break.
>
> You mean something that auto-generates the list of keywords to try?
Autogenerating this list would be good, I didn't good that far in
exploring this.
Or just making a shell script that extracts the %token lines to try to
create table with a keyword as a name.
The shell script would just have a "list of unallowed keyword" to
filter out the %tokens that are not allowed, for those tokens that are
really reserved keywords.
> > It's not nice, but it's simple and we don't mingle with flex.
> >
> > I have attached an example patchset (see patch 2/2), it's incomplete.
> > I could also have a look at adding such regression test.
>
> Ah, I tried that path but always ended with shift/reduce conflicts. They
> appear when replacing DYNAMIC with e.g. TABLE, CHAIN or RULE in your
> patch.
Probably we have to set some explicit restrictions, like table, chain,
rule, set, map and flowtable are reserved keywords. For example, not
allowing to call a table '>'. That was not possible since the
beginning anyway.
The concern is to add a new token and break backward as it happened
with 'dynamic' as Florian reported I think.
> Of course we may declare that none of those is a sane name for a
> table, but I wonder if we'll discover less obvious cases later.
BTW, Florian mentioned your patch makes unhappy the tests infra?
What's the issue?
