Hi Tim,

On Wed, Feb 05, 2020 at 09:00:50PM +0100, Tim Duesterhus wrote:
(...)
> Consider a configuration like this:
> 
> > acl t always_true
> > acl or always_false
> >
> > http-response set-header Foo Bar if t or t
> 
> The 'or' within the condition will be treated as a logical disjunction
> and the header will be set, despite the ACL 'or' being falsy.

I was surprized because I discovered that we've always accepted empty
expressions thus"if" without even a condition or "if or" are valid, which
adds to the confusion. So your patch does indeed make sense. However we
don't need to test for "||" because it's already invalid as an ACL name
and the existing message is more precise about it (invalid chars in the
name).

Also I'd rather have the "goto out" statement after setting the error,
otherwise it becomes confusing and will certainly trap someone adding
yet another test later.

> May be backported to older branches, it should not break anything
> and might improve the users' lifes.

I also think we should, indeed.

Willy

Reply via email to