Re: [PATCH] MINOR: sample: add json_string
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 converter
With the json_query can a JSON value be extacted from a Header
or body of the request and saved to a variable.
This converter makes it possible to handle some JSON Workload
to route requests to differnt backends.
Typo: different.
---
doc/configuration.txt | 32
reg-tests/converter/json_query.vtc | 116 +
src/sample.c | 95 +++
3 files changed, 243 insertions(+)
create mode 100644 reg-tests/converter/json_query.vtc
diff --git a/doc/configuration.txt b/doc/configuration.txt
index f242300e7..374e7939b 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15961,6 +15961,38 @@ json([])
Output log:
{"ip":"127.0.0.1","user-agent":"Very \"Ugly\" UA 1\/2"}
+
This empty line should not be there.
+json_query(,[])
+ This converter searches for the key given by and returns
+ the value.
+ must be a valid JSONPath String as defined in
I'd use string in lowercase.
+ https://datatracker.ietf.org/doc/draft-ietf-jsonpath-base/
+
+ A floating point value will always be returned as String.
+
+ The follwing JSON types are recognized.
Typo: following.
I'd also use a ':' instead of '.'.
+ - 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
+ converted to a String. If its known that the value is a
+ integer then add 'int' to the which helps
+ haproxy to convert the value to a integer for further usage;
I'd probably completely rephrase this as:
The json_query converter supports the JSON types string, boolean and number.
Floating point numbers will be returned as a string. By specifying the
output_type 'int' the value will be converted to an Integer. If conversion is
not possible the json_query converter fails.
Well I would like to here also some other opinions about the wording.
+ Example:
+ # get the value of the key 'iss' from a JWT Bearer token
+ http-request set-var(txn.token_payload)
req.hdr(Authorization),word(2,.),ub64dec,json_query('$.iss')
+
+ # get a integer value from the request body
+ # "{"integer":4}" => 5
+ http-request set-var(txn.pay_int)
req.body,json_query('$.integer','int'),add(1)
+
+ # get a key with '.' in the name
+ # {"my.key":"myvalue"} => myvalue
+ http-request set-var(txn.pay_mykey) req.body,json_query('$.my\\.key')
+
+ # {"boolean-false":false} => 0
+ http-request set-var(txn.pay_boolean_false)
req.body,json_query('$.boolean-false')
These examples look good to me. I'd just move the JWT example to the bottom, so
that the simple examples come first.
I prefer to keep it like this.
language([,])
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 0..88ef58a0c
--- /dev/null
+++ b/reg-tests/converter/json_query.vtc
@@ -0,0 +1,116 @@
+varnishtest "JSON Query converters Test"
+#REQUIRE_VERSION=2.4
+
+feature ignore_unknown_macro
+
+server s1 {
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+ + rxreq
+ txresp
+ + rxreq
+ txresp
+} -start
You can use '-repeat 8' to simplify the server definition.
Good hint, thanks.
+haproxy h1 -conf {
+ defaults
+ mode http
+ timeout connect 1s
+ timeout client 1s
+ timeout server 1s
+ option http-buffer-request
+
+ frontend fe
+ bind "fd@${fe}"
+ tcp-request inspect-delay 1s
+
+ http-request set-var(sess.header_json)
req.hdr(Authorization),json_query('$.iss')
+ http-request set-var(sess.pay_json) req.body,json_query('$.iss')
+ http-request set-var(sess.pay_int)
req.body,json_query('$.integer',"int"),add(1)
+ http-request set-var(sess.pay_neg_int)
req.body,json_query('$.negativ-integer',"int"),add(1)
Inconsistent indentation here.
+ http-request set-var(sess.pay_double) req.body,json_query('$.double')
+ http-request set-var(sess.pay_boolean_true)
req.body,json_query('$.boolean-true')
+ http-request set-var(sess.pay_boolean_false)
req.body,json_query('$.boolean-false')
+ http-request set-var(sess.pay_mykey) req.body,json_query('$.my\\.key')
+
+ http-response set-header x-var_header %[var(sess.header_json)]
+ http-response set-header x-var_body %[var(sess.pay_json)]
+
Re: [PATCH] MINOR: ist: Add `istclear(struct ist*)`
On Wed, Apr 14, 2021 at 07:14:30PM +0200, Tim Duesterhus wrote: > > You can pass a pointer on dst. In the normalizer, you can update its > > size. It is thus possible to use dst when calling > > http_replace_req_path() or http_replace_req_query(). > > > > I see, that makes sense to me. I thought of the size being the current length > of the ist and did not think of simply resetting it to 0 before starting the > processing. > > Here's a preparation patch to the ist library, allowing this pattern to be > cleanly implemented. I've already incorporated it into my local normalization > branch, but I'd like to implement the remaining feedback before sending a new > series for it. I like it. I thought we already had something to reset an ist that we could repurpose, but no, right now resets are done by assigning so there was nothing to do that yet. Now applied, thanks! Willy
Re: [PATCH] fix cirrus-ci builds by installing "pcre" package
Willy, On 4/14/21 7:00 PM, Илья Шипицин wrote: something changed in freebsd packaging. we now need to install pcre directly. This one looks good to me. Best regards Tim Düsterhus
[PATCH] MINOR: ist: Add `istclear(struct ist*)`
Christopher,
Willy,
On 4/13/21 6:34 PM, Christopher Faulet wrote:
>> Thus I can't simply take a `struct ist*` for the destination, as an ist
>> cannot communicate the size of the underlying buffer. I could
>> technically take a `struct buffer`, but I'd still like the result to
>> reside in an ist, because this is what the HTX API expects.
>
> Hum, I don't understand. If you create an ist using the trash buffer
> this way:
>
> struct ist dst = ist2(replace->area, replace->size);
>
> You can pass a pointer on dst. In the normalizer, you can update its
> size. It is thus possible to use dst when calling
> http_replace_req_path() or http_replace_req_query().
>
I see, that makes sense to me. I thought of the size being the current length
of the ist and did not think of simply resetting it to 0 before starting the
processing.
Here's a preparation patch to the ist library, allowing this pattern to be
cleanly implemented. I've already incorporated it into my local normalization
branch, but I'd like to implement the remaining feedback before sending a new
series for it.
So: Please already take this patch if you think it is good.
Best regards
Tim Düsterhus
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
istclear allows one to easily reset an ist to zero-size, while preserving the
previous size, indicating the length of the underlying buffer.
---
include/import/ist.h | 30 ++
1 file changed, 30 insertions(+)
diff --git a/include/import/ist.h b/include/import/ist.h
index af9bbac3c..0dc3008f5 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -281,6 +281,36 @@ static inline struct ist isttrim(const struct ist ist,
size_t size)
return ret;
}
+/* Sets the of the to zero and returns the previous length.
+ *
+ * This function is meant to be used in functions that receive an ist
containing
+ * the destination buffer and the buffer's size. The returned size must be
stored
+ * to prevent an overflow of such a destination buffer.
+ *
+ * If you simply want to clear an ist and do not care about the previous length
+ * then you should use `isttrim(ist, 0)`.
+ *
+ * Example Usage (fill the complete buffer with 'x'):
+ *
+ * void my_func(struct ist* dst)
+ * {
+ * size_t dst_size = istclear(dst);
+ * size_t i;
+ *
+ * for (i = 0; i < dst_size; i++)
+ * *dst = __istappend(*dst, 'x');
+ * }
+ */
+__attribute__((warn_unused_result))
+static inline size_t istclear(struct ist* ist)
+{
+ size_t len = ist->len;
+
+ ist->len = 0;
+
+ return len;
+}
+
/* trims string to no more than -1 characters and ensures that a
* zero is placed after (possibly reduced by one) and before ,
* unless is already zero. The string is returned. This is mostly aimed
--
2.31.1
[PATCH] fix cirrus-ci builds by installing "pcre" package
Hello, something changed in freebsd packaging. we now need to install pcre directly. Ilya From 406a5b8cfee330cc74c18f0eca1811195e7eff6d Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Wed, 14 Apr 2021 21:47:34 +0500 Subject: [PATCH] CI: cirrus: install "pcre" package it turned out that our cirrus-ci freebsd builds got broken because of missing "pcre". Most probably it was installed earlier as a dependency. let us install it directly. --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 0ac21bf0d..fdabfdcdd 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -4,7 +4,7 @@ FreeBSD_task: image_family: freebsd-12-2 only_if: $CIRRUS_BRANCH =~ 'master|next' install_script: -- pkg update -f && pkg upgrade -y && pkg install -y openssl git gmake lua53 socat +- pkg update -f && pkg upgrade -y && pkg install -y openssl git gmake lua53 socat pcre script: - git clone https://github.com/VTest/VTest.git ../vtest - make -C ../vtest -- 2.30.2
Re: [PATCH] MINOR: sample: add json_string
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 be extacted from a Header
or body of the request and saved to a variable.
This converter makes it possible to handle some JSON Workload
to route requests to differnt backends.
Typo: different.
---
doc/configuration.txt | 32
reg-tests/converter/json_query.vtc | 116 +
src/sample.c | 95 +++
3 files changed, 243 insertions(+)
create mode 100644 reg-tests/converter/json_query.vtc
diff --git a/doc/configuration.txt b/doc/configuration.txt
index f242300e7..374e7939b 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15961,6 +15961,38 @@ json([])
Output log:
{"ip":"127.0.0.1","user-agent":"Very \"Ugly\" UA 1\/2"}
+
This empty line should not be there.
+json_query(,[])
+ This converter searches for the key given by and returns
+ the value.
+ must be a valid JSONPath String as defined in
I'd use string in lowercase.
+ https://datatracker.ietf.org/doc/draft-ietf-jsonpath-base/
+
+ A floating point value will always be returned as String.
+
+ The follwing JSON types are recognized.
Typo: following.
I'd also use a ':' instead of '.'.
+ - 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
+ converted to a String. If its known that the value is a
+ integer then add 'int' to the which helps
+ haproxy to convert the value to a integer for further usage;
I'd probably completely rephrase this as:
The json_query converter supports the JSON types string, boolean and
number. Floating point numbers will be returned as a string. By
specifying the output_type 'int' the value will be converted to an
Integer. If conversion is not possible the json_query converter fails.
+ Example:
+ # get the value of the key 'iss' from a JWT Bearer token
+ http-request set-var(txn.token_payload)
req.hdr(Authorization),word(2,.),ub64dec,json_query('$.iss')
+
+ # get a integer value from the request body
+ # "{"integer":4}" => 5
+ http-request set-var(txn.pay_int)
req.body,json_query('$.integer','int'),add(1)
+
+ # get a key with '.' in the name
+ # {"my.key":"myvalue"} => myvalue
+ http-request set-var(txn.pay_mykey) req.body,json_query('$.my\\.key')
+
+ # {"boolean-false":false} => 0
+ http-request set-var(txn.pay_boolean_false)
req.body,json_query('$.boolean-false')
These examples look good to me. I'd just move the JWT example to the
bottom, so that the simple examples come first.
language([,])
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 0..88ef58a0c
--- /dev/null
+++ b/reg-tests/converter/json_query.vtc
@@ -0,0 +1,116 @@
+varnishtest "JSON Query converters Test"
+#REQUIRE_VERSION=2.4
+
+feature ignore_unknown_macro
+
+server s1 {
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+
+ rxreq
+ txresp
+} -start
You can use '-repeat 8' to simplify the server definition.
+haproxy h1 -conf {
+defaults
+ mode http
+ timeout connect 1s
+ timeout client 1s
+ timeout server 1s
+ option http-buffer-request
+
+frontend fe
+ bind "fd@${fe}"
+ tcp-request inspect-delay 1s
+
+ http-request set-var(sess.header_json)
req.hdr(Authorization),json_query('$.iss')
+ http-request set-var(sess.pay_json) req.body,json_query('$.iss')
+ http-request set-var(sess.pay_int)
req.body,json_query('$.integer',"int"),add(1)
+http-request set-var(sess.pay_neg_int)
req.body,json_query('$.negativ-integer',"int"),add(1)
Inconsistent indentation here.
+ http-request set-var(sess.pay_double) req.body,json_query('$.double')
+ http-request set-var(sess.pay_boolean_true)
req.body,json_query('$.boolean-true')
+ http-request set-var(sess.pay_boolean_false)
req.body,json_query('$.boolean-false')
+ http-request set-var(sess.pay_mykey) req.body,json_query('$.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
Re: Still 100% CPU usage in 2.3.9 & 2.2.13 (Was: Re: [2.2.9] 100% CPU usage)
Le 10/04/2021 à 00:34, Robin H. Johnson a écrit : On Fri, Apr 09, 2021 at 10:14:26PM +0200, Christopher Faulet wrote: It seems you have a blocking call in one of your lua script. The threads dump shows many threads blocked in hlua_ctx_init. Many others are executing lua. Unfortunately, for a unknown reason, there is no stack traceback. All of our Lua is string handling. Parsing headers, and then building TXN variables as well as a set of applets that return responses in cases where we don't want to go to a backend server as the response is simple enough to generate inside the LB. For the 2.3 and prior, the lua scripts are executed under a global lock. Thus blocking calls in a lua script are awful, because it does not block only one thread but all others too. I guess the same issue exists on the 1.8, but there is no watchdog on this version. Thus, time to time HAProxy hangs and may report huge latencies but, at the end it recovers and continues to process data. It is exactly the purpose of the watchdog, reporting hidden bugs related to spinning loops and deadlocks. Nothing in this Lua code should have blocking calls at all. The Lua code has zero calls to external services, no sockets, no sleeps, no print, no Map.new (single call in the Lua startup, not inside any applet or fetch), no usage of other packages, no file handling, no other IO. I'm hoping I can get $work to agree to fully open-source the Lua, so you can see this fact and review the code to confirm that it SHOULD be non-blocking. I trust you on this point. If you are not using external component, it should indeed be ok. So, it is probably a contention issue on the global Lua lock. If you are able to generate and inspect a core file, it should help you to figure out what really happens. However, I may be wrong. It may be just a contention problem because your are executing lua with 64 threads and a huge workload. In this case, you may give a try to the 2.4 (under development). There is a way to have a separate lua context for each thread loading the scripts with "lua-load-per-thread" directive. Out of curiosity, on the 1.8, are you running HAProxy with several threads or are you spawning several processes? nbthread=64, nbproc=1 on both 1.8/2.x It is thus surprising, if it is really a contention issue, that you never observed slow down on the 1.8. There is no watchdog, but the thread implementation is a bit awkward on the 1.8. 2.X are better on this point, the best being the 2.4. Yes, we're hoping to try 2.4.x, just working on some parts to get there. -- Christopher Faulet
Re: [PATCH] MINOR: sample: add json_string
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 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) ?
This would then make the converter return nothing in case of violation
(i.e. risk of losing precision). This would also reject NaN and infinite
that the lib supports.
You mean the same check which is done in arith_add().
Not exactly because arith_add only checks for overflows after addition
and tries to cap the result, but I'd rather just say that if the decoded
number is <= -2^53 or >= 2^53 then the converter should return a no match
in case an integer was requested.
Okay got you.
There is such a check in stats.c which I copied to sample.c but this does
not look right.
Maybe I should create a include/haproxy/json-t.h and add the values there,
what do you think?
```
/* Limit JSON integer values to the range [-(2**53)+1, (2**53)-1] as per
* the recommendation for interoperable integers in section 6 of RFC 7159.
*/
#define JSON_INT_MAX ((1ULL << 53) - 1)
#define JSON_INT_MIN (0 - JSON_INT_MAX)
/* 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();
const char *p; /* holds the temporay string from mjson_find */
int tok, n; /* holds the token enum and the length of the value */
int rc; /* holds the return code from mjson_get_string */
tok = mjson_find(smp->data.u.str.area, smp->data.u.str.data,
args[0].data.str.area, , );
switch(tok) {
case MJSON_TOK_NUMBER:
if (args[1].type == ARGT_SINT) {
smp->data.u.sint = atoll(p);
if (smp->data.u.sint < JSON_INT_MIN || smp->data.u.sint >
JSON_INT_MAX)
return 0;
smp->data.type = SMP_T_SINT;
} else {
...
```
H that's not your fault but now I'm seeing that we already have a
converter inappropriately called "json", so we don't even know in which
direction it works by just looking at its name :-( Same issue as for
base64.
May I suggest that you call yours "json_decode" or maybe shorter
"json_dec" so that it's more explicit that it's the decode one ? Because
for me "json_string" is the one that will emit a json string from some
input (which it is not). Then we could later create "json_enc" and warn
when "json" alone is used. Or even "jsdec" and "jsenc" which are much
shorter and still quite explicit.
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 it sounds natural to others I'm fine
with this.
I'm seeing that there's a very nice mjson_find() which does *exactly* what
you need:
"In a JSON string s, len, find an element by its JSONPATH path. Save
found element in tokptr, toklen. If not found, return JSON_TOK_INVALID.
If found, return one of: MJSON_TOK_STRING, MJSON_TOK_NUMBER,
MJSON_TOK_TRUE, MJSON_TOK_FALSE, MJSON_TOK_NULL, MJSON_TOK_ARRAY,
MJSON_TOK_OBJECT.
If a searched key contains ., [ or ] characters, they should be escaped
by a backslash."
So you get the type in return. I think you can then call one of the
related functions depending on what is found, which is more reliable
than iterating over multiple attempts.
Oh yes, this sounds like a better approach.
I have now used this suggestion and I hope you can help me to fix the double
parsing
issue or is it acceptable to parse the input twice?
From what I've seen in the code in the lib you have no other option.
I thought it might be possible to call mjson_get_string() on the
resulting pointer but you would need to find a way to express that
you want to extract the immediate content, maybe by having an empty
key designation or something like this. This point is not clear to
me and the unit tests in the project all re-parse the input string
after mjson_find(), so probably this is the way to do it.
The check functions handles the int arg now as suggested.
```
/* 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)
{
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) {
if (strncmp(arg[1].data.str.area, "int",
[OT PATCH 2/2] MINOR: opentracing: transfer of context names without prefix
Hello all,
In order to enable the assignment of a context name, and yet exclude the
use of that name (prefix in this case) when extracting the context from
the HTTP header, a special character '-' has been added, which can be
specified at the beginning of the prefix.
So let's say if we look at examples of the fe-be configuration, we can
transfer the context via an HTTP header without a prefix like this:
fe/ot.cfg:
..
span "HAProxy session"
inject "" use-headers
event on-backend-http-request
Such a context can be read in another process using a name that has a
special '-' sign at the beginning:
be/ot.cfg:
ot-scope frontend_http_request
extract "-ot-ctx" use-headers
span "HAProxy session" child-of "-ot-ctx" root
..
This means that the context name will be '-ot-ctx' but it will not be
used when extracting data from HTTP headers.
Of course, if the context does not have a prefix set, all HTTP headers
will be inserted into the OpenTracing library as context. All of the
above will only work correctly if that library can figure out what is
relevant to the context and what is not.
Best regards,
--
Zaga
What can change the nature of a man?
>From 6ede4cf6e83b327db132e830dd09b728057ae375 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac
Date: Wed, 14 Apr 2021 11:47:28 +0200
Subject: [PATCH 2/2] MINOR: opentracing: transfer of context names without
prefix
In order to enable the assignment of a context name, and yet exclude the
use of that name (prefix in this case) when extracting the context from
the HTTP header, a special character '-' has been added, which can be
specified at the beginning of the prefix.
So let's say if we look at examples of the fe-be configuration, we can
transfer the context via an HTTP header without a prefix like this:
fe/ot.cfg:
..
span "HAProxy session"
inject "" use-headers
event on-backend-http-request
Such a context can be read in another process using a name that has a
special '-' sign at the beginning:
be/ot.cfg:
ot-scope frontend_http_request
extract "-ot-ctx" use-headers
span "HAProxy session" child-of "-ot-ctx" root
..
This means that the context name will be '-ot-ctx' but it will not be
used when extracting data from HTTP headers.
Of course, if the context does not have a prefix set, all HTTP headers
will be inserted into the OpenTracing library as context. All of the
above will only work correctly if that library can figure out what is
relevant to the context and what is not.
---
addons/ot/include/parser.h | 1 +
addons/ot/src/http.c | 13 +
2 files changed, 14 insertions(+)
diff --git a/addons/ot/include/parser.h b/addons/ot/include/parser.h
index 5e7b298d5..75a39cc0c 100644
--- a/addons/ot/include/parser.h
+++ b/addons/ot/include/parser.h
@@ -38,6 +38,7 @@
#define FLT_OT_PARSE_SPAN_REF_CHILD "child-of"
#define FLT_OT_PARSE_SPAN_REF_FOLLOWS "follows-from"
#define FLT_OT_PARSE_CTX_AUTONAME "-"
+#define FLT_OT_PARSE_CTX_IGNORE_NAME'-'
#define FLT_OT_PARSE_CTX_USE_HEADERS"use-headers"
#define FLT_OT_PARSE_CTX_USE_VARS "use-vars"
#define FLT_OT_PARSE_OPTION_HARDERR "hard-errors"
diff --git a/addons/ot/src/http.c b/addons/ot/src/http.c
index 72b31b76f..4a12ed854 100644
--- a/addons/ot/src/http.c
+++ b/addons/ot/src/http.c
@@ -99,6 +99,19 @@ struct otc_text_map *flt_ot_http_headers_get(struct channel *chn, const char *pr
if (chn == NULL)
FLT_OT_RETURN(retptr);
+ /*
+ * The keyword 'inject' allows you to define the name of the OpenTracing
+ * context without using a prefix. In that case all HTTP headers are
+ * transferred because it is not possible to separate them from the
+ * OpenTracing context (this separation is usually done via a prefix).
+ *
+ * When using the 'extract' keyword, the context name must be specified.
+ * To allow all HTTP headers to be extracted, the first character of
+ * that name must be set to FLT_OT_PARSE_CTX_IGNORE_NAME.
+ */
+ if (FLT_OT_STR_ISVALID(prefix) && (*prefix == FLT_OT_PARSE_CTX_IGNORE_NAME))
+ prefix_len = 0;
+
htx = htxbuf(&(chn->buf));
for (pos = htx_get_first(htx); pos != -1; pos = htx_get_next(htx, pos)) {
--
2.30.1
[OT PATCH 1/2] MINOR: opentracing: correct calculation of the number of arguments in the args[]
Hello all,
It is possible that some arguments within the configuration line are not
specified; that is, they are set to a blank string.
For example:
keyword '' arg_2
In that case the content of the args field will be like this:
args[0]: 'keyword'
args[1]: NULL pointer
args[2]: 'arg_2'
args[3 .. MAX_LINE_ARGS): NULL pointers
The previous way of calculating the number of arguments (as soon as a
null pointer is encountered) could not place an argument on an empty
string.
All of the above is essential for passing the OpenTracing context via
the HTTP headers (keyword 'inject'), where one of the arguments is the
context name prefix. This way we can set an empty prefix, which is very
useful if we get context from some other process that can't add a prefix
to that data; or we want to pass the context to some process that cannot
handle the prefix of that data.
Best regards,
--
Zaga
What can change the nature of a man?
>From 0c0c0c77fe5b1a71ea639b5693eab85a917e9df0 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac
Date: Wed, 14 Apr 2021 11:44:58 +0200
Subject: [PATCH 1/2] MINOR: opentracing: correct calculation of the number of
arguments in the args[]
It is possible that some arguments within the configuration line are not
specified; that is, they are set to a blank string.
For example:
keyword '' arg_2
In that case the content of the args field will be like this:
args[0]: 'keyword'
args[1]: NULL pointer
args[2]: 'arg_2'
args[3 .. MAX_LINE_ARGS): NULL pointers
The previous way of calculating the number of arguments (as soon as a
null pointer is encountered) could not place an argument on an empty
string.
All of the above is essential for passing the OpenTracing context via
the HTTP headers (keyword 'inject'), where one of the arguments is the
context name prefix. This way we can set an empty prefix, which is very
useful if we get context from some other process that can't add a prefix
to that data; or we want to pass the context to some process that cannot
handle the prefix of that data.
---
addons/ot/src/parser.c | 17
addons/ot/src/util.c | 44 +++---
2 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index c7522e942..5dec8629d 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -189,7 +189,7 @@ static const char *flt_ot_parse_invalid_char(const char *name, int type)
*/
static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, const void *id, const struct flt_ot_parse_data *parse_data, size_t parse_data_size, const struct flt_ot_parse_data **pdata, char **err)
{
- int i, retval = ERR_NONE;
+ int i, argc, retval = ERR_NONE;
FLT_OT_FUNC("\"%s\", %d, %p, %p, %p, %zu, %p:%p, %p:%p", file, linenum, args, id, parse_data, parse_data_size, FLT_OT_DPTR_ARGS(pdata), FLT_OT_DPTR_ARGS(err));
@@ -197,12 +197,15 @@ static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, co
*pdata = NULL;
+ /* First check here if args[0] is the correct keyword. */
for (i = 0; (*pdata == NULL) && (i < parse_data_size); i++)
if (strcmp(parse_data[i].name, args[0]) == 0)
*pdata = parse_data + i;
if (*pdata == NULL)
FLT_OT_PARSE_ERR(err, "'%s' : unknown keyword", args[0]);
+ else
+ argc = flt_ot_args_count(args);
if ((retval & ERR_CODE) || (id == NULL))
/* Do nothing. */;
@@ -214,20 +217,16 @@ static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, co
* line than is required.
*/
if (!(retval & ERR_CODE))
- for (i = 1; i < (*pdata)->args_min; i++)
- if (!FLT_OT_ARG_ISVALID(i))
-FLT_OT_PARSE_ERR(err, "'%s' : too few arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
+ if (argc < (*pdata)->args_min)
+ FLT_OT_PARSE_ERR(err, "'%s' : too few arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
/*
* Checking that more arguments are specified in the configuration
* line than the maximum allowed.
*/
- if (!(retval & ERR_CODE) && ((*pdata)->args_max > 0)) {
- for ( ; (i <= (*pdata)->args_max) && FLT_OT_ARG_ISVALID(i); i++);
-
- if (i > (*pdata)->args_max)
+ if (!(retval & ERR_CODE) && ((*pdata)->args_max > 0))
+ if (argc > (*pdata)->args_max)
FLT_OT_PARSE_ERR(err, "'%s' : too many arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
- }
/* Checking that the first argument has only allowed characters. */
if (!(retval & ERR_CODE) && ((*pdata)->check_name > 0)) {
diff --git a/addons/ot/src/util.c b/addons/ot/src/util.c
index 3adc5a300..f6812bcc2 100644
--- a/addons/ot/src/util.c
+++ b/addons/ot/src/util.c
@@ -37,13 +37,13 @@
*/
void flt_ot_args_dump(char **args)
{
- int i, n;
+ int i, argc;
- for (n = 1; FLT_OT_ARG_ISVALID(n); n++);
+ argc = flt_ot_args_count(args);
- (void)fprintf(stderr,
[OT PATCH 0/2] opentracing: use of the context name without prefix
Hello all, the next two patches allow you to transfer the OpenTracing context via an HTTP headers without using a name prefix. This is very useful when transferring context from or to some other process that cannot add or remove a prefix from that data. Best regards, -- Zaga What can change the nature of a man?
Memory allocation for haproxy
Hi all, I am working on a new patch for haproxy and would need the malloc method used to allocate memory for structs like the connection struct in connection-t.c In case I want to expand the struct with a new variable, do I need to take care of the memory for this variable or is there already a dynamic memory allocation for the given struct? Best regards ___ Robert Ionescu *The information contained in this message is confidential and may be legally privileged. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, dissemination, or reproduction is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.*
Re: [PATCH] MINOR: sample: add json_string
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 with no decimal part) ?
This would then make the converter return nothing in case of violation
(i.e. risk of losing precision). This would also reject NaN and infinite
that the lib supports.
You mean the same check which is done in arith_add().
Not exactly because arith_add only checks for overflows after addition
and tries to cap the result, but I'd rather just say that if the decoded
number is <= -2^53 or >= 2^53 then the converter should return a no match
in case an integer was requested.
Okay got you.
There is such a check in stats.c which I copied to sample.c but this does
not look right.
Maybe I should create a include/haproxy/json-t.h and add the values there,
what do you think?
```
/* Limit JSON integer values to the range [-(2**53)+1, (2**53)-1] as per
* the recommendation for interoperable integers in section 6 of RFC 7159.
*/
#define JSON_INT_MAX ((1ULL << 53) - 1)
#define JSON_INT_MIN (0 - JSON_INT_MAX)
/* 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();
const char *p; /* holds the temporay string from mjson_find */
int tok, n;/* holds the token enum and the length of the value */
int rc;/* holds the return code from mjson_get_string */
tok = mjson_find(smp->data.u.str.area, smp->data.u.str.data,
args[0].data.str.area, , );
switch(tok) {
case MJSON_TOK_NUMBER:
if (args[1].type == ARGT_SINT) {
smp->data.u.sint = atoll(p);
if (smp->data.u.sint < JSON_INT_MIN ||
smp->data.u.sint > JSON_INT_MAX)
return 0;
smp->data.type = SMP_T_SINT;
} else {
...
```
H that's not your fault but now I'm seeing that we already have a
converter inappropriately called "json", so we don't even know in which
direction it works by just looking at its name :-( Same issue as for
base64.
May I suggest that you call yours "json_decode" or maybe shorter
"json_dec" so that it's more explicit that it's the decode one ? Because
for me "json_string" is the one that will emit a json string from some
input (which it is not). Then we could later create "json_enc" and warn
when "json" alone is used. Or even "jsdec" and "jsenc" which are much
shorter and still quite explicit.
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 it sounds natural to others I'm fine
with this.
I'm seeing that there's a very nice mjson_find() which does *exactly* what
you need:
"In a JSON string s, len, find an element by its JSONPATH path. Save
found element in tokptr, toklen. If not found, return JSON_TOK_INVALID.
If found, return one of: MJSON_TOK_STRING, MJSON_TOK_NUMBER,
MJSON_TOK_TRUE, MJSON_TOK_FALSE, MJSON_TOK_NULL, MJSON_TOK_ARRAY,
MJSON_TOK_OBJECT.
If a searched key contains ., [ or ] characters, they should be escaped
by a backslash."
So you get the type in return. I think you can then call one of the
related functions depending on what is found, which is more reliable
than iterating over multiple attempts.
Oh yes, this sounds like a better approach.
I have now used this suggestion and I hope you can help me to fix the double
parsing
issue or is it acceptable to parse the input twice?
From what I've seen in the code in the lib you have no other option.
I thought it might be possible to call mjson_get_string() on the
resulting pointer but you would need to find a way to express that
you want to extract the immediate content, maybe by having an empty
key designation or something like this. This point is not clear to
me and the unit tests in the project all re-parse the input string
after mjson_find(), so probably this is the way to do it.
The check functions handles the int arg now as suggested.
```
/* 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)
{
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) {
Re: [PATCH] MINOR: sample: add json_string
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 representative user and am aware of the "jq" command- > > line utility that does this. So if it sounds natural to others I'm fine > > with this. > > +1 for json_query. It was among my suggestions for an improved name. > > We are not decoding the full JSON into a data structure, we are querying the > value of a single entry (as in jq or SQL). json_decode or something like > that would be misleading. OK, fine by me then. Thanks, Willy
Re: [PATCH] MINOR: sample: add json_string
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 it sounds natural to others I'm fine with this. +1 for json_query. It was among my suggestions for an improved name. We are not decoding the full JSON into a data structure, we are querying the value of a single entry (as in jq or SQL). json_decode or something like that would be misleading. Best regards Tim Düsterhus
