diff --git a/include/proto/arg.h b/include/proto/arg.h index db63b0e21..884e5bbf2 100644 --- a/include/proto/arg.h +++ b/include/proto/arg.h @@ -80,7 +80,7 @@ extern struct arg empty_arg_list[ARGM_NBARGS]; struct arg_list *arg_list_clone(const struct arg_list *orig); struct arg_list *arg_list_add(struct arg_list *orig, struct arg *arg, int pos); int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, - char **err_msg, const char **err_ptr, int *err_arg, + char **err_msg, const char **end_ptr, int *err_arg, struct arg_list *al); #endif /* _PROTO_ARG_H */ diff --git a/src/acl.c b/src/acl.c index bef3d4ea1..4b8a6d49a 100644 --- a/src/acl.c +++ b/src/acl.c @@ -193,27 +193,12 @@ struct acl_expr *parse_acl_expr(const char **args, char **err, struct arg_list * for (arg = args[0]; is_idchar(*arg); arg++) ; - endt = arg; - if (*endt == '(') { - /* look for the end of this term and skip the opening parenthesis */ - endt = ++arg; - while (*endt && *endt != ')') - endt++; - if (*endt != ')') { - memprintf(err, "missing closing ')' after arguments to ACL keyword '%s'", aclkw->kw); - goto out_free_smp; - } - } - /* At this point, we have : * - args[0] : beginning of the keyword * - arg : end of the keyword, first character not part of keyword - * nor the opening parenthesis (so first character of args - * if present). - * - endt : end of the term (=arg or last parenthesis if args are present) */ - nbargs = make_arg_list(arg, endt - arg, smp->fetch->arg_mask, &smp->arg_p, - err, NULL, NULL, al); + nbargs = make_arg_list(arg, -1, smp->fetch->arg_mask, &smp->arg_p, + err, &endt, NULL, al); if (nbargs < 0) { /* note that make_arg_list will have set here */ memprintf(err, "ACL keyword '%s' : %s", aclkw->kw, *err); @@ -241,9 +226,8 @@ struct acl_expr *parse_acl_expr(const char **args, char **err, struct arg_list * while (*arg) { struct sample_conv *conv; struct sample_conv_expr *conv_expr; - - if (*arg == ')') /* skip last closing parenthesis */ - arg++; + int err_arg; + int argcnt; if (*arg && *arg != ',') { if (ckw) @@ -255,6 +239,9 @@ struct acl_expr *parse_acl_expr(const char **args, char **err, struct arg_list * goto out_free_smp; } + /* FIXME: how long should we support such idiocies ? Maybe we + * should already warn ? + */ while (*arg == ',') /* then trailing commas */ arg++; @@ -279,16 +266,6 @@ struct acl_expr *parse_acl_expr(const char **args, char **err, struct arg_list * } arg = endw; - if (*arg == '(') { - /* look for the end of this term */ - while (*arg && *arg != ')') - arg++; - if (*arg != ')') { - memprintf(err, "ACL keyword '%s' : syntax error: missing ')' after converter '%s'.", - aclkw->kw, ckw); - goto out_free_smp; - } - } if (conv->in_type >= SMP_TYPES || conv->out_type >= SMP_TYPES) { memprintf(err, "ACL keyword '%s' : returns type of converter '%s' is unknown.", @@ -312,35 +289,26 @@ struct acl_expr *parse_acl_expr(const char **args, char **err, struct arg_list * conv_expr->conv = conv; acl_conv_found = 1; - if (arg != endw) { - int err_arg; - - if (!conv->arg_mask) { - memprintf(err, "ACL keyword '%s' : converter '%s' does not support any args.", - aclkw->kw, ckw); - goto out_free_smp; - } - - al->kw = smp->fetch->kw; - al->conv = conv_expr->conv->kw; - if (make_arg_list(endw + 1, arg - endw - 1, conv->arg_mask, &conv_expr->arg_p, err, NULL, &err_arg, al) < 0) { - memprintf(err, "ACL keyword '%s' : invalid arg %d in converter '%s' : %s.", - aclkw->kw, err_arg+1, ckw, *err); - goto out_free_smp; - } - - if (!conv_expr->arg_p) - conv_expr->arg_p = empty_arg_list; - - if (conv->val_args && !conv->val_args(conv_expr->arg_p, conv, file, line, err)) { - memprintf(err, "ACL keyword '%s' : invalid args in converter '%s' : %s.", - aclkw->kw, ckw, *err); - goto out_free_smp; - } + al->kw = smp->fetch->kw; + al->conv = conv_expr->conv->kw; + argcnt = make_arg_list(endw, -1, conv->arg_mask, &conv_expr->arg_p, err, &arg, &err_arg, al); + if (argcnt < 0) { + memprintf(err, "ACL keyword '%s' : invalid arg %d in converter '%s' : %s.", + aclkw->kw, err_arg+1, ckw, *err); + goto out_free_smp; } - else if (ARGM(conv->arg_mask)) { - memprintf(err, "ACL keyword '%s' : missing args for converter '%s'.", - aclkw->kw, ckw); + + if (argcnt && !conv->arg_mask) { + memprintf(err, "converter '%s' does not support any args", ckw); + goto out_free_smp; + } + + if (!conv_expr->arg_p) + conv_expr->arg_p = empty_arg_list; + + if (conv->val_args && !conv->val_args(conv_expr->arg_p, conv, file, line, err)) { + memprintf(err, "ACL keyword '%s' : invalid args in converter '%s' : %s.", + aclkw->kw, ckw, *err); goto out_free_smp; } } diff --git a/src/arg.c b/src/arg.c index 2719b5395..ce4904621 100644 --- a/src/arg.c +++ b/src/arg.c @@ -81,21 +81,30 @@ struct arg_list *arg_list_add(struct arg_list *orig, struct arg *arg, int pos) return new; } -/* This function builds an argument list from a config line. It returns the - * number of arguments found, or <0 in case of any error. Everything needed - * it automatically allocated. A pointer to an error message might be returned - * in err_msg if not NULL, in which case it would be allocated and the caller - * will have to check it and free it. The output arg list is returned in argp - * which must be valid. The returned array is always terminated by an arg of - * type ARGT_STOP (0), unless the mask indicates that no argument is supported. - * Unresolved arguments are appended to arg list , which also serves as a - * template to create new entries. The mask is composed of a number of - * mandatory arguments in its lower ARGM_BITS bits, and a concatenation of each - * argument type in each subsequent ARGT_BITS-bit sblock. If is not - * NULL, it must point to a freeable or NULL pointer. +/* This function builds an argument list from a config line, and stops at the + * first non-matching character, which is pointed to in . A valid arg + * list starts with an opening parenthesis '(', contains a number of comma- + * delimited words, and ends with the closing parenthesis ')'. An empty list + * (with or without the parenthesis) will lead to a valid empty argument if the + * keyword has a mandatory one. The function returns the number of arguments + * emitted, or <0 in case of any error. Everything needed it automatically + * allocated. A pointer to an error message might be returned in err_msg if not + * NULL, in which case it would be allocated and the caller will have to check + * it and free it. The output arg list is returned in argp which must be valid. + * The returned array is always terminated by an arg of type ARGT_STOP (0), + * unless the mask indicates that no argument is supported. Unresolved arguments + * are appended to arg list , which also serves as a template to create new + * entries. The mask is composed of a number of mandatory arguments in its lower + * ARGM_BITS bits, and a concatenation of each argument type in each subsequent + * ARGT_BITS-bit sblock. If is not NULL, it must point to a freeable + * or NULL pointer. The caller is expected to restart the parsing from the new + * pointer set in , which is the first character considered as not + * being part of the arg list. The input string ends on the first between + * characters (when len is positive) or the first NUL character. Placing -1 in + * will make it virtually unbounded (~2GB long strings). */ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, - char **err_msg, const char **err_ptr, int *err_arg, + char **err_msg, const char **end_ptr, int *err_arg, struct arg_list *al) { int nbarg; @@ -105,10 +114,22 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, char *word = NULL; const char *ptr_err = NULL; int min_arg; + int empty; struct arg_list *new_al = al; *argp = NULL; + empty = 0; + if (!len || *in != '(') { + /* it's already not for us, stop here */ + empty = 1; + len = 0; + } else { + /* skip opening parenthesis */ + len--; + in++; + } + min_arg = mask & ARGM_MASK; mask >>= ARGM_BITS; @@ -122,7 +143,7 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, /* Note: an empty input string contains an empty argument if this argument * is marked mandatory. Otherwise we can ignore it. */ - if (!len && !min_arg) + if (empty && !min_arg) goto end_parse; arg = *argp = calloc(nbarg + 1, sizeof(*arg)); @@ -132,7 +153,7 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, unsigned int uint; beg = in; - while (len && *in != ',') { + while (len && *in != ',' && *in && *in != ')') { in++; len--; } @@ -261,7 +282,7 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, arg++; /* don't go back to parsing if we reached end */ - if (!len || pos >= nbarg) + if (!len || !*in || *in == ')' || pos >= nbarg) break; /* skip comma */ @@ -279,16 +300,24 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, goto err; } - if (len) { - /* too many arguments, starting at */ + if (empty) { + /* nothing to do */ + } else if (*in == ')') { + /* skip the expected closing parenthesis */ + in++; + } else { /* the caller is responsible for freeing this message */ - word = my_strndup(in, len); - if (nbarg) - memprintf(err_msg, "end of arguments expected at position %d, but got '%s'", - pos + 1, word); - else - memprintf(err_msg, "no argument supported, but got '%s'", word); - free(word); word = NULL; + word = (len > 0) ? my_strndup(in, len) : (char *)in; + memprintf(err_msg, "expected ')' before '%s'", word); + if (len > 0) + free(word); + word = NULL; + /* when we're missing a right paren, the empty part preceeding + * already created an empty arg, adding one to the position, so + * let's fix the reporting to avoid being confusing. + */ + if (pos > 1) + pos--; goto err; } @@ -297,8 +326,8 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, */ if (err_arg) *err_arg = pos; - if (err_ptr) - *err_ptr = in; + if (end_ptr) + *end_ptr = in; return pos; err: @@ -312,8 +341,8 @@ int make_arg_list(const char *in, int len, uint64_t mask, struct arg **argp, *argp = NULL; if (err_arg) *err_arg = pos; - if (err_ptr) - *err_ptr = in; + if (end_ptr) + *end_ptr = in; return -1; empty_err: diff --git a/src/sample.c b/src/sample.c index d9439fb50..d82451fc3 100644 --- a/src/sample.c +++ b/src/sample.c @@ -856,24 +856,9 @@ struct sample_expr *sample_parse_expr(char **str, int *idx, const char *file, in goto out_error; } - endt = endw; - if (*endt == '(') { - /* look for the end of this term and skip the opening parenthesis */ - endt = ++endw; - while (*endt && *endt != ')') - endt++; - if (*endt != ')') { - memprintf(err_msg, "missing closing ')' after arguments to fetch keyword '%s'", fkw); - goto out_error; - } - } - /* At this point, we have : * - begw : beginning of the keyword * - endw : end of the keyword, first character not part of keyword - * nor the opening parenthesis (so first character of args - * if present). - * - endt : end of the term (=endw or last parenthesis if args are present) */ if (fetch->out_type >= SMP_TYPES) { @@ -896,11 +881,16 @@ struct sample_expr *sample_parse_expr(char **str, int *idx, const char *file, in */ al->kw = expr->fetch->kw; al->conv = NULL; - if (make_arg_list(endw, endt - endw, fetch->arg_mask, &expr->arg_p, err_msg, NULL, &err_arg, al) < 0) { + if (make_arg_list(endw, -1, fetch->arg_mask, &expr->arg_p, err_msg, &endt, &err_arg, al) < 0) { memprintf(err_msg, "fetch method '%s' : %s", fkw, *err_msg); goto out_error; } + /* now endt is our first char not part of the arg list, typically the + * comma after the sample fetch name or after the closing parenthesis, + * or the NUL char. + */ + if (!expr->arg_p) { expr->arg_p = empty_arg_list; } @@ -926,9 +916,6 @@ struct sample_expr *sample_parse_expr(char **str, int *idx, const char *file, in int err_arg; int argcnt; - if (*endt == ')') /* skip last closing parenthesis */ - endt++; - if (*endt && *endt != ',') { if (ckw) memprintf(err_msg, "missing comma after converter '%s'", ckw); @@ -937,6 +924,9 @@ struct sample_expr *sample_parse_expr(char **str, int *idx, const char *file, in goto out_error; } + /* FIXME: how long should we support such idiocies ? Maybe we + * should already warn ? + */ while (*endt == ',') /* then trailing commas */ endt++; @@ -965,18 +955,6 @@ struct sample_expr *sample_parse_expr(char **str, int *idx, const char *file, in goto out_error; } - endt = endw; - if (*endt == '(') { - /* look for the end of this term */ - endt = ++endw; - while (*endt && *endt != ')') - endt++; - if (*endt != ')') { - memprintf(err_msg, "syntax error: missing ')' after converter '%s'", ckw); - goto out_error; - } - } - if (conv->in_type >= SMP_TYPES || conv->out_type >= SMP_TYPES) { memprintf(err_msg, "returns type of converter '%s' is unknown", ckw); goto out_error; @@ -998,7 +976,7 @@ struct sample_expr *sample_parse_expr(char **str, int *idx, const char *file, in al->kw = expr->fetch->kw; al->conv = conv_expr->conv->kw; - argcnt = make_arg_list(endw, endt - endw, conv->arg_mask, &conv_expr->arg_p, err_msg, NULL, &err_arg, al); + argcnt = make_arg_list(endw, -1, conv->arg_mask, &conv_expr->arg_p, err_msg, &endt, &err_arg, al); if (argcnt < 0) { memprintf(err_msg, "invalid arg %d in converter '%s' : %s", err_arg+1, ckw, *err_msg); goto out_error;