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
