Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 05:19:26PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> Fun. I didn't receive your reply on company mail. I only got it from the
> list using my personal subscription. I hope this message threads properly.

Yep it does.

> Muscle memory is too strong :-/ I even used search and replace to adjust
> the name in all files, except configuration.txt. I even had to fix the
> 'either' within the GPL license comment.
> 
> Willy, find a patch attached. I've put Miroslav as 'Reported-by'.

Thanks, now merged (just after the dev4 release).

Willy



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Tim Düsterhus , WoltLab GmbH
Miroslav,

Am 11.09.20 um 17:10 schrieb Miroslav Zagorac:
> there is a small typo in the patch, if says 'iff' instead of 'iif':
> 
> ---
> +  Example:
> +    http-request set-header x-forwarded-proto %[ssl_fc,iff(https,http)]
> ---
> 

Fun. I didn't receive your reply on company mail. I only got it from the
list using my personal subscription. I hope this message threads properly.

Muscle memory is too strong :-/ I even used search and replace to adjust
the name in all files, except configuration.txt. I even had to fix the
'either' within the GPL license comment.

Willy, find a patch attached. I've put Miroslav as 'Reported-by'.

Best regards
Tim Düsterhus
Developer WoltLab GmbH

-- 

WoltLab GmbH
Nedlitzer Str. 27B
14469 Potsdam

Tel.: +49 331 96784338

duester...@woltlab.com
www.woltlab.com

Managing director:
Marcel Werk

AG Potsdam HRB 26795 P
>From e9ea0196ab20d8cd92ba8a9651e0da7e9575f9ae Mon Sep 17 00:00:00 2001
From: Tim Duesterhus 
Date: Fri, 11 Sep 2020 17:13:12 +0200
Subject: [PATCH] DOC: Fix typo in iif() example
To: haproxy@formilux.org
Cc: w...@1wt.eu

It should read 'iif', not 'iff'.

Reported-by: Miroslav Zagorac 
---
 doc/configuration.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 0e34f620d..075d3d5c8 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15155,7 +15155,7 @@ iif(,)
   string otherwise.
 
   Example:
-http-request set-header x-forwarded-proto %[ssl_fc,iff(https,http)]
+http-request set-header x-forwarded-proto %[ssl_fc,iif(https,http)]
 
 in_table()
   Uses the string representation of the input sample to perform a look up in
-- 
2.28.0



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Miroslav Zagorac

Hello all,

there is a small typo in the patch, if says 'iff' instead of 'iif':

---
+  Example:
+http-request set-header x-forwarded-proto %[ssl_fc,iff(https,http)]
---

--
Zaga

What can change the nature of a man?



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 04:55:45PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> I consider 'iif' a bit obscure. It easily looks like a typo. Similar to
> 'iff' for 'if and only if' which tends to generate a number of questions
> as well.

I agree but others possibly know it and we should not consider our own
case as representative, so let's see.

> But I've certainly seen 'iif' being used before and I don't strongly
> care for the naming. I'm happy with anything.
> 
> I've attached an updated patch and leave it up to you whether you want
> to merge it and whether you'd like to wait for more opinions or not :-)

I've just merged it so that it gets better exposure, that's the best way
to receive criticisms and proposals for another name :-)

Thanks!
Willy



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Tim Düsterhus , WoltLab GmbH
Willy,

Am 11.09.20 um 16:46 schrieb Willy Tarreau:
> First, I really like the feature, that's a great idea.

:-)

>>> - choice (my initial choice)
>>> - ifor / if_or
>>> - ifelse / if_else
>>> - iftrue (with the  argument being optional)
>>
>> Maybe something like this would be appropriate (IIF)?
> 
> I was thinking about "select" which I've seldom seen, I never heard about
> "iif" previously but it has the benefit of being quite short (convenient
> for long expressions) and already in use elsewhere. And it also has the
> benefit that someone searching for "if" would easily stumble on it.
> 
> I'm fine with the patch BTW, so if everyone agrees on "iif", let's suggest
> we go with it ?
> 

I consider 'iif' a bit obscure. It easily looks like a typo. Similar to
'iff' for 'if and only if' which tends to generate a number of questions
as well.

But I've certainly seen 'iif' being used before and I don't strongly
care for the naming. I'm happy with anything.

I've attached an updated patch and leave it up to you whether you want
to merge it and whether you'd like to wait for more opinions or not :-)

Best regards
Tim Düsterhus
Developer WoltLab GmbH

-- 

WoltLab GmbH
Nedlitzer Str. 27B
14469 Potsdam

Tel.: +49 331 96784338

duester...@woltlab.com
www.woltlab.com

Managing director:
Marcel Werk

AG Potsdam HRB 26795 P
>From 9022a2f0b3e2ab449debfe6db919312aa97a5262 Mon Sep 17 00:00:00 2001
From: Tim Duesterhus 
Date: Fri, 11 Sep 2020 14:25:23 +0200
Subject: [PATCH] MINOR: Add iif(,) converter
To: haproxy@formilux.org
Cc: w...@1wt.eu

iif() takes a boolean as input and returns one of the two argument
strings depending on whether the boolean is true.

This converter most likely is most useful to return the proper scheme
depending on the value returned by the `ssl_fc` fetch, e.g. for use within
the `x-forwarded-proto` request header.

However it can also be useful for use within a template that is sent to
the client using `http-request return` with a `lf-file`. It allows the
administrator to implement a simple condition, without needing to prefill
variables within the regular configuration using `http-request
set-var(req.foo)`.
---
 doc/configuration.txt   |  7 ++
 reg-tests/converter/iif.vtc | 45 +
 src/sample.c| 22 ++
 3 files changed, 74 insertions(+)
 create mode 100644 reg-tests/converter/iif.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index c1f6f8219..dd8d5889a 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15150,6 +15150,13 @@ http_date([])
   microseconds since epoch. Offset is assumed to have the same unit as
   input timestamp.
 
+iif(,)
+  Returns the  string if the input value is true. Returns the 
+  string otherwise.
+
+  Example:
+http-request set-header x-forwarded-proto %[ssl_fc,iff(https,http)]
+
 in_table()
   Uses the string representation of the input sample to perform a look up in
   the specified table. If the key is not found in the table, a boolean false
diff --git a/reg-tests/converter/iif.vtc b/reg-tests/converter/iif.vtc
new file mode 100644
index 0..360b63f96
--- /dev/null
+++ b/reg-tests/converter/iif.vtc
@@ -0,0 +1,45 @@
+varnishtest "iif converter Test"
+
+feature ignore_unknown_macro
+
+server s1 {
+	rxreq
+	txresp
+} -repeat 3 -start
+
+haproxy h1 -conf {
+defaults
+	mode http
+	timeout connect 1s
+	timeout client  1s
+	timeout server  1s
+
+frontend fe
+	bind "fd@${fe}"
+
+	 requests
+	http-request set-var(txn.iif) req.hdr_cnt(count),iif(ok,ko)
+	http-response set-header iif %[var(txn.iif)]
+
+	default_backend be
+
+backend be
+	server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+	txreq
+	rxresp
+	expect resp.status == 200
+	expect resp.http.iif == "ko"
+	txreq \
+		-hdr "count: 1"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.iif == "ok"
+	txreq \
+		-hdr "count: 1,2"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.iif == "ok"
+} -run
diff --git a/src/sample.c b/src/sample.c
index 3a1534865..a9c08ef54 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -3121,6 +3121,26 @@ static int sample_conv_secure_memcmp(const struct arg *arg_p, struct sample *smp
 }
 #endif
 
+/* Takes a boolean as input. Returns the first argument if that boolean is true and
+ * the second argument otherwise.
+ */
+static int sample_conv_iif(const struct arg *arg_p, struct sample *smp, void *private)
+{
+	smp->data.type = SMP_T_STR;
+	smp->flags |= SMP_F_CONST;
+
+	if (smp->data.u.sint) {
+		smp->data.u.str.data = arg_p[0].data.str.data;
+		smp->data.u.str.area = arg_p[0].data.str.area;
+	}
+	else {
+		smp->data.u.str.data = arg_p[1].data.str.data;
+		smp->data.u.str.area = arg_p[1].data.str.area;
+	}
+
+	return 1;
+}
+
 #define GRPC_MSG_COMPRESS_FLAG_SZ 1 /* 1 byte */
 #define GRPC_MSG_LENGTH_SZ4 /* 4 bytes */
 #define GRPC_MSG_HEADER_SZ(GRPC_MSG_COMPRESS_FLAG_SZ + GRPC_MSG_LENGTH_SZ)
@@ -3782,6 +3802,8 

Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Willy Tarreau
Hi guys,

First, I really like the feature, that's a great idea.

On Fri, Sep 11, 2020 at 04:28:31PM +0200, Miroslav Zagorac wrote:
> On 09/11/2020 03:56 PM, Tim Düsterhus, WoltLab GmbH wrote:
> > We've had a bit of discussion regarding the naming of the converter. I
> > wanted to avoid calling it `if`, because then we could have stuff like this:
> > 
> >http-request set-var(txn.foo) bool(1),if(bar,baz)
> > 
> > which can easily be confused with:
> > 
> >http-request set-var(txn.foo) bool(1) if bar
> > 
> > Some other naming suggestions that came up:
> > 
> > - choice (my initial choice)
> > - ifor / if_or
> > - ifelse / if_else
> > - iftrue (with the  argument being optional)
> 
> Maybe something like this would be appropriate (IIF)?

I was thinking about "select" which I've seldom seen, I never heard about
"iif" previously but it has the benefit of being quite short (convenient
for long expressions) and already in use elsewhere. And it also has the
benefit that someone searching for "if" would easily stumble on it.

I'm fine with the patch BTW, so if everyone agrees on "iif", let's suggest
we go with it ?

thanks,
Willy



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Miroslav Zagorac

On 09/11/2020 03:56 PM, Tim Düsterhus, WoltLab GmbH wrote:

We've had a bit of discussion regarding the naming of the converter. I
wanted to avoid calling it `if`, because then we could have stuff like this:

   http-request set-var(txn.foo) bool(1),if(bar,baz)

which can easily be confused with:

   http-request set-var(txn.foo) bool(1) if bar

Some other naming suggestions that came up:

- choice (my initial choice)
- ifor / if_or
- ifelse / if_else
- iftrue (with the  argument being optional)



Maybe something like this would be appropriate (IIF)?

https://en.wikipedia.org/wiki/IIf

--
Zaga

What can change the nature of a man?



[PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Tim Düsterhus , WoltLab GmbH
Willy,

[keep this email in CC, it's not subscribed to the list]

"either() takes a boolean as input and returns one of the two argument
strings depending on whether the boolean is true."

Find the full details in the attached patch.

---

We've had a bit of discussion regarding the naming of the converter. I
wanted to avoid calling it `if`, because then we could have stuff like this:

  http-request set-var(txn.foo) bool(1),if(bar,baz)

which can easily be confused with:

  http-request set-var(txn.foo) bool(1) if bar

Some other naming suggestions that came up:

- choice (my initial choice)
- ifor / if_or
- ifelse / if_else
- iftrue (with the  argument being optional)

I'm happy to adjust the patch based on your preferences.

I'd also appreciate if this one could make a backport into 2.2. But I
would fully understand if you refuse to backport this into a LTS branch.
November is not too far away :-)

Best regards
Tim Düsterhus
Developer WoltLab GmbH

-- 

WoltLab GmbH
Nedlitzer Str. 27B
14469 Potsdam

Tel.: +49 331 96784338

duester...@woltlab.com
www.woltlab.com

Managing director:
Marcel Werk

AG Potsdam HRB 26795 P
>From 52e50548f97a795cb891adf0f18a8ebc1e38b188 Mon Sep 17 00:00:00 2001
From: Tim Duesterhus 
Date: Fri, 11 Sep 2020 14:25:23 +0200
Subject: [PATCH] MINOR: Add either(,) converter
To: haproxy@formilux.org
Cc: w...@1wt.eu

either() takes a boolean as input and returns one of the two argument
strings depending on whether the boolean is true.

This converter most likely is most useful to return the proper scheme
depending on the value returned by the `ssl_fc` fetch, e.g. for use within
the `x-forwarded-proto` request header.

However it can also be useful for use within a template that is sent to
the client using `http-request return` with a `lf-file`. It allows the
administrator to implement a simple condition, without needing to prefill
variables within the regular configuration using `http-request
set-var(req.foo)`.
---
 doc/configuration.txt  |  7 ++
 reg-tests/converter/either.vtc | 45 ++
 src/sample.c   | 22 +
 3 files changed, 74 insertions(+)
 create mode 100644 reg-tests/converter/either.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index c1f6f8219..197b5cec5 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15094,6 +15094,13 @@ djb2([])
   32-bit hash is trivial to break. See also "crc32", "sdbm", "wt6", "crc32c",
   and the "hash-type" directive.
 
+either(,)
+  Returns the  string if the input value is true. Returns the 
+  string otherwise.
+
+  Example:
+http-request set-header x-forwarded-proto %[ssl_fc,either(https,http)]
+
 even
   Returns a boolean TRUE if the input value of type signed integer is even
   otherwise returns FALSE. It is functionally equivalent to "not,and(1),bool".
diff --git a/reg-tests/converter/either.vtc b/reg-tests/converter/either.vtc
new file mode 100644
index 0..8a155c356
--- /dev/null
+++ b/reg-tests/converter/either.vtc
@@ -0,0 +1,45 @@
+varnishtest "either converter Test"
+
+feature ignore_unknown_macro
+
+server s1 {
+	rxreq
+	txresp
+} -repeat 3 -start
+
+haproxy h1 -conf {
+defaults
+	mode http
+	timeout connect 1s
+	timeout client  1s
+	timeout server  1s
+
+frontend fe
+	bind "fd@${fe}"
+
+	 requests
+	http-request set-var(txn.either) req.hdr_cnt(count),either(ok,ko)
+	http-response set-header either %[var(txn.either)]
+
+	default_backend be
+
+backend be
+	server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+	txreq
+	rxresp
+	expect resp.status == 200
+	expect resp.http.either == "ko"
+	txreq \
+		-hdr "count: 1"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.either == "ok"
+	txreq \
+		-hdr "count: 1,2"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.either == "ok"
+} -run
diff --git a/src/sample.c b/src/sample.c
index 3a1534865..d6baf6c89 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -3121,6 +3121,26 @@ static int sample_conv_secure_memcmp(const struct arg *arg_p, struct sample *smp
 }
 #endif
 
+/* Takes a boolean as input. Returns the first argument if that boolean is true and
+ * the second argument otherwise.
+ */
+static int sample_conv_either(const struct arg *arg_p, struct sample *smp, void *private)
+{
+	smp->data.type = SMP_T_STR;
+	smp->flags |= SMP_F_CONST;
+
+	if (smp->data.u.sint) {
+		smp->data.u.str.data = arg_p[0].data.str.data;
+		smp->data.u.str.area = arg_p[0].data.str.area;
+	}
+	else {
+		smp->data.u.str.data = arg_p[1].data.str.data;
+		smp->data.u.str.area = arg_p[1].data.str.area;
+	}
+
+	return 1;
+}
+
 #define GRPC_MSG_COMPRESS_FLAG_SZ 1 /* 1 byte */
 #define GRPC_MSG_LENGTH_SZ4 /* 4 bytes */
 #define GRPC_MSG_HEADER_SZ(GRPC_MSG_COMPRESS_FLAG_