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.

On 4/12/21 10:09 PM, Aleksandar Lazic wrote:
This should write the double value to the string but I think I have here some
issue.

I've responded inline to that.

Patch feedback:

From 8cb1bc4aaedd17c7189d4985a57f662ab1b533a4 Mon Sep 17 00:00:00 2001
From: Aleksandar Lazic <al-hapr...@none.at>
Date: Mon, 12 Apr 2021 22:01:04 +0200
Subject: [PATCH] MINOR: sample: converter: add JSON Path handling

With json_path can a JSON value be extacted from a Header
or body
---
 Makefile                           |    3 +-
 doc/configuration.txt              |   29 +
 include/import/mjson.h             |  209 ++++++
 reg-tests/converter/json_query.vtc |   94 +++
 src/mjson.c                        | 1048 ++++++++++++++++++++++++++++
 src/sample.c                       |   94 +++
 6 files changed, 1476 insertions(+), 1 deletion(-)
 create mode 100644 include/import/mjson.h
 create mode 100644 reg-tests/converter/json_query.vtc
 create mode 100644 src/mjson.c

diff --git a/Makefile b/Makefile
index 9b22fe4be..56d0aa28d 100644
--- a/Makefile
+++ b/Makefile
@@ -883,7 +883,8 @@ OBJS += src/mux_h2.o src/mux_fcgi.o src/http_ana.o 
src/stream.o                \
         src/ebistree.o src/auth.o src/wdt.o src/http_acl.o                     
\
         src/hpack-enc.o src/hpack-huff.o src/ebtree.o src/base64.o             
\
         src/hash.o src/dgram.o src/version.o src/fix.o src/mqtt.o src/dns.o    
\
-        src/server_state.o src/proto_uxdg.o src/init.o src/cfgdiag.o
+        src/server_state.o src/proto_uxdg.o src/init.o src/cfgdiag.o           
\
+               src/mjson.o

Incorrect indentation here.

 ifneq ($(TRACE),)
 OBJS += src/calltrace.o
diff --git a/doc/configuration.txt b/doc/configuration.txt
index f21a29a68..4393d5c1f 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15958,6 +15958,35 @@ json([<input-code>])
   Output log:
      {"ip":"127.0.0.1","user-agent":"Very \"Ugly\" UA 1\/2"}
+json_query(<json_path>,[<output_type>])
+  The <json_path> is mandatory.

This is redundant. It is implied by the method signature (no square brackets around json_path).

+  By default will the follwing JSON types recognized.
+   - string  : This is the default search type and returns a string;
+ - number : When the JSON value is a number then will the value be + converted to a string. If you know that the value is a + integer then can you help haproxy to convert the value + to a integer when you add "sint" to the <output_type>;
+   - boolean : If the JSON value is not a String
+
+  This converter uses the mjson library https://github.com/cesanta/mjson
+ This converter extracts the value located at <json_path> from the JSON + string in the input value. + <json_path> must be a valid JsonPath string as defined at + https://goessner.net/articles/JsonPath/
+
+  A floating point value will always be returned as string!
+ + Example:
+     # get the value of the key kubernetes.io/serviceaccount/namespace
+     # => openshift-logging
+     http-request set-var(sess.json) 
req.hdr(Authorization),b64dec,json_string('$.kubernetes\\.io/serviceaccount/namespace')
+ + # get the value of the key 'iss' from a JWT
+     # => kubernetes/serviceaccount
+     http-request set-var(sess.json) 
req.hdr(Authorization),b64dec,json_string('$.iss')
+
+
+
 language(<value>[,<default>])
   Returns the value with the highest q-factor from a list as extracted from the
   "accept-language" header using "req.fhdr". Values with no q-factor have a
diff --git a/reg-tests/converter/json_query.vtc 
b/reg-tests/converter/json_query.vtc
new file mode 100644
index 000000000..b7c0e2c3a
--- /dev/null
+++ b/reg-tests/converter/json_query.vtc
@@ -0,0 +1,94 @@
+varnishtest "JSON Query converters Test"
+#REQUIRE_VERSION=2.4
+
+feature ignore_unknown_macro
+
+server s1 {
+       rxreq
+       txresp
+
+    rxreq
+    expect req.url == "/"
+    txresp  -body "{\"iss\":\"kubernetes/serviceaccount\"}"
+
+    rxreq
+    expect req.url == "/"
+    txresp  -body "{\"integer\":4}"
+
+    rxreq
+       txresp -body "{\"boolean-true\":true}"
+
+    rxreq
+       txresp -body "{\"boolean-false\":false}"
+
+    rxreq
+       txresp -body "{\"my.key\":\"myvalue\"}"

Incorrect indentation above. You are mixing tabs and spaces.

+} -start
+
+haproxy h1 -conf {
+    defaults
+       mode http
+       timeout connect 1s
+       timeout client  1s
+       timeout server  1s
+
+    frontend fe
+       bind "fd@${fe}"
+
+    http-request set-var(sess.header_json) 
req.hdr(Authorization),json_string('$.iss')
+    http-request set-var(sess.pay_json) req.body,json_string('$.iss')
+    http-request set-var(sess.pay_int) 
req.body,json_string('$.integer',"sint"),add(1)
+    http-request set-var(sess.pay_boolean_true) 
req.body,json_string('$.boolean-true')
+    http-request set-var(sess.pay_boolean_false) 
req.body,json_string('$.boolean-false')
+    http-request set-var(sess.pay_mykey) req.body,json_string('$.my\\.key')
+
+    http-response set-header x-var_header %[var(sess.header_json)]
+    http-response set-header x-var_body %[var(sess.pay_json)]
+    http-response set-header x-var_body_int %[var(sess.pay_int)]
+    http-response set-header x-var_body_boolean_true 
%[var(sess.pay_boolean_true)]
+    http-response set-header x-var_body_boolean_false 
%[var(sess.pay_boolean_false)]
+    http-response set-header x-var_body_mykey %[var(sess.pay_mykey)]
+ + default_backend be
+
+    backend be
+       server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+       txreq -url "/" \
+      -hdr "Authorization: {\"iss\":\"kubernetes.io/serviceaccount\"}"
+       rxresp
+    expect resp.status == 200
+    expect resp.http.x-var_header ~ "kubernetes.io/serviceaccount"
+
+    txreq -url "/" \
+      -body "{\"iss\":\"kubernetes.io/serviceaccount\"}"
+       rxresp
+    expect resp.status == 200
+    expect resp.http.x-var_body ~ "kubernetes.io/serviceaccount"
+
+    txreq -url "/" \
+      -body "{\"integer\":4}"
+       rxresp
+    expect resp.status == 200
+    expect resp.http.x-var_body_int ~ "5"
+ + txreq -url "/" \
+      -body "{\"boolean-true\":true}"
+       rxresp
+    expect resp.status == 200
+    expect resp.http.x-var_body_boolean_true == 1
+
+    txreq -url "/" \
+      -body "{\"boolean-false\":false}"
+       rxresp
+    expect resp.status == 200
+    expect resp.http.x-var_body_boolean_false == 0
+
+    txreq -url "/" \
+      -body "{\"my.key\":\"myvalue\"}"
+       rxresp
+    expect resp.status == 200
+    expect resp.http.x-var_body_mykey ~ "myvalue"
+} -run
diff --git a/src/sample.c b/src/sample.c
index 835a18115..d6cb6379d 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -3653,6 +3654,97 @@ static int sample_conv_rtrim(const struct arg *arg_p, 
struct sample *smp, void *
        return 1;
 }
+/* This function checks the "json_query" converter's arguments. */
+static int sample_check_json_query(struct arg *arg, struct sample_conv *conv,
+                           const char *file, int line, char **err)
+{
+       int result;

This variable is useless.

+
+       if (arg[0].data.str.data == 0) { /* empty */
+               memprintf(err, "json_path must not be empty");
+               return 0;
+       }
+
+       if (arg[1].data.str.data != 0) {
+                       result = strncmp(arg[1].data.str.area, "sint", 
sizeof("sint"));

a) Incorrect indentation here (remove 1 Tab).
b) Simply using `strcmp(arg[1].data.str.area, "sint")` should be safe here.
c) I believe you should convert the argument's type to a simple integer for performance reasons (no need to compare the string when executing the converter). Willy probably give more detailed advice here.

+                       if (result != 0) {
+                               memprintf(err, "output_type only supports \"sint\" 
as argument");
+                               return 0;
+                       }
+       }
+       return 1;
+}
+
+/* This sample function get the value from a given json string.
+ * The mjson library is used to parse the json struct
+ */
+static int sample_conv_json_query(const struct arg *args, struct sample *smp, 
void *private)
+{
+       struct buffer *trash = get_trash_chunk();
+       int rc;            /* holds the return value of mjson* functions */
+       int bool;          /* holds the value of mjson_get_bool */
+       int result;        /* holds the return value of strncmp */

This one will become obsolete if you adjust this as suggested above.

+       double double_val; /* holds the value of mjson_get_number */
+
+       /* if the second argument is "sint" then are the other checks not 
necessary. */
+       if (args[1].data.str.data != 0) {
+                       result = strncmp(args[1].data.str.area, "sint", 
sizeof("sint"));

Incorrect indentation (Remove 1 Tab).

+                       if (result != 0) {
+                               /* "output_type only supports \"sint\" as 
argument"); */
+                               return 0;
+                       } else {
+                               rc = mjson_get_number(smp->data.u.str.area, 
smp->data.u.str.data, args[0].data.str.area, &double_val);
+                               if (rc == 0) {
+                                       /* mjson does not recognized a number */
+                                       return 0;
+                               } else {
+                                       smp->data.type = SMP_T_SINT;
+                                       smp->data.u.sint = (unsigned long 
long)double_val;
+                                       return 1;
+                               }
+                               
+                       }
+       }
+       
+       /* No output_type was given try to guess the type */
+
+       rc = mjson_get_string(smp->data.u.str.area, smp->data.u.str.data, 
args[0].data.str.area, trash->area, trash->size);
+
+       if (rc == -1 ) {
+
+               rc = mjson_get_number(smp->data.u.str.area, smp->data.u.str.data, 
args[0].data.str.area, &double_val);
+
+               if (rc == 0) {
+                       
+                       rc = mjson_get_bool(smp->data.u.str.area, 
smp->data.u.str.data, args[0].data.str.area,&bool);

Missing space after the last comma.

+                       if (rc == 1 && bool == 1) {
+                               smp->data.type = SMP_T_BOOL;
+                               smp->data.u.sint = 1;
+                       } else if(rc == 1 && bool == 0) {
+                               smp->data.type = SMP_T_BOOL;
+                               smp->data.u.sint = 0;

I believe this can be simplified to:

if (rc == 1) {
 smp->data.type = SMP_T_BOOL;
 smp->data.u.sint = bool;
}

+                       } else {
+                               return 0;
+                       }
+               } else {
+                       printf("\n>>>DOUBLE rc:%d: double:%f:\n",rc, 
double_val);
+                       trash->size = snprintf(trash->area,
+ trash->data, + "%g",double_val);

I believe you mixed up trash->size und trash->data here.

+                       smp->data.u.str = *trash;
+                       smp->data.type = SMP_T_STR;
+               }               
+       } else {
+               trash->data = rc;
+               smp->data.u.str = *trash;
+               smp->data.type = SMP_T_STR;
+       }
+
+       return 1;
+}
+
+
 /************************************************************************/
 /*       All supported sample fetch functions must be declared here     */
 /************************************************************************/
@@ -4165,6 +4257,8 @@ static struct sample_conv_kw_list sample_conv_kws = {ILH, 
{
        { "cut_crlf", sample_conv_cut_crlf,           0, NULL, SMP_T_STR,  
SMP_T_STR  },
        { "ltrim",    sample_conv_ltrim,    ARG1(1,STR), NULL, SMP_T_STR,  
SMP_T_STR  },
        { "rtrim",    sample_conv_rtrim,    ARG1(1,STR), NULL, SMP_T_STR,  
SMP_T_STR  },
+    { "json_string", sample_conv_json_query, ARG2(1,STR,STR),  
sample_check_json_query , SMP_T_STR, SMP_T_ANY },

a) Incorrect indentation.
b) The converter still is called "json_string". You forgot to update the name.

+
        { NULL, NULL, 0, 0, 0 },
 }};
--
2.25.1

Best regards
Tim Düsterhus

Reply via email to