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

Reply via email to