MEDIUM: arg: copy parsed arguments into the trash instead of allocating them
For each and every argument parsed by make_arg_list(), there was an
strndup() call, just so that we have a trailing zero for most functions,
and this temporary buffer is released afterwards except for strings where
it is kept.
Proceeding like this is not convenient because 1) it performs a huge
malloc/free dance, and 2) it forces to decide upfront where the argument
ends, which is what prevents commas and right parenthesis from being used.
This patch makes the function copy the temporary argument into the trash
instead, so that we avoid the malloc/free dance for most all non-string
args (e.g. integers, addresses, time, size etc), and that we can later
produce the contents on the fly while parsing the input. It adds a length
check to make sure that the argument is not longer than the buffer size,
which should obviously never be the case but who knows what people put in
their configuration.
diff --git a/src/arg.c b/src/arg.c
index ce49046..927aaa4 100644
--- a/src/arg.c
+++ b/src/arg.c
@@ -15,6 +15,8 @@
#include <arpa/inet.h>
#include <common/standard.h>
+#include <common/chunk.h>
+#include <types/global.h>
#include <proto/arg.h>
const char *arg_type_names[ARGT_NBTYPES] = {
@@ -111,7 +113,6 @@
int pos;
struct arg *arg;
const char *beg;
- char *word = NULL;
const char *ptr_err = NULL;
int min_arg;
int empty;
@@ -163,17 +164,18 @@
* By default, the output argument will be the same type of the
* expected one.
*/
- free(word);
- word = my_strndup(beg, in - beg);
+ if (!chunk_strncpy(&trash, beg, in - beg))
+ goto buffer_err;
arg->type = (mask >> (pos * ARGT_BITS)) & ARGT_MASK;
switch (arg->type) {
case ARGT_SINT:
- if (in == beg) // empty number
+ if (!trash.data) // empty number
goto empty_err;
- arg->data.sint = read_int64(&beg, in);
- if (beg < in)
+ beg = trash.area;
+ arg->data.sint = read_int64(&beg, trash.area + trash.data);
+ if (beg < trash.area + trash.data)
goto parse_err;
arg->type = ARGT_SINT;
break;
@@ -196,56 +198,55 @@
* during the parsing. The caller must at one point resolve
* them and free the string.
*/
- arg->data.str.area = word;
- arg->data.str.data = in - beg;
- arg->data.str.size = arg->data.str.data + 1;
- word = NULL;
+ arg->data.str.area = my_strndup(trash.area, trash.data);
+ arg->data.str.data = trash.data;
+ arg->data.str.size = trash.data + 1;
break;
case ARGT_IPV4:
- if (in == beg) // empty address
+ if (!trash.data) // empty address
goto empty_err;
- if (inet_pton(AF_INET, word, &arg->data.ipv4) <= 0)
+ if (inet_pton(AF_INET, trash.area, &arg->data.ipv4) <= 0)
goto parse_err;
break;
case ARGT_MSK4:
- if (in == beg) // empty mask
+ if (!trash.data) // empty mask
goto empty_err;
- if (!str2mask(word, &arg->data.ipv4))
+ if (!str2mask(trash.area, &arg->data.ipv4))
goto parse_err;
arg->type = ARGT_IPV4;
break;
case ARGT_IPV6:
- if (in == beg) // empty address
+ if (!trash.data) // empty address
goto empty_err;
- if (inet_pton(AF_INET6, word, &arg->data.ipv6) <= 0)
+ if (inet_pton(AF_INET6, trash.area, &arg->data.ipv6) <= 0)
goto parse_err;
break;
case ARGT_MSK6:
- if (in == beg) // empty mask
+ if (!trash.data) // empty mask
goto empty_err;
- if (!str2mask6(word, &arg->data.ipv6))
+ if (!str2mask6(trash.area, &arg->data.ipv6))
goto parse_err;
arg->type = ARGT_IPV6;
break;
case ARGT_TIME:
- if (in == beg) // empty time
+ if (!trash.data) // empty time
goto empty_err;
- ptr_err = parse_time_err(word, &uint, TIME_UNIT_MS);
+ ptr_err = parse_time_err(trash.area, &uint, TIME_UNIT_MS);
if (ptr_err) {
if (ptr_err == PARSE_TIME_OVER || ptr_err == PARSE_TIME_UNDER)
- ptr_err = word;
+ ptr_err = trash.area;
goto parse_err;
}
arg->data.sint = uint;
@@ -253,10 +254,10 @@
break;
case ARGT_SIZE:
- if (in == beg) // empty size
+ if (!trash.data) // empty size
goto empty_err;
- ptr_err = parse_size_err(word, &uint);
+ ptr_err = parse_size_err(trash.area, &uint);
if (ptr_err)
goto parse_err;
@@ -265,10 +266,10 @@
break;
case ARGT_PBUF_FNUM:
- if (in == beg)
+ if (!trash.data)
goto empty_err;
- if (!parse_dotted_uints(word, &arg->data.fid.ids, &arg->data.fid.sz))
+ if (!parse_dotted_uints(trash.area, &arg->data.fid.ids, &arg->data.fid.sz))
goto parse_err;
break;
@@ -290,8 +291,6 @@
}
end_parse:
- free(word); word = NULL;
-
if (pos < min_arg) {
/* not enough arguments */
memprintf(err_msg,
@@ -307,11 +306,10 @@
in++;
} else {
/* the caller is responsible for freeing this message */
- word = (len > 0) ? my_strndup(in, len) : (char *)in;
+ char *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.
@@ -331,7 +329,6 @@
return pos;
err:
- free(word);
if (new_al == al) {
/* only free the arg area if we have not queued unresolved args
* still pointing to it.
@@ -351,12 +348,18 @@
goto err;
parse_err:
+ /* come here with the word attempted to parse in trash */
memprintf(err_msg, "failed to parse '%s' as type '%s' at position %d",
- word, arg_type_names[(mask >> (pos * ARGT_BITS)) & ARGT_MASK], pos + 1);
+ trash.area, arg_type_names[(mask >> (pos * ARGT_BITS)) & ARGT_MASK], pos + 1);
goto err;
not_impl:
memprintf(err_msg, "parsing for type '%s' was not implemented, please report this bug",
arg_type_names[(mask >> (pos * ARGT_BITS)) & ARGT_MASK]);
goto err;
+
+ buffer_err:
+ memprintf(err_msg, "too small buffer size to store decoded argument %d, increase bufsize ?",
+ pos + 1);
+ goto err;
}