Re: Request for feedback: Add support for txn args as arguments in converter "bytes"

[Lokesh Jindal]

PFA the patch.

I have two questions:
1. Can you explain what this method does and why is it needed? (I used it
in my patch following the pattern in the converter "sub") -
*smp_set_owner(&smp_arg0,
smp->px, smp->sess, smp->strm, smp->opt);*
2. In my patch, *sample_conv_bytes* returns 0 in case of an invalid arg
e.g. negative offset. Can you explain when a converter should return 0 V 1?
I am not sure how that impacts the haproxy behavior.

On Mon, Aug 28, 2023 at 9:56 AM Lokesh Jindal <15ljin...@gmail.com> wrote:

> Thanks for the response and the corrections, Willy.
>
> *We need to decide what to do when the variable does not*
>
> *exist or is empty. We can't make converters fail for now, so most
> likelyit will have to end up as value zero for offset and/or length*.
>
> Here is the implementation today - link
> .
> We set the length of the output to 0 in case of an invalid input (e.g. arg0
> value >= length of the bytes in the input to the converter).
> So, for all other invalid inputs (e.g. variable in arg[0] does not exist),
> we can do the same.
>
> We can discuss more after I share the patch.
>
> Thanks
> Lokesh
>
> On Mon, Aug 28, 2023 at 2:53 AM Willy Tarreau  wrote:
>
>> Hi Lokesh,
>>
>> On Fri, Aug 25, 2023 at 01:44:48PM -0700, Lokesh Jindal wrote:
>> > Hey folks
>> >
>> > I am writing to gather feedback on an idea before doing the
>> implementation.
>> > Per the documentation, converter "bytes" accepts integer values as
>> > arguments, but not txn args.
>> > i.e. ,bytes(2,8) will work
>> > but ,bytes(txn.start_idx,txn.length) will not work.
>> >
>> > For our use case, we need to parse some binary data (a cookie) to
>> extract
>> > some info in haproxy. However, the bytes that need to be extracted are
>> not
>> > fixed and will depend on the request. We could use simple arithmetic to
>> > determine the index/length for bytes to be extracted and store them in
>> txn
>> > args. These txn args can then be used with converter "bytes".
>> >
>> > I can see that the converter "sub" already supports txn args as
>> arguments.
>> > I have successfully validated the proposed idea with an implementation
>> > following the same pattern as "sub".
>>
>> In fact it's not "txn", it's variables in general. Most of the arithmetic
>> functions support a variable name as an alternative to a numeric value. I
>> tend to think it would indeed make sense to support both integers and
>> variables for bytes(), it could even be the same for a few other ones
>> (maybe field(), word(), ltrim(), rtrim() etc).
>>
>> > Let me know what you think. If there are no concerns, I can send a
>> patch.
>>
>> I'm all for it. We need to decide what to do when the variable does not
>> exist or is empty. We can't make converters fail for now, so most likely
>> it will have to end up as value zero for offset and/or length.
>>
>> Thanks,
>> Willy
>>
>


0001-MEDIUM-sample-Enhances-converter-bytes-to-take-varia.patch
Description: Binary data

Re: Request for feedback: Add support for txn args as arguments in converter "bytes"

[Lokesh Jindal]

Thanks for the response and the corrections, Willy.

*We need to decide what to do when the variable does not*

*exist or is empty. We can't make converters fail for now, so most likelyit
will have to end up as value zero for offset and/or length*.

Here is the implementation today - link
.
We set the length of the output to 0 in case of an invalid input (e.g. arg0
value >= length of the bytes in the input to the converter).
So, for all other invalid inputs (e.g. variable in arg[0] does not exist),
we can do the same.

We can discuss more after I share the patch.

Thanks
Lokesh

On Mon, Aug 28, 2023 at 2:53 AM Willy Tarreau  wrote:

> Hi Lokesh,
>
> On Fri, Aug 25, 2023 at 01:44:48PM -0700, Lokesh Jindal wrote:
> > Hey folks
> >
> > I am writing to gather feedback on an idea before doing the
> implementation.
> > Per the documentation, converter "bytes" accepts integer values as
> > arguments, but not txn args.
> > i.e. ,bytes(2,8) will work
> > but ,bytes(txn.start_idx,txn.length) will not work.
> >
> > For our use case, we need to parse some binary data (a cookie) to extract
> > some info in haproxy. However, the bytes that need to be extracted are
> not
> > fixed and will depend on the request. We could use simple arithmetic to
> > determine the index/length for bytes to be extracted and store them in
> txn
> > args. These txn args can then be used with converter "bytes".
> >
> > I can see that the converter "sub" already supports txn args as
> arguments.
> > I have successfully validated the proposed idea with an implementation
> > following the same pattern as "sub".
>
> In fact it's not "txn", it's variables in general. Most of the arithmetic
> functions support a variable name as an alternative to a numeric value. I
> tend to think it would indeed make sense to support both integers and
> variables for bytes(), it could even be the same for a few other ones
> (maybe field(), word(), ltrim(), rtrim() etc).
>
> > Let me know what you think. If there are no concerns, I can send a patch.
>
> I'm all for it. We need to decide what to do when the variable does not
> exist or is empty. We can't make converters fail for now, so most likely
> it will have to end up as value zero for offset and/or length.
>
> Thanks,
> Willy
>

Re: Request for feedback: Add support for txn args as arguments in converter "bytes"

[Willy Tarreau]

Hi Lokesh,

On Fri, Aug 25, 2023 at 01:44:48PM -0700, Lokesh Jindal wrote:
> Hey folks
> 
> I am writing to gather feedback on an idea before doing the implementation.
> Per the documentation, converter "bytes" accepts integer values as
> arguments, but not txn args.
> i.e. ,bytes(2,8) will work
> but ,bytes(txn.start_idx,txn.length) will not work.
> 
> For our use case, we need to parse some binary data (a cookie) to extract
> some info in haproxy. However, the bytes that need to be extracted are not
> fixed and will depend on the request. We could use simple arithmetic to
> determine the index/length for bytes to be extracted and store them in txn
> args. These txn args can then be used with converter "bytes".
> 
> I can see that the converter "sub" already supports txn args as arguments.
> I have successfully validated the proposed idea with an implementation
> following the same pattern as "sub".

In fact it's not "txn", it's variables in general. Most of the arithmetic
functions support a variable name as an alternative to a numeric value. I
tend to think it would indeed make sense to support both integers and
variables for bytes(), it could even be the same for a few other ones
(maybe field(), word(), ltrim(), rtrim() etc).

> Let me know what you think. If there are no concerns, I can send a patch.

I'm all for it. We need to decide what to do when the variable does not
exist or is empty. We can't make converters fail for now, so most likely
it will have to end up as value zero for offset and/or length.

Thanks,
Willy

Request for feedback: Add support for txn args as arguments in converter "bytes"

[Lokesh Jindal]

Hey folks

I am writing to gather feedback on an idea before doing the implementation.
Per the documentation, converter "bytes" accepts integer values as
arguments, but not txn args.
i.e. ,bytes(2,8) will work
but ,bytes(txn.start_idx,txn.length) will not work.

For our use case, we need to parse some binary data (a cookie) to extract
some info in haproxy. However, the bytes that need to be extracted are not
fixed and will depend on the request. We could use simple arithmetic to
determine the index/length for bytes to be extracted and store them in txn
args. These txn args can then be used with converter "bytes".

I can see that the converter "sub" already supports txn args as arguments.
I have successfully validated the proposed idea with an implementation
following the same pattern as "sub".

Let me know what you think. If there are no concerns, I can send a patch.

Thanks
Lokesh