MEDIUM: arg: make make_arg_list() stop after its own arguments

The main problem we're having with argument parsing is that at the
moment the caller looks for the first character looking like an end
of arguments (')') and calls make_arg_list() on the sub-string inside
the parenthesis.

Let's first change the way it works so that make_arg_list() also
consumes the parenthesis and returns the pointer to the first char not
consumed. This will later permit to refine each argument parsing.

For now there is no functional change.
This commit is contained in:
Willy Tarreau 2020-02-14 08:40:37 +01:00
parent ed2c662b01
commit 80b53ffb1c
4 changed files with 95 additions and 120 deletions

View File

@ -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 */

View File

@ -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 <err> 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;
}
}

View File

@ -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 <al>, 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 <err_msg> 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 <end_ptr>. 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 <al>, 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 <err_msg> 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 <end_ptr>, which is the first character considered as not
* being part of the arg list. The input string ends on the first between <len>
* characters (when len is positive) or the first NUL character. Placing -1 in
* <len> 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 <in> */
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:

View File

@ -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;