On Wed, Jun 08, 2022 at 01:56:20AM -0600, astrotha...@gmail.com wrote:
> +param(,[])
> + This extracts the first occurrence of the parameter in the input
> string
> + where parameters are delimited by , which defaults to "&", and the
> name
> + and value of the parameter are separated by a "=". If there is no "=" and
> value
> + before the end of the parameter segment, it is treated as equivalent to a
> value
> + of an empty string.
> +
> + This can be useful for extracting parameters from a query string, or
> possibly a
> + x-www-form-urlencoded body. In particular, `query,param()` can be
> used as
> + an alternative to `urlp()` which only uses "&" as a delimiter,
> whereas "urlp"
> + also uses "?" and ";".
> +
> + Note that this converter doesn't do anything special with url encoded
> characters. If
> + you want to decode the value, you can use the url_dec converter on the
> output. If
> + the name of the parameter in the input might contain encoded characters,
> you'll probably
Be careful to trim your long lines in the doc so that they don't overflow 80
characters.
> +static int sample_conv_param(const struct arg *arg_p, struct sample *smp,
> void *private)
> +{
> + char *pos, *end, *pend, *equal;
> + char delim = '&';
> + const char *name = arg_p[0].data.str.area;
> + size_t name_l = arg_p[0].data.str.data;
> +
> + if (arg_p[1].type == ARGT_STR)
> + delim = *arg_p[1].data.str.area;
> +
> + pos = smp->data.u.str.area;
> + end = pos + smp->data.u.str.data;
> + while (pos < end) {
> + equal = pos + name_l;
> + /* Parameter not found */
> + if (equal > end)
> + break;
> +
> + if (equal == end || *equal == '&') {
^^^
here it should be "delim", I guess it's a leftover from a previous version
> + if (memcmp(pos, name, name_l) == 0) {
> + /* input contains parameter, but no value is
> suplied */
^^^
supplied :-)
> + smp->data.u.str.data = 0;
> + return 1;
> + }
> + pos = equal + 1;
> + continue;
> + }
> +
> + if (*equal == '=' && memcmp(pos, name, name_l) == 0) {
> + pos = equal + 1;
> + pend = memchr(pos, delim, end - pos);
> + if (pend == NULL)
> + pend = end;
> + b_set_area_sub(&(smp->data.u.str), pos, pend - pos);
Ah, I didn't notice that b_set_area_sub() comes in a previous patch.
I'm not fond of this function, because it standardizes something that
should never work except in this exceptional case, and will inevitably
result in someone using it for the wrong thing. The point is that the
part of a buffer is allocated from pools normally and must not
be changed at all, since for regular use, there are start offset and
length that do the job. However here in samples we're still using buffers
to store strings because they're easy to allocate and are convenient
enough to use, but I would say it's just a convenience that's almost an
abuse. Most likely that the string part of samples could be turned to IST
instead. Thus I'd rather see both the ->area and ->data parts here being
manually assigned, it's more readable and doesn't standardize an exception.
I suspect it would simply become:
smp->data.u.str.area = pos;
smp->data.u.str.data = pend - pos;
And that's way more readable for those trying to rule out the possibility of
a bug here.
> /* Note: must not be declared as its list will be overwritten */
> static struct sample_conv_kw_list sample_conv_kws = {ILH, {
> - { "add_item",sample_conv_add_item, ARG3(2,STR,STR,STR),
> smp_check_add_item, SMP_T_STR, SMP_T_STR },
> - { "debug", sample_conv_debug,ARG2(0,STR,STR),
> smp_check_debug, SMP_T_ANY, SMP_T_ANY },
> - { "b64dec", sample_conv_base642bin, 0, NULL,
> SMP_T_STR, SMP_T_BIN },
> + { "add_item",sample_conv_add_item, ARG3(2,STR,STR,STR),
> smp_check_add_item, SMP_T_STR, SMP_T_STR }, { "debug",
> sample_conv_debug,ARG2(0,STR,STR), smp_check_debug,
> SMP_T_ANY, SMP_T_ANY }, { "b64dec", sample_conv_base642bin, 0,
> NULL, SMP_T_STR, SMP_T_BIN },
There's a mistake here, you accidently folded 3 lines. Probably "J" being
pressed there by accident in vim :-)
Otherwise it looks good. Please recheck, but I'm overall fine with merging
this.
Thank you!
Willy
