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