Re: [PATCH 1/1] TMP: Add reg-test to check scoping of txn:get_priv()

2018-08-22 Thread Frederic Lecaille

On 08/22/2018 04:20 PM, Tim Düsterhus wrote:

Frederic,

Am 22.08.2018 um 15:21 schrieb Frederic Lecaille:

Thank you a lot for this reg testing file.

Just a little detail (see below).


You are correct of course. It was quickly cobbled together to provide a
reproducer, not meant for committing (a patch is just a convenient means
of shipping the files). That's why I tagged it with 'TMP' and why it's
missing a proper commit message description.


Ok, did not notice the TMP flag.


Also it should be included with the actual commit fixing the issue, like
I did in
http://git.haproxy.org/?p=haproxy.git;a=commitdiff;h=65189c17c694b0b44e0d324d63c055f5329e61c9,
not added separately. Test + Fix should be an atomic unit.


Yes, even if not mandatory, it would be preferable.


Whoever fixes the actual issue should just copy the reg-test into their
commit, fix your remark and add me to the commit message as


I agree again. We will check that.


Co-authored-by: Tim Düsterhus 

:-)


Also note that -run is a shorcut for -start -wait.


Good to know, thanks.

Best regards
Tim Düsterhus






Re: [PATCH 1/1] TMP: Add reg-test to check scoping of txn:get_priv()

2018-08-22 Thread Tim Düsterhus
Frederic,

Am 22.08.2018 um 15:21 schrieb Frederic Lecaille:
> Thank you a lot for this reg testing file.
> 
> Just a little detail (see below).

You are correct of course. It was quickly cobbled together to provide a
reproducer, not meant for committing (a patch is just a convenient means
of shipping the files). That's why I tagged it with 'TMP' and why it's
missing a proper commit message description.

Also it should be included with the actual commit fixing the issue, like
I did in
http://git.haproxy.org/?p=haproxy.git;a=commitdiff;h=65189c17c694b0b44e0d324d63c055f5329e61c9,
not added separately. Test + Fix should be an atomic unit.

Whoever fixes the actual issue should just copy the reg-test into their
commit, fix your remark and add me to the commit message as

Co-authored-by: Tim Düsterhus 

:-)

> Also note that -run is a shorcut for -start -wait.

Good to know, thanks.

Best regards
Tim Düsterhus



[PATCH] MEDIUM: reset lua transaction between http requests

2018-08-22 Thread Patrick Hemmer
Not sure if this is the right approach, but this addresses the issue for me.
This should be backported to 1.8.

-Patrick
From 9087400de99a3925380cac4128a431cd48a09145 Mon Sep 17 00:00:00 2001
From: Patrick Hemmer 
Date: Wed, 22 Aug 2018 10:02:00 -0400
Subject: [PATCH] MEDIUM: reset lua transaction between http requests

Previously LUA code would maintain the transaction state between http requests,
resulting in things like txn:get_priv() retrieving data from a previous requst.
This addresses the issue by ensuring the LUA state is reset between requests.
---
 src/proto_http.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/proto_http.c b/src/proto_http.c
index a7a8dadd6..93c8857cd 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4460,6 +4460,9 @@ void http_end_txn_clean_session(struct stream *s)
s->flags &= ~(SF_CURR_SESS|SF_REDIRECTABLE|SF_SRV_REUSED);
s->flags &= ~(SF_ERR_MASK|SF_FINST_MASK|SF_REDISP);
 
+   hlua_ctx_destroy(s->hlua);
+   s->hlua = NULL;
+
s->txn->meth = 0;
http_reset_txn(s);
s->txn->flags |= TX_NOT_FIRST | TX_WAIT_NEXT_RQ;
-- 
2.18.0



Re: [PATCH 1/1] TMP: Add reg-test to check scoping of txn:get_priv()

2018-08-22 Thread Frederic Lecaille

Hello Tim,

Thank you a lot for this reg testing file.

Just a little detail (see below).

Also note that -run is a shorcut for -start -wait.

Regards.


On 08/22/2018 02:47 PM, Tim Duesterhus wrote:

diff --git a/reg-tests/lua/h1.lua b/reg-tests/lua/h1.lua
new file mode 100644
index ..999ea887
--- /dev/null
+++ b/reg-tests/lua/h1.lua
@@ -0,0 +1,15 @@
+core.register_action("bug", { "http-res" }, function(txn)
+   data = txn:get_priv()
+   if not data then
+   data = 0
+   end
+   data = data + 1
+   print(string.format("set to %d", data))
+   txn.http:res_set_status(200 + data)
+   txn:set_priv(data)
+end)
+
+core.register_service("fakeserv", "http", function(applet)
+   applet:set_status(200)
+   applet:start_response()
+end)
diff --git a/reg-tests/lua/h1.vtc b/reg-tests/lua/h1.vtc
new file mode 100644
index ..b11f21c9
--- /dev/null
+++ b/reg-tests/lua/h1.vtc
@@ -0,0 +1,36 @@
+varnishtest "Lua: txn:get_priv() scope"
+feature ignore_unknown_macro
+


's1' server is useless in this case because 'h1' haproxy configuration 
below does not use any server as backend.



+server s1 -repeat 2 {
+rxreq
+txresp
+} -start
+
+haproxy h1 -conf {
+global
+lua-load ${testdir}/h1.lua
+
+frontend fe1
+mode http
+bind "fd@${fe1}"
+default_backend b1
+
+http-response lua.bug
+
+backend b1
+mode http
+http-request use-service lua.fakeserv
+} -start
+
+client c0 -connect ${h1_fe1_sock} {
+txreq -url "/"
+rxresp
+expect resp.status == 201
+txreq -url "/"
+rxresp
+expect resp.status == 201
+}
+
+client c0 -start
+
+client c0 -wait





[PATCH 0/1] Re: BUG: LUA txn:get_priv() scoped to connection, not transaction

2018-08-22 Thread Tim Duesterhus
Hi

attached comes a reg-test that can be used to verify that the behaviour
that Patrick described actually exists.

I cannot comment on whether txn:get_priv() is working correctly or not,
this is up to you. The test only checks that the value is reset for the
second request.

Best regards
Tim Düsterhus

Tim Duesterhus (1):
  TMP: Add reg-test to check scoping of txn:get_priv()

 reg-tests/lua/h1.lua | 15 +++
 reg-tests/lua/h1.vtc | 36 
 2 files changed, 51 insertions(+)
 create mode 100644 reg-tests/lua/h1.lua
 create mode 100644 reg-tests/lua/h1.vtc

-- 
2.18.0




[PATCH 1/1] TMP: Add reg-test to check scoping of txn:get_priv()

2018-08-22 Thread Tim Duesterhus
see https://www.mail-archive.com/haproxy@formilux.org/msg31015.html
---
 reg-tests/lua/h1.lua | 15 +++
 reg-tests/lua/h1.vtc | 36 
 2 files changed, 51 insertions(+)
 create mode 100644 reg-tests/lua/h1.lua
 create mode 100644 reg-tests/lua/h1.vtc

diff --git a/reg-tests/lua/h1.lua b/reg-tests/lua/h1.lua
new file mode 100644
index ..999ea887
--- /dev/null
+++ b/reg-tests/lua/h1.lua
@@ -0,0 +1,15 @@
+core.register_action("bug", { "http-res" }, function(txn)
+   data = txn:get_priv()
+   if not data then
+   data = 0
+   end
+   data = data + 1
+   print(string.format("set to %d", data))
+   txn.http:res_set_status(200 + data)
+   txn:set_priv(data)
+end)
+
+core.register_service("fakeserv", "http", function(applet)
+   applet:set_status(200)
+   applet:start_response()
+end)
diff --git a/reg-tests/lua/h1.vtc b/reg-tests/lua/h1.vtc
new file mode 100644
index ..b11f21c9
--- /dev/null
+++ b/reg-tests/lua/h1.vtc
@@ -0,0 +1,36 @@
+varnishtest "Lua: txn:get_priv() scope"
+feature ignore_unknown_macro
+
+server s1 -repeat 2 {
+rxreq
+txresp
+} -start
+
+haproxy h1 -conf {
+global
+lua-load ${testdir}/h1.lua
+
+frontend fe1
+mode http
+bind "fd@${fe1}"
+default_backend b1
+
+http-response lua.bug
+
+backend b1
+mode http
+http-request use-service lua.fakeserv
+} -start
+
+client c0 -connect ${h1_fe1_sock} {
+txreq -url "/"
+rxresp
+expect resp.status == 201
+txreq -url "/"
+rxresp
+expect resp.status == 201
+}
+
+client c0 -start
+
+client c0 -wait
-- 
2.18.0




Re: BUG: LUA txn:get_priv() scoped to connection, not transaction

2018-08-22 Thread Patrick Hemmer


On 2018/8/22 05:16, Thierry Fournier wrote:
> Hi Patrick,
>
> Could you retry adding the keyword “local” before data. Unfortunately,
> by default, Lua variables are global.
>
Makes no difference, still get the same result. I don't think it would
do anything anyway as the `txn:get_priv()` will still return a value,
even if nil, and overwrite whatever is in a previous definition.

>
>> core.register_action("test", { "http-req" }, function(txn)
>> *local*data = txn:get_priv()
>> if not data then
>> data = 0
>> end
>> data = data + 1
>> print(string.format("set to %d", data))
>> txn:set_priv(data)
>> end)
>
> BR,
> Thierry
>
>
>> On 22 Aug 2018, at 05:57, Patrick Hemmer > > wrote:
>>
>> There is a bug in the current stable haproxy (1.8.13) where the LUA
>> function txn:get_priv() is returning data stored from other
>> transactions. This was discovered as we have code that triggers on
>> certain requests, and it was triggering on requests it should not
>> have been.
>>
>> You can reproduce with this config:
>> global
>> lua-load haproxy.lua
>>
>> defaults
>> mode http
>>
>> frontend f1
>> bind :8000
>> default_backend b1
>> http-request lua.test
>>
>> backend b1
>> http-request use-service lua.fakeserv
>>
>> And this lua file:
>> core.register_action("test", { "http-req" }, function(txn)
>> data = txn:get_priv()
>> if not data then
>> data = 0
>> end
>> data = data + 1
>> print(string.format("set to %d", data))
>> txn:set_priv(data)
>> end)
>>
>> core.register_service("fakeserv", "http", function(applet)
>> applet:set_status(200)
>> applet:start_response()
>> end)
>>
>> And this curl command:
>> curl http://localhost:8000 http://localhost:8000
>>
>> Which provides this output:
>> set to 1
>> set to 2
>>
>>
>>
>> Version information:
>> HA-Proxy version 1.8.13 2018/07/30
>> Copyright 2000-2018 Willy Tarreau 
>>
>> Build options :
>> TARGET  = osx
>> CPU = generic
>> CC  = gcc
>> CFLAGS  = -O0 -g -fno-strict-aliasing
>> -Wdeclaration-after-statement -fwrapv -fno-strict-overflow
>> -Wno-address-of-packed-member -Wno-null-dereference -Wno-unused-label
>> OPTIONS = USE_ZLIB=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE=1
>>
>> Default settings :
>> maxconn = 2000, bufsize = 16384, maxrewrite = 1024,
>> maxpollevents = 200
>>
>> Built with OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
>> Running on OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
>> OpenSSL library supports TLS extensions : yes
>> OpenSSL library supports SNI : yes
>> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2
>> Built with Lua version : Lua 5.3.4
>> Built with transparent proxy support using:
>> Encrypted password support via crypt(3): yes
>> Built with PCRE version : 8.42 2018-03-20
>> Running on PCRE version : 8.42 2018-03-20
>> PCRE library supports JIT : no (USE_PCRE_JIT not set)
>> Built with zlib version : 1.2.11
>> Running on zlib version : 1.2.11
>> Compression algorithms supported : identity("identity"),
>> deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
>> Built with network namespace support.
>>
>> Available polling systems :
>>  kqueue : pref=300,  test result OK
>>  poll : pref=200,  test result OK
>>  select : pref=150,  test result OK
>> Total: 3 (3 usable), will use kqueue.
>>
>> Available filters :
>> [SPOE] spoe
>> [COMP] compression
>> [TRACE] trace
>>
>>
>> -Patrick
>



Re: BUG: LUA txn:get_priv() scoped to connection, not transaction

2018-08-22 Thread Thierry Fournier
Hi Patrick,

Could you retry adding the keyword “local” before data. Unfortunately, by 
default, Lua variables are global.


> core.register_action("test", { "http-req" }, function(txn)
> local data = txn:get_priv()
> if not data then
> data = 0
> end
> data = data + 1
> print(string.format("set to %d", data))
> txn:set_priv(data)
> end)


BR,
Thierry


> On 22 Aug 2018, at 05:57, Patrick Hemmer  wrote:
> 
> There is a bug in the current stable haproxy (1.8.13) where the LUA function 
> txn:get_priv() is returning data stored from other transactions. This was 
> discovered as we have code that triggers on certain requests, and it was 
> triggering on requests it should not have been.
> 
> You can reproduce with this config:
> global
> lua-load haproxy.lua
> 
> defaults
> mode http
> 
> frontend f1
> bind :8000
> default_backend b1
> http-request lua.test
> 
> backend b1
> http-request use-service lua.fakeserv
> 
> And this lua file:
> core.register_action("test", { "http-req" }, function(txn)
> data = txn:get_priv()
> if not data then
> data = 0
> end
> data = data + 1
> print(string.format("set to %d", data))
> txn:set_priv(data)
> end)
> 
> core.register_service("fakeserv", "http", function(applet)
> applet:set_status(200)
> applet:start_response()
> end)
> 
> And this curl command:
> curl http://localhost:8000  http://localhost:8000 
> 
> 
> Which provides this output:
> set to 1
> set to 2
> 
> 
> 
> Version information:
> HA-Proxy version 1.8.13 2018/07/30
> Copyright 2000-2018 Willy Tarreau  
> 
> 
> Build options :
> TARGET  = osx
> CPU = generic
> CC  = gcc
> CFLAGS  = -O0 -g -fno-strict-aliasing -Wdeclaration-after-statement 
> -fwrapv -fno-strict-overflow -Wno-address-of-packed-member 
> -Wno-null-dereference -Wno-unused-label
> OPTIONS = USE_ZLIB=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE=1
> 
> Default settings :
> maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 
> 200
> 
> Built with OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
> Running on OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2
> Built with Lua version : Lua 5.3.4
> Built with transparent proxy support using:
> Encrypted password support via crypt(3): yes
> Built with PCRE version : 8.42 2018-03-20
> Running on PCRE version : 8.42 2018-03-20
> PCRE library supports JIT : no (USE_PCRE_JIT not set)
> Built with zlib version : 1.2.11
> Running on zlib version : 1.2.11
> Compression algorithms supported : identity("identity"), 
> deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
> Built with network namespace support.
> 
> Available polling systems :
>  kqueue : pref=300,  test result OK
>  poll : pref=200,  test result OK
>  select : pref=150,  test result OK
> Total: 3 (3 usable), will use kqueue.
> 
> Available filters :
> [SPOE] spoe
> [COMP] compression
> [TRACE] trace
> 
> 
> -Patrick



Re: connection leak (stuck in CLOSE_WAIT) on master

2018-08-22 Thread Willy Tarreau
Hi Patrick,

On Thu, Aug 09, 2018 at 01:01:27AM -0400, Patrick Hemmer wrote:
> There's an issue on current master (287527a) where haproxy is losing
> track of its connections, and they're getting stuck in CLOSE_WAIT. And
> it's counting these connections towards limits (such as maxconn).
> Eventually maxconn is reached and everything starts queuing, and timing out.

Without being certain about it, it looks like it could be related to
the few corner cases we've already met with the start of rework of the
layering, that Olivier fixed yesterday. Your config is running here
fine with 100ms response time and I haven't seen any single sQ in the
logs. However I couldn't see it fail with commit 287527a either, so it
doesn't mean much.

I'm aware there's still one issue with this that I met this morning
and which causes some connections not to terminate, so if you don't
have much time to re-run a test, let's wait for our confirmation that 
it's fixed first.

Thanks,
Willy



Re: haproxy processing request of a disconnected client

2018-08-22 Thread Willy Tarreau
On Thu, Aug 09, 2018 at 01:00:04PM -0400, Patrick Hemmer wrote:
> So I just noticed the behavior that when a request is queued and the
> client closes the connection, once a server slot frees up that request
> is still sent to the server which processes it and sends a response back.
> What's even more interesting is that the log indicates that everything
> was processed normally. It basically says the response was sent back to
> the client just fine.
> 
> Example config:
> global
> stats socket /tmp/haproxy.sock level admin
> defaults
> log 127.0.0.1:1234 daemon
> mode http
> option httplog
> timeout queue 5s
> frontend f1
> bind :8001
> default_backend b1
> backend b1
> server s1 127.0.0.1:8081 maxconn 1
> 
> Log output:
> <30>Aug  9 12:50:40 haproxy[12384]: 127.0.0.1:64723
> [09/Aug/2018:12:50:35.167] f1 b1/s1 0/0/0/5106/5106 200 118 - - 
> 2/2/1/1/0 0/0 "GET /y HTTP/1.1"
> <30>Aug  9 12:50:45 haproxy[12384]: 127.0.0.1:64726
> [09/Aug/2018:12:50:35.526] f1 b1/s1 0/4749/0/5102/9851 200 118 - - 
> 1/1/0/1/0 0/1 "GET /x HTTP/1.1"
> 
> ^ In this, the server sleeps for 5 seconds, and then replies. I sent the
> request for /y, and then a request for /x, but killed the client on the
> /x request after 1 second. The request for /y was processed, but then so
> was the request for /x. The close flags ("") indicate everything
> went fine.

Yes, and it's definitely intentional, so that scripts doing stuff like

  echo -e "GET /check HTTP/1.1\r\nHost: bar\r\n\r\n" | nc 127.0.0.1 80

work properly. I've seen quite a bunch of these in monitoring scripts
over the years. Sometimes it's even more obvious as the response is
really not needed :

  echo -e "GET /set-var?name=X=Y HTTP/1.0\r\n\r\n" > /dev/tcp/127.0.0.1/80

If you want to abort processing of a pending request when the client
closes, you simply need to add "option abortonclose". It's generally
recommended on public-facing configs. In this case you'll see something
like "CQ", "CC" or "CH" in the logs.

Regards
Willy



Re: [PATCH 0/2] sample fetches for available connections

2018-08-22 Thread Willy Tarreau
On Wed, Aug 22, 2018 at 12:03:40AM -0400, Patrick Hemmer wrote:
> Sorry for all the pings, lots of emails which look like they might have
> fallen off the radar.

No problem, you're right, it indeed fell off the radar, being quite busy
on tons of other stuff these days.

Thanks!
Willy



Re: [PATCH 2/2] MINOR: Add srv_conn_free sample fetch

2018-08-22 Thread Willy Tarreau
On Thu, Aug 09, 2018 at 06:46:29PM -0400, Patrick Hemmer wrote:
> 
> This adds the 'srv_conn_free([/])' sample fetch. This fetch
> provides the number of available connections on the designated server.

Fine with this as well, though just like with the previous one, I
disagree with this special case of -1 and would rather only count
the really available connections (i.e. 0 if it's not possible to
use the server).

Willy



Re: [PATCH 1/2] MINOR: add be_conn_free sample fetch

2018-08-22 Thread Willy Tarreau
Hi Patrick,

On Thu, Aug 09, 2018 at 06:46:28PM -0400, Patrick Hemmer wrote:
> 
> This adds the sample fetch 'be_conn_free([])'. This sample fetch
> provides the total number of unused connections across available servers
> in the specified backend.

Thanks for writing this one, I recently figured I needed the same for my
build farm :-)

> +be_conn_free([]) : integer
> +  Returns an integer value corresponding to the number of available 
> connections
> +  across available servers in the backend. Queue slots are not included. 
> Backup
> +  servers are also not included, unless all other servers are down. If no
> +  backend name is specified, the current one is used. But it is also possible
> +  to check another backend. It can be used to use a specific farm when the
> +  nominal one is full. See also the "be_conn" and "connslots" criteria.
> +
> +  OTHER CAVEATS AND NOTES: at this point in time, the code does not take care
> +  of dynamic connections. Also, if any of the server maxconn, or maxqueue is 
> 0,
> +  then this fetch clearly does not make sense, in which case the value 
> returned
> +  will be -1.

I disagree with making a special case above for maxconn 0. In fact for me
it just means that such a server cannot accept connections, so it simply
doesn't count in the sum, just as if it were not present.

> + px = iterator->proxy;
> + if (!srv_currently_usable(iterator) || ((iterator->flags & 
> SRV_F_BACKUP) &&
> + (px->srv_act ||
> + (iterator != 
> px->lbprm.fbck && !(px->options & PR_O_USE_ALL_BK)
> + continue;

Please slightly reorder the condition to improve the indent above for
better readability, for example :

if (!srv_currently_usable(iterator) ||
((iterator->flags & SRV_F_BACKUP) &&
 (px->srv_act || (iterator != px->lbprm.fbck && 
!(px->options & PR_O_USE_ALL_BK)

After checking, I'm OK with the condition :-)

> + if (iterator->maxconn == 0) {
> + /* configuration is stupid */
> + smp->data.u.sint = -1;  /* FIXME: stupid value! */
> + return 1;
> + }
> +
> + smp->data.u.sint += (iterator->maxconn - iterator->cur_sess);

Here I'd simply suggest this to replace the whole block :

if (iterator->maxconn > iterator->cur_sess)
smp->data.u.sint += (iterator->maxconn - 
iterator->cur_sess);

And then it can properly count available connections through all
available servers, regardless of their individual configuration.

Otherwise I'm fine with this patch.

Thanks,
Willy



Re: BUG: LUA txn:get_priv() scoped to connection, not transaction

2018-08-22 Thread Willy Tarreau
CCing Thierry.

On Tue, Aug 21, 2018 at 11:57:52PM -0400, Patrick Hemmer wrote:
> There is a bug in the current stable haproxy (1.8.13) where the LUA
> function txn:get_priv() is returning data stored from other
> transactions. This was discovered as we have code that triggers on
> certain requests, and it was triggering on requests it should not have been.
> 
> You can reproduce with this config:
> global
> lua-load haproxy.lua
> 
> defaults
> mode http
> 
> frontend f1
> bind :8000
> default_backend b1
> http-request lua.test
> 
> backend b1
> http-request use-service lua.fakeserv
> 
> And this lua file:
> core.register_action("test", { "http-req" }, function(txn)
> data = txn:get_priv()
> if not data then
> data = 0
> end
> data = data + 1
> print(string.format("set to %d", data))
> txn:set_priv(data)
> end)
> 
> core.register_service("fakeserv", "http", function(applet)
> applet:set_status(200)
> applet:start_response()
> end)
> 
> And this curl command:
> curl http://localhost:8000 http://localhost:8000
> 
> Which provides this output:
> set to 1
> set to 2
> 
> 
> 
> Version information:
> HA-Proxy version 1.8.13 2018/07/30
> Copyright 2000-2018 Willy Tarreau 
> 
> Build options :
> TARGET  = osx
> CPU = generic
> CC  = gcc
> CFLAGS  = -O0 -g -fno-strict-aliasing
> -Wdeclaration-after-statement -fwrapv -fno-strict-overflow
> -Wno-address-of-packed-member -Wno-null-dereference -Wno-unused-label
> OPTIONS = USE_ZLIB=1 USE_OPENSSL=1 USE_LUA=1 USE_PCRE=1
> 
> Default settings :
> maxconn = 2000, bufsize = 16384, maxrewrite = 1024,
> maxpollevents = 200
> 
> Built with OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
> Running on OpenSSL version : OpenSSL 1.1.0h  27 Mar 2018
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2
> Built with Lua version : Lua 5.3.4
> Built with transparent proxy support using:
> Encrypted password support via crypt(3): yes
> Built with PCRE version : 8.42 2018-03-20
> Running on PCRE version : 8.42 2018-03-20
> PCRE library supports JIT : no (USE_PCRE_JIT not set)
> Built with zlib version : 1.2.11
> Running on zlib version : 1.2.11
> Compression algorithms supported : identity("identity"),
> deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
> Built with network namespace support.
> 
> Available polling systems :
>  kqueue : pref=300,  test result OK
>  poll : pref=200,  test result OK
>  select : pref=150,  test result OK
> Total: 3 (3 usable), will use kqueue.
> 
> Available filters :
> [SPOE] spoe
> [COMP] compression
> [TRACE] trace
> 
> 
> -Patrick



Re: HTTP/2 issues and segfaults with current 1.9-dev [7ee465]

2018-08-22 Thread Cyril Bonté

Hi Willy,

Le 22/08/2018 à 05:42, Willy Tarreau a écrit :

On Wed, Aug 22, 2018 at 04:32:49AM +0200, Willy Tarreau wrote:

Excellent, I think I found it :

 trash.data = recv(conn->handle.fd, trash.area, trash.size,
  MSG_PEEK);
 if (trash.data < 0) {
 if (errno == EINTR)
 continue;
 if (errno == EAGAIN) {
 fd_cant_recv(conn->handle.fd);
 return 0;
 }
...

trash.data is a size_t now so it cannot be negative. Thus it's believed
that recv() never fails. This it's clearly related to the buffer changes.
I'm seeing a few other such places that require using an intermediate
variable for the test. After all it's not that bad because we've inherited
such assignments from a good decade, and it's time to clean this up as well.


So I've now addressed all those I could find (quite a bunch in fact).
I think everything's OK now regarding this. I haven't checked for the
capture yet but since it was already broken in 1.6, it can still wait
a bit ;-)


Great job ! I can't reproduce the issue anymore, even with nbthread > 1 
(I noticed too late it was easier to trigger). We can consider this as 
fixed ;-)

I'll upgrade the haproxy instance on my test server in the morning.

--
Cyril Bonté