Hi Cyril,
On Thu, Dec 14, 2017 at 06:48:28PM +0100, Cyril Bonté wrote:
> It's embarrassing, I was working on a patch for a nasty bug I've seen
> several times last week (randomly haproxy won't start), but now I've updated
> to the current master branch, it's even worse with this specific patch.
>
> Example :
> listen capture
> bind :9000
> mode http
> http-request capture hdr(X-Forwarded-Proto) len 8
>
> With the new "len" converter, it fails with :
> parsing [capture.cfg:4] : error detected in proxy 'capture' while parsing
> 'http-request capture' rule : expects 'len' or 'id', found '8'.
>
> It seems that we have to fix sample_parse_expr(). Currently, be careful if
> you want to backport it.
Wow that's very strange. I suspect we indeed have a bug somewhere in the
sample parser, very likely it reads past the last delimiter and goes on
with the word it has found that happens to match a registered converter.
And indeed, if I go further I get the same error by giving this :
http-request capture hdr(X-Forwarded-Proto) upper lower base64 sha1 len 8
But placing "foo" in the middle immediately causes an error. That's scary!
> Otherwise, my original bug concerns the exact same configuration and looks
> to exist for a long time : there is an unwanted "check_ptr" assignment in
> parse_http_req_capture() when the "len" parameter is set instead of "id".
>
> diff --git a/src/proto_http.c b/src/proto_http.c
> index 1330ac864..481688fdf 100644
> --- a/src/proto_http.c
> +++ b/src/proto_http.c
> @@ -12149,7 +12149,6 @@ enum act_parse_ret parse_http_req_capture(const char
> **args, int *orig_arg, stru
>
> rule->action = ACT_CUSTOM;
> rule->action_ptr = http_action_req_capture;
> - rule->check_ptr = check_http_req_capture;
> rule->arg.cap.expr = expr;
> rule->arg.cap.hdr = hdr;
> }
Well, at first glance I didn't agree but in fact you're totally right,
the "len" and "id" are mutually exclusive and the one above if only for
the id.
If you want to make a patch for this one, in parallel I'm checking
sample_parse_expr().
Thanks!
Willy