Le 24/11/2016 à 19:50, Willy Tarreau a écrit :
Hi Christopher,
On Thu, Nov 24, 2016 at 03:06:13PM +0100, Christopher Faulet wrote:
>From 7ed3c2942d57ea2ddfc8973cce9cc1c94bca01da Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Thu, 24 Nov 2016 14:53:22 +0100
Subject: [PATCH] BUG: spoe: Fix parsing of SPOE actions in ACK frames
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4
For "SET-VAR" actions, data was not correctly parsed. 'idx' variable was not
correctly updated when the 3rd argument was parsed.
Are you sure your patch is correct ? I think it's bogus :
diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 0b722b6..8227140 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -2017,7 +2017,7 @@ process_spoe_actions(struct stream *s, struct
spoe_context *ctx,
goto skip;
memset(&smp, 0, sizeof(smp));
smp_set_owner(&smp, s->be, s->sess, s,
dir|SMP_OPT_FINAL);
- if (decode_spoe_data(p+idx, p+size, &smp) == -1)
+ if ((idx += decode_spoe_data(p+idx, p+size,
&smp)) == -1)
goto skip;
The only case it will work is when idx = 0 before decoding, which doesn't
really look like the only case you're interested in. I guess you wanted to
do this instead :
- if (decode_spoe_data(p+idx, p+size, &smp) == -1)
+ ret = decode_spoe_data(p+idx, p+size, &smp);
+ if (ret == -1)
goto skip;
+ idx += ret;
Am I wrong ? That's the reason why I hate assignments in "if" conditions,
half of the time they are bogus, the other half they make the reader
scratch his head wondering if it's bogus or intended :-)
Ahhhhh of course you're right ! too tired...
New patch attached. thanks.
--
Christopher
>From 1b46491bf93c76602122ea5814c42fdcfbf2d816 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Thu, 24 Nov 2016 14:53:22 +0100
Subject: [PATCH] BUG: spoe: Fix parsing of SPOE actions in ACK frames
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4
For "SET-VAR" actions, data was not correctly parsed. 'idx' variable was not
correctly updated when the 3rd argument was parsed.
---
src/flt_spoe.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 0b722b6..776848e 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -2017,8 +2017,10 @@ process_spoe_actions(struct stream *s, struct spoe_context *ctx,
goto skip;
memset(&smp, 0, sizeof(smp));
smp_set_owner(&smp, s->be, s->sess, s, dir|SMP_OPT_FINAL);
- if (decode_spoe_data(p+idx, p+size, &smp) == -1)
+
+ if ((i = decode_spoe_data(p+idx, p+size, &smp)) == -1)
goto skip;
+ idx += i;
SPOE_PRINTF(stderr, "%d.%06d [SPOE/%-15s] %s: stream=%p"
" - set-var '%s.%s.%.*s'\n",
--
2.7.4
