Hi Florian,
On Mon, Jun 16, 2025 at 11:16:18AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > > > I wonder if we should just replace all BUGs in evaluate.c
> > > > with expr_error() calls, it avoids constant whack-a-mole.
> >
> > I think that can help uncover bugs, or are those json induced bugs?
>
> This one is bison, see included bogon input.
For this particular case I think it makes sense to turn it into error.
Nitpick for this patch: I'd suggest this error:
+ return expr_error(ctx->msgs, *expr, "Unexpected datatype %s",
+ (*expr)->dtype->name);
Which results in:
[...]
ruleset:3:61-67: Error: Unexpected datatype verdict
type ipv4_addr . inet_service : ipv4_addr . verdict
^^^^^^^
> I meant in general.
It's not so straight forward, I have quickly browsed the existing
BUG() calls and, unlike this one, they are mostly trapping nonsense,
I guess we can find a way to keep them around while not invoking
assert().
> How do these BUGs help at all?
Maybe you can turn them into error record adding "BUG:" prefix if
you prefer, I guess that will not crash a daemon process which is your
concern?
ie. add expr_bug() to highlight an input is triggering a buggy internal
state so this can still be reported without crashing the daemon
process?
