Re: [PATCH] Fix 'tcp-request content [accept|reject] if condition' parser for missing 'if'.

Hi,

Willy Tarreau <w#1wt.eu> wrote:
> Hi Maik,
>
> On Tue, May 12, 2009 at 01:36:46AM +0200, Maik Broemme wrote:
> > Hi,
> >
> > attached is a patch which fixes a configuration mistake regarding the
> > 'tcp-request' option. If you have the following in your configuration
> > file:
> >
> > acl localnet dst 10.0.0.0/8
> > tcp-request content reject if localnet
> >
> > This will work fine, but if you change the 'tcp-request' line and remove
> > the 'if' haproxy-1.3.17 will segfault, I think the following changelog
> > entry in 1.3.18 addresses this problem:
> >
> > [BUG] fix parser crash on unconditional tcp content rules
>
> yes precisely.
>
> > But now in 1.3.18 the default behaviour is a bit weird. If you remove
> > the 'if' statement the haproxy will reject every connection, regardless
> > of matching to 'localnet' or not and the configuration seems to be valid,
> > but which is definetly not what expected.
>
> I can't reproduce the issue here. For me, what happens is the right thing :
>
> - the following config rejects everything :
>
> tcp-request content reject
>
> - the following config rejects everything which was not accepted :
>
> tcp-request content accept if <cond>
> tcp-request content reject
>
> - the following config rejects only everything which matches the condition :
>
> tcp-request content reject if <cond>
>
> The second case above was precisely what led me to discover the segfault
> bug, which was introduced in 1.3.17 with the refinement of the config
> warnings. But the behaviour has not changed since 1.3.16.
>

You have missed the non-working case. :-)

• the following config seems to be ok, but didn't work as expected.

    tcp-request content reject <cond>

This is just because of the missing 'if' and in 1.3.17 this missing 'if' result in a crash. A crash isn't better, but in case of crash you know that something was misconfigured.

> > I have changed this to the following behaviour: If nothing is specified
> > after accept or reject the default condition will apply (like source and
> > documentation says) and if there is some parameter after accept or
> > reject it has to be 'if' or 'unless' anything else will result in:
> >
> > [ALERT] 131/012555 (27042) : parsing [/etc/haproxy/haproxy.cfg:94] :
> > 'tcp-request content reject' expects 'if', 'unless' or nothing, but
> > found 'localnet'
> > [ALERT] 131/012555 (27042) : Error reading configuration file :
> > /etc/haproxy/haproxy.cfg
> >
> > I think this is much more accurate. At least it took me some time to
> > verify why the hell my configuration file is valid, but did not work as
> > expected. :)
>
> in fact not, that's precisely what I don't want. To workaround the bug
> I encountered, I had to write that :
>
> tcp-request content accept if <cond>
> tcp-request content reject if TRUE
>
> That's pretty annoying. All conditionnal actions support either
> "if/unless cond" or inconditional execution if no condition is
> specified.
>
> Are you sure your config was OK ? Can you post the example which
> causes you trouble ? Maybe your example is right and the doc is
> wrong ;-)
>

Sure I have attached the file. If you remove the 'if' in the 'tcp-request' the config file is ok, haproxy starts but every request from everywhere is dropped.

> Regards,
> Willy
>

--Maik