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 # global configuration section. global maxconn 32768 chroot /var/lib/haproxy userhaproxy group haproxy daemon quiet # default configuration and timeouts. defaults log global retries 10 maxconn 32768 timeout connect 60s timeout server 60s timeout client 60s timeout queue 60s timeout tarpit 60s # service and balance configuration. listen client-filter 10.0.1.7:10080 modehttp cookie SERVERID nocache balance roundrobin acl localnet-1 dst 192.168.0.0/16 acl localnet-2 dst 172.16.0.0/12 acl localnet-3 dst 10.0.0.0/8 tcp-request content reject if localnet-1 tcp-request content reject if localnet-2 tcp-request content reject if localnet-3 option forwardfor header X-Forwarded-For option originalto header X-Original-To option httpclose server squid1 10.0.3.10:3128 cookie squid1 check inter 10s rise 1 fall 10 server squid2 10.0.3.11:3128 cookie squid2 check inter 10s rise 1 fall 10 server squid3 10.0.3.12:3128 cookie squid3 check inter 10s rise 1 fall 10 # service for client login. listen client-login10.0.1.7:10081 modehttp cookie SERVERID nocache balance roundrobin acl localnet-1 dst 192.168.0.0/16 acl localnet-2 dst 172.16.0.0/12 acl localnet-3 dst 10.0.0.0/8
Re: [PATCH] Fix 'tcp-request content [accept|reject] if condition' parser for missing 'if'.
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. 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 ;-) Regards, Willy
[PATCH] Fix 'tcp-request content [accept|reject] if condition' parser for missing 'if'.
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
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 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. :)
--Maik
diff -Nur haproxy-1.3.18/src/proto_tcp.c
haproxy-1.3.18-tcp-request-condition-fix/src/proto_tcp.c
--- haproxy-1.3.18/src/proto_tcp.c 2009-05-10 20:27:47.0 +0200
+++ haproxy-1.3.18-tcp-request-condition-fix/src/proto_tcp.c2009-05-12
01:25:48.0 +0200
@@ -509,6 +509,13 @@
pol = ACL_COND_IF;
else if (!strcmp(args[3], unless))
pol = ACL_COND_UNLESS;
+ else {
+ if (args[3][0] != '\0') {
+ snprintf(err, errlen, '%s %s %s' expects 'if',
'unless' or nothing, but found '%s',
+args[0], args[1], args[2], args[3]);
+ return -1;
+ }
+ }
/* Note: we consider if TRUE when there is no condition */
if (pol != ACL_COND_NONE
