Re: [PATCH] MINOR: sample: add json_string

2021-04-20 Thread Tim Düsterhus
Willy, On 4/20/21 8:35 PM, Willy Tarreau wrote: But who am I to complain? Updated patches attached. I left the 'trash' one as-is, because that one really is a common pattern. I hope I did not miss anything else :-) I believe you might've missed my updated patches. Oops you're right, I was a

Re: [PATCH] MINOR: sample: add json_string

2021-04-20 Thread Willy Tarreau
Hi Tim! On Tue, Apr 20, 2021 at 08:29:30PM +0200, Tim Düsterhus wrote: > Willy, > > On 4/16/21 8:30 PM, Tim Düsterhus wrote: > > But who am I to complain? Updated patches attached. I left the 'trash' > > one as-is, because that one really is a common pattern. I hope I did not > > miss anything

Re: [PATCH] MINOR: sample: add json_string

2021-04-20 Thread Tim Düsterhus
Willy, On 4/16/21 8:30 PM, Tim Düsterhus wrote: But who am I to complain? Updated patches attached. I left the 'trash' one as-is, because that one really is a common pattern. I hope I did not miss anything else :-) I believe you might've missed my updated patches. Best regards Tim Düsterhus

Re: [PATCH] MINOR: sample: add json_string

2021-04-16 Thread Tim Düsterhus
Willy, On 4/16/21 8:24 AM, Willy Tarreau wrote: - tok = mjson_find(smp->data.u.str.area, smp->data.u.str.data, args[0].data.str.area, , ); + enum mjson_tok token_type = mjson_find(smp->data.u.str.area, smp->data.u.str.data, args[0].data.str.area, , _size); As much as possible,

Re: [PATCH] MINOR: sample: add json_string

2021-04-16 Thread Willy Tarreau
On Thu, Apr 15, 2021 at 06:47:29PM +0200, Tim Düsterhus wrote: > You know that I'm obsessed with the correct use of data types, so please > find three CLEANUP patches attached. Feel free to drop the ones you don't > agree with :-) I don't disagree with them, but am just having a few comments on

Re: [PATCH] MINOR: sample: add json_string

2021-04-16 Thread Willy Tarreau
On Thu, Apr 15, 2021 at 05:44:44PM +0200, Aleksandar Lazic wrote: > Now the Statement what I wanted to say ;-) > > HAProxy have now at least 4 possibilities to route traffic and > catch some data. > > HTTP fields > GRPC fields > FIX fields > JSON fields > > Have I missed something? You can

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Tim Düsterhus
Willy, On 4/15/21 5:09 PM, Willy Tarreau wrote: On Thu, Apr 15, 2021 at 04:49:00PM +0200, Aleksandar Lazic wrote: #define JSON_INT_MAX ((1ULL << 53) - 1) ^ Sorry I was not clear, please drop that 'U' here. I'm also sorry, I was in a tunnel :-/ Attached now the

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Aleksandar Lazic
On 15.04.21 17:09, Willy Tarreau wrote: On Thu, Apr 15, 2021 at 04:49:00PM +0200, Aleksandar Lazic wrote: #define JSON_INT_MAX ((1ULL << 53) - 1) ^ Sorry I was not clear, please drop that 'U' here. I'm also sorry, I was in a tunnel :-/ Attached now the next

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Willy Tarreau
On Thu, Apr 15, 2021 at 04:49:00PM +0200, Aleksandar Lazic wrote: > > > #define JSON_INT_MAX ((1ULL << 53) - 1) > >^ > > Sorry I was not clear, please drop that 'U' here. > > I'm also sorry, I was in a tunnel :-/ > > Attached now the next patches. Thank you! Now

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Aleksandar Lazic
On 15.04.21 16:09, Willy Tarreau wrote: On Thu, Apr 15, 2021 at 04:05:27PM +0200, Aleksandar Lazic wrote: Well I don't think so because 4 is still bigger then -9007199254740991 ;-) This is because *you* think it is -9007199254740991 but the reality is that it's not this.due to ULL:

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Willy Tarreau
On Thu, Apr 15, 2021 at 04:05:27PM +0200, Aleksandar Lazic wrote: > Well I don't think so because 4 is still bigger then -9007199254740991 ;-) This is because *you* think it is -9007199254740991 but the reality is that it's not this.due to ULL: #define JSON_INT_MAX ((1ULL << 53) - 1) #define

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Aleksandar Lazic
On 15.04.21 15:55, Willy Tarreau wrote: On Thu, Apr 15, 2021 at 03:41:18PM +0200, Aleksandar Lazic wrote: Now when I remove the check "smp->data.u.sint < 0" every positive value is bigger then JSON INT_MIN and returns 0. But don't you agree that this test DOES nothing ? If it changes anything

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Willy Tarreau
On Thu, Apr 15, 2021 at 03:41:18PM +0200, Aleksandar Lazic wrote: > > > Now when I remove the check "smp->data.u.sint < 0" every positive value is > > > bigger then JSON INT_MIN and returns 0. > > > > But don't you agree that this test DOES nothing ? If it changes anything > > it means the issue

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Aleksandar Lazic
On 15.04.21 14:48, Willy Tarreau wrote: On Thu, Apr 15, 2021 at 02:17:45PM +0200, Aleksandar Lazic wrote: I, by far, prefer Tim's proposal here, as I do not even understand the first one, sorry Aleks, please don't feel offended :-) Well you know my focus is to support HAProxy and therefore

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Willy Tarreau
On Thu, Apr 15, 2021 at 02:17:45PM +0200, Aleksandar Lazic wrote: > > I, by far, prefer Tim's proposal here, as I do not even understand the > > first one, sorry Aleks, please don't feel offended :-) > > Well you know my focus is to support HAProxy and therefore it's okay. > The contribution was

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Aleksandar Lazic
On 15.04.21 09:08, Willy Tarreau wrote: On Wed, Apr 14, 2021 at 09:52:31PM +0200, Aleksandar Lazic wrote: +   - string  : This is the default search type and returns a String; +   - boolean : If the JSON value is not a String or a Number +   - number  : When the JSON value is a Number then will

Re: [PATCH] MINOR: sample: add json_string

2021-04-15 Thread Willy Tarreau
On Wed, Apr 14, 2021 at 09:52:31PM +0200, Aleksandar Lazic wrote: > > > +   - string  : This is the default search type and returns a String; > > > +   - boolean : If the JSON value is not a String or a Number > > > +   - number  : When the JSON value is a Number then will the value be > > > + 

Re: [PATCH] MINOR: sample: add json_string

2021-04-14 Thread Aleksandar Lazic
On 14.04.21 18:41, Tim Düsterhus wrote: Aleks, On 4/14/21 1:19 PM, Aleksandar Lazic wrote: From 46ddac8379324b645c662e19de39d5de4ac74a77 Mon Sep 17 00:00:00 2001 From: Aleksandar Lazic Date: Wed, 14 Apr 2021 13:11:26 +0200 Subject: [PATCH 2/2] MINOR: sample: converter: Add json_query

Re: [PATCH] MINOR: sample: add json_string

2021-04-14 Thread Tim Düsterhus
Aleks, On 4/14/21 1:19 PM, Aleksandar Lazic wrote: From 46ddac8379324b645c662e19de39d5de4ac74a77 Mon Sep 17 00:00:00 2001 From: Aleksandar Lazic Date: Wed, 14 Apr 2021 13:11:26 +0200 Subject: [PATCH 2/2] MINOR: sample: converter: Add json_query converter With the json_query can a JSON value

Re: [PATCH] MINOR: sample: add json_string

2021-04-14 Thread Aleksandar Lazic
Hi. here now the current version of the patches. Regards Aleks. On 14.04.21 10:45, Aleksandar Lazic wrote: On 14.04.21 04:36, Willy Tarreau wrote: On Wed, Apr 14, 2021 at 03:02:20AM +0200, Aleksandar Lazic wrote: But then, could it make sense to also support "strict integers": values that

Re: [PATCH] MINOR: sample: add json_string

2021-04-14 Thread Aleksandar Lazic
On 14.04.21 04:36, Willy Tarreau wrote: On Wed, Apr 14, 2021 at 03:02:20AM +0200, Aleksandar Lazic wrote: But then, could it make sense to also support "strict integers": values that can accurately be represented as integers and which are within the JSON valid range for integers (-2^52 to 2^52

Re: [PATCH] MINOR: sample: add json_string

2021-04-14 Thread Willy Tarreau
On Wed, Apr 14, 2021 at 09:05:53AM +0200, Tim Düsterhus wrote: > Willy, > > On 4/14/21 4:36 AM, Willy Tarreau wrote: > > > How about "json_query" because it's exactly what it does :-) > > > > I'm not familiar with the notion of "query" to decode and extract contents > > but I'm not the most

Re: [PATCH] MINOR: sample: add json_string

2021-04-14 Thread Tim Düsterhus
Willy, On 4/14/21 4:36 AM, Willy Tarreau wrote: How about "json_query" because it's exactly what it does :-) I'm not familiar with the notion of "query" to decode and extract contents but I'm not the most representative user and am aware of the "jq" command- line utility that does this. So if

Re: [PATCH] MINOR: sample: add json_string

2021-04-13 Thread Willy Tarreau
On Wed, Apr 14, 2021 at 03:02:20AM +0200, Aleksandar Lazic wrote: > > But then, could it make sense to also support "strict integers": values > > that can accurately be represented as integers and which are within the > > JSON valid range for integers (-2^52 to 2^52 with no decimal part) ? > >

Re: [PATCH] MINOR: sample: add json_string

2021-04-13 Thread Aleksandar Lazic
On 13.04.21 11:26, Willy Tarreau wrote: Hi Aleks, On Mon, Apr 12, 2021 at 10:09:08PM +0200, Aleksandar Lazic wrote: Hi. another patch which honer the feedback. Thank you. FWIW I agree with all the points reported by Tim. I'll add a few comments and/or suggestions below. On a general note,

Re: [PATCH] MINOR: sample: add json_string

2021-04-13 Thread Willy Tarreau
Hi Aleks, On Mon, Apr 12, 2021 at 10:09:08PM +0200, Aleksandar Lazic wrote: > Hi. > > another patch which honer the feedback. Thank you. FWIW I agree with all the points reported by Tim. I'll add a few comments and/or suggestions below. On a general note, please be careful about your indenting,

Re: [PATCH] MINOR: sample: add json_string

2021-04-12 Thread Willy Tarreau
On Mon, Apr 12, 2021 at 10:36:30PM +0200, Tim Düsterhus wrote: > Aleks, > > [Willy: I believe the patch is in a state that warrants you taking a look at > it!] Yeah, I'll have a look today, thanks for your exchanges on this. Willy

Re: [PATCH] MINOR: sample: add json_string

2021-04-12 Thread Moemen MHEDHBI
On 08/04/2021 21:55, Aleksandar Lazic wrote: > Hi. > > Attached the patch to add the json_string sample. > > In combination with the JWT patch is a pre-validation of a bearer token > part possible. > > I have something like this in mind. > > http-request set-var(sess.json) >

Re: [PATCH] MINOR: sample: add json_string

2021-04-12 Thread Tim Düsterhus
Aleks, [Willy: I believe the patch is in a state that warrants you taking a look at it!] thank you. This looks *much* better now. I primarily have some style complaints. I'll probably also complain about the documentation a bit more, but you already said that you are still working on it.

Re: [PATCH] MINOR: sample: add json_string

2021-04-12 Thread Aleksandar Lazic
Hi. another patch which honer the feedback. The doc will be enhanced but I have a question about that sequence. This should write the double value to the string but I think I have here some issue. ``` printf("\n>>>DOUBLE rc:%d: double:%f:\n",rc, double_val);

Re: [PATCH] MINOR: sample: add json_string

2021-04-11 Thread Tim Düsterhus
Aleks, On 4/11/21 12:28 PM, Aleksandar Lazic wrote: Agree. I have now rethink how to do it and suggest to add a output type. ``` json_query(,)   The and are mandatory.   This converter uses the mjson library https://github.com/cesanta/mjson   This converter extracts the value located at

Re: [PATCH] MINOR: sample: add json_string

2021-04-11 Thread Aleksandar Lazic
On 10.04.21 13:22, Tim Düsterhus wrote: Aleks, On 4/10/21 12:24 AM, Aleksandar Lazic wrote: +json_string() : string I don't like the name. A few suggestions: - json_query - json_get - json_decode maybe json_get_string because there could be some more getter like bool, int, ... The

Re: [PATCH] MINOR: sample: add json_string

2021-04-10 Thread Tim Düsterhus
Aleks, On 4/10/21 12:24 AM, Aleksandar Lazic wrote: +json_string() : string I don't like the name. A few suggestions: - json_query - json_get - json_decode maybe json_get_string because there could be some more getter like bool, int, ... The '_string' suffix does not make sense to me,

Re: [PATCH] MINOR: sample: add json_string

2021-04-09 Thread Aleksandar Lazic
Tim. On 09.04.21 18:55, Tim Düsterhus wrote: Aleks, > I have taken a first look. Find my remarks below. Please note that for the actual > source code there might be further remarks by Willy (put in CC) or so. I might have > missed something or might have told you something incorrect. So

Re: [PATCH] MINOR: sample: add json_string

2021-04-09 Thread Tim Düsterhus
Aleks, I have taken a first look. Find my remarks below. Please note that for the actual source code there might be further remarks by Willy (put in CC) or so. I might have missed something or might have told you something incorrect. So maybe before making changes wait for their opinion.

Re: [PATCH] MINOR: sample: add json_string

2021-04-08 Thread Aleksandar Lazic
Hi. Sorry I have now seen the copy paste error. please use this patch Regards Alex On 08.04.21 21:55, Aleksandar Lazic wrote: Hi. Attached the patch to add the json_string sample. In combination with the JWT patch is a pre-validation of a bearer token part possible. I have something like