Re: [PATCH] MAJOR: filters: Add filters support

2016-09-23 Thread Willy Tarreau 
On Fri, Sep 23, 2016 at 12:53:21AM +0100, Bertrand Jacquin wrote:
> > Thanks for all these information. It helps me to find the bug. I
> > attached a patch. Could you check if it fixes your bug ?
> 
> Thanks a lot for this, I confirm this patch this the issue I was observing.

Great, thanks guys, I've now merged it.

Willy

Re: [PATCH] MAJOR: filters: Add filters support

2016-09-22 Thread Bertrand Jacquin 

Hi Christopher,

On 22/09/2016 14:59, Christopher Faulet wrote:

Le 22/09/2016 à 04:05, Bertrand Jacquin a écrit :

On Tue, Sep 20, 2016 at 08:16:09AM +0200, Willy Tarreau wrote:

Hi Bertrand,

On Tue, Sep 20, 2016 at 12:13:32AM +0100, Bertrand Jacquin wrote:

And finally, If you can share with me your HA and
Nginx configurations, this could help.


I'm attaching a strip down version of haproxy/nginx/php-fpm on which 
I

can reproduice this issue.


I think another thing would be extremely useful, it would be a 
full-packet
network capture between haproxy and nginx so that we have the full 
headers,
the chunk sizes (if any) and the response timing, which generally 
matters a
lot. Ideally the dump in text (or binary) format in a distinct file 
using

curl -i would be nice as well.


Sure thing, you will find attached a tcpdump between haproxy and nginx
along with a curl output.

I changed initial configuration to use TCP instead of UNIX socket for
capture purpose. I also removed the proxy protocol between haproxy and
nginx to get an easier output to parse.


Thanks for all these information. It helps me to find the bug. I
attached a patch. Could you check if it fixes your bug ?


Thanks a lot for this, I confirm this patch this the issue I was 
observing.


Cheers,

--
Bertrand

Re: [PATCH] MAJOR: filters: Add filters support

2016-09-22 Thread Christopher Faulet 
Le 22/09/2016 à 04:05, Bertrand Jacquin a écrit :
> On Tue, Sep 20, 2016 at 08:16:09AM +0200, Willy Tarreau wrote:
>> Hi Bertrand,
>>
>> On Tue, Sep 20, 2016 at 12:13:32AM +0100, Bertrand Jacquin wrote:
 And finally, If you can share with me your HA and
 Nginx configurations, this could help.
>>>
>>> I'm attaching a strip down version of haproxy/nginx/php-fpm on which I
>>> can reproduice this issue.
>>
>> I think another thing would be extremely useful, it would be a full-packet
>> network capture between haproxy and nginx so that we have the full headers,
>> the chunk sizes (if any) and the response timing, which generally matters a
>> lot. Ideally the dump in text (or binary) format in a distinct file using
>> curl -i would be nice as well.
> 
> Sure thing, you will find attached a tcpdump between haproxy and nginx
> along with a curl output.
> 
> I changed initial configuration to use TCP instead of UNIX socket for
> capture purpose. I also removed the proxy protocol between haproxy and
> nginx to get an easier output to parse.
> 

Hi Bertrand,

Thanks for all these information. It helps me to find the bug. I
attached a patch. Could you check if it fixes your bug ?

Willy, if Bertrand confirms that his bug is gone, and if everything is
ok for you, you will be able to merge it.

-- 
Christopher Faulet
>From d756fba92d1c14ab80c2c962843ddd556c5135ea Mon Sep 17 00:00:00 2001
From: Christopher Faulet 
Date: Thu, 22 Sep 2016 15:31:43 +0200
Subject: [PATCH] BUG: http/compression: Fix how chunked data are copied during
 the HTTP body parsing

When the compression is enable on HTTP responses, the chunked data are copied in
a temporary buffer during the HTTP body parsing and then compressed when
everything is forwarded to the client. But the amout of data that can be copied
was not correctly calculated. In many cases, it worked, else on the edge when
the channel buffer was almost full.
---
 src/flt_http_comp.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 9ddc858..249ccdf 100644
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -177,15 +177,17 @@ comp_http_data(struct stream *s, struct filter *filter, struct http_msg *msg)
 	}
 
 	if (msg->flags & HTTP_MSGF_TE_CHNK) {
-		int block = bi_contig_data(buf);
+		int block;
 
 		len = MIN(tmpbuf->size - buffer_len(tmpbuf), len);
-		if (len > block) {
-			memcpy(bi_end(tmpbuf), b_ptr(buf, *nxt), block);
-			memcpy(bi_end(tmpbuf)+block, buf->data, len - block);
-		}
-		else
-			memcpy(bi_end(tmpbuf), b_ptr(buf, *nxt), len);
+
+		b_adv(buf, *nxt);
+		block = bi_contig_data(buf);
+		memcpy(bi_end(tmpbuf), bi_ptr(buf), block);
+		if (len > block)
+			memcpy(bi_end(tmpbuf)+block, buf->data, len-block);
+		b_rew(buf, *nxt);
+
 		tmpbuf->i += len;
 		ret= len;
 	}
-- 
2.7.4

Re: [PATCH] MAJOR: filters: Add filters support

2016-09-20 Thread Willy Tarreau 
Hi Bertrand,

On Tue, Sep 20, 2016 at 12:13:32AM +0100, Bertrand Jacquin wrote:
> > And finally, If you can share with me your HA and
> > Nginx configurations, this could help.
> 
> I'm attaching a strip down version of haproxy/nginx/php-fpm on which I
> can reproduice this issue.

I think another thing would be extremely useful, it would be a full-packet
network capture between haproxy and nginx so that we have the full headers,
the chunk sizes (if any) and the response timing, which generally matters a
lot. Ideally the dump in text (or binary) format in a distinct file using
curl -i would be nice as well.

Thanks!
Willy

Re: [PATCH] MAJOR: filters: Add filters support

2016-09-19 Thread Bertrand Jacquin 
On Mon, Sep 19, 2016 at 10:08:32AM +0200, Christopher Faulet wrote:
> Le 18/09/2016 à 04:17, Bertrand Jacquin a écrit :
> > Today I noticed data corruption when haproxy is used for compression
> > offloading. I bisected twice, and it lead to this specific commit but
> > I'm not 100% confident this commit is the actual root cause.
> > 
> > HTTP body coming from the nginx backend is consistent, but HTTP headers
> > are different depending on the setup I'm enabling. Data corruption only
> > happens with transfer encoding chunked. HTTP body coming then from
> > haproxy to curl can be randomly corrupted, I attached a diff
> > (v1.7-dev1-50-gd7c9196ae56e.Transfer-Encoding-chunked.diff) revealing an
> > unrelated blob like TLS structure in the middle of the javascript. For
> > example, you will find my x509 client certificate in there
> > 
> > I'm also attaching HTTP headers from haproxy to nginx may that help.
> > 
> > Note that I tested with zlib 1.2.8 and libslz 1.0.0, result remains the
> > same in both case.
> 
> I've done some tests. Unfortunately, I'm unable to reproduce the bug. So
> I need more information. First, I need to known how you hit it. Is this
> happen under load or randomly when you do a single request ?

I can reproduce this issue with 100% accuracy on arm or amd64:

  $ for (( i = 0 ; i < 25 ; i++ )) ; do
  curl -s -H 'Accept-Encoding: gzip' \
'https://pants-off.xyz/v1.7-dev1-50-gd7c9196ae56e.js' \
  | zcat | md5sum
done
  01a32fcef0a6894caf112c1a9d5c2a5d  -
  b2a109843f4c43fcde3cb051e4fbf8d2  -
  dedc59fb28ae5d91713234e3e5c08dec  -
  3c8f6d8d53c0ab36bb464b8283570355  -
  e1957e16479bc3106adc68fee2019be8  -
  4cc54367717e5adcdf940f619949ea72  -
  bf637a26e62582c35da6888a4928d4ec  -
  3eeecd478f8e6ea4d690c70f9444954a  -
  79ab805209777ab02bdc6fb829048c74  -
  2aaf9577c1fefdd107a5173aee270c83  -
  .. and so on, shrinking the output here

Note that md5sum of the file should be a4d8bb8ba2a76d7caf090ab632708d7d.

> Then, do
> you still have the bug when you are not using SSL ? Let me also know how
> often the bug appear.

I did not do that test since is was easy for me to track output
containing details about my x509 client certificate.

Running same test as before with 100 iterations, counting similar
output.

  $ for (( i = 0 ; i < 100 ; i++ )) ; do
  curl -s -H 'Accept-Encoding: gzip' \
'http://pants-off.xyz/v1.7-dev1-50-gd7c9196ae56e.js' \
  | zcat | md5sum
done | uniq -c
  1 6c38ef6556efa9e0fa6825803679b2f2  -
 99 a4d8bb8ba2a76d7caf090ab632708d7d  -

Note that 6c38ef6556efa9e0fa6825803679b2f2 appears for the first
iteration. Second test after a few seconds.

  1 ffaf62147b43f82d587df59c39b48e54  -
 29 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 ae6e4404422b93c9fe64bffdea87f36d  -
 41 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 3e8c507e16733af8b728e229c00f21c3  -
  4 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 f6195005b050edcb5ca682b1cde9777f  -
 22 a4d8bb8ba2a76d7caf090ab632708d7d  -

Third test:

  1 6c38ef6556efa9e0fa6825803679b2f2  -
 80 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 3e8c507e16733af8b728e229c00f21c3  -
 18 a4d8bb8ba2a76d7caf090ab632708d7d  -

So it looks a bit more stable. Now if if query HTTP and HTTPS at the
same time, here is what I get:

HTTPS:
  2 17bfe6f7f6296cc5e1d623381afc9e55  -
  1 cbc1779ce5636c31bcf3ea175088da11  -
  1 52ba63995295f5399ddd91b9f9bdf81d  -
  1 5b4115080f35ac5f564b7164a3ada701  -
  1 adfb87fe9efc33e0218a891b2b8b4d42  -
  1 a6f8707556b2f760d20b51dd59b11fb4  -
  .. and so on

HTTP:
  1 3a794f99df4f7a282f822bbaca508852  -
  1 24242f218d9041383c523984d19feddc  -
  2 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 9987d0621c7fbe4b399e462f421b2157  -
  1 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 e261d9cdf988c4fd3d75877812fa5028  -
  .. and so on

Yet it does not look stable. Let's do a test with HTTP only from 2
different hosts:

HTTP client 1:
  1 64cd299604d1f7fac29ef7b2b623b1d0  -
  6 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 bd0372d30c564925ebd1866cf2476474  -
 11 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 64cd299604d1f7fac29ef7b2b623b1d0  -
  9 a4d8bb8ba2a76d7caf090ab632708d7d  -

HTTP client 2:
  1 8749926476d446ead3bd8d81523330eb  -
 16 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 c533c33a3ff469086bdbb6a936e2  -
 14 a4d8bb8ba2a76d7caf090ab632708d7d  -
  1 bd89ab7eab271b2ac13dff42e8e96ba4  -

We are again in a less stable situation.

> And finally, If you can share with me your HA and
> Nginx configurations, this could help.

I'm attaching a strip down version of haproxy/nginx/php-fpm on which I
can reproduice this issue.

Cheers,

-- 
Bertrand


v1.7-dev1-50-gd7c9196ae56e.tgz
Description: GNU Unix tar archive


signature.asc
Description: Digital signature

Re: [PATCH] MAJOR: filters: Add filters support

2016-09-19 Thread Christopher Faulet 
Le 18/09/2016 à 04:17, Bertrand Jacquin a écrit :
> Today I noticed data corruption when haproxy is used for compression
> offloading. I bisected twice, and it lead to this specific commit but
> I'm not 100% confident this commit is the actual root cause.
> 
> HTTP body coming from the nginx backend is consistent, but HTTP headers
> are different depending on the setup I'm enabling. Data corruption only
> happens with transfer encoding chunked. HTTP body coming then from
> haproxy to curl can be randomly corrupted, I attached a diff
> (v1.7-dev1-50-gd7c9196ae56e.Transfer-Encoding-chunked.diff) revealing an
> unrelated blob like TLS structure in the middle of the javascript. For
> example, you will find my x509 client certificate in there
> 
> I'm also attaching HTTP headers from haproxy to nginx may that help.
> 
> Note that I tested with zlib 1.2.8 and libslz 1.0.0, result remains the
> same in both case.

Hi Bertrand,

I've done some tests. Unfortunately, I'm unable to reproduce the bug. So
I need more information. First, I need to known how you hit it. Is this
happen under load or randomly when you do a single request ? Then, do
you still have the bug when you are not using SSL ? Let me also know how
often the bug appear. And finally, If you can share with me your HA and
Nginx configurations, this could help.

Thanks,
-- 
Christopher

Re: [PATCH] MAJOR: filters: Add filters support

2016-09-18 Thread Christopher Faulet 
On 18/09/2016 04:17, Bertrand Jacquin wrote:
> Hi Christopher and Willy,
> 
> Today I noticed data corruption when haproxy is used for compression
> offloading. I bisected twice, and it lead to this specific commit but
> I'm not 100% confident this commit is the actual root cause.
> 
> HTTP body coming from the nginx backend is consistent, but HTTP headers
> are different depending on the setup I'm enabling. Data corruption only
> happens with transfer encoding chunked. HTTP body coming then from
> haproxy to curl can be randomly corrupted, I attached a diff
> (v1.7-dev1-50-gd7c9196ae56e.Transfer-Encoding-chunked.diff) revealing an
> unrelated blob like TLS structure in the middle of the javascript. For
> example, you will find my x509 client certificate in there
> 
> I'm also attaching HTTP headers from haproxy to nginx may that help.
> 
> Note that I tested with zlib 1.2.8 and libslz 1.0.0, result remains the
> same in both case.
> 
> Here is the setup I am using:
> 
>   HA-Proxy version 1.7-dev4-41d5e3a 2016/08/14
>   Copyright 2000-2016 Willy Tarreau 
> 
>   Build options :
> TARGET  = linux2628
> CPU = generic
> CC  = armv7a-hardfloat-linux-gnueabi-gcc
> CFLAGS  = -march=native -O2 -pipe -fomit-frame-pointer 
> -fno-strict-aliasing
> OPTIONS = USE_LIBCRYPT=1 USE_GETADDRINFO=1 USE_SLZ=1 USE_OPENSSL=1 
> USE_PCRE=1 USE_PCRE_JIT=1
> 
>   Default settings :
> maxconn = 2000, bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
> 
>   Encrypted password support via crypt(3): yes
>   Built with libslz for stateless compression.
>   Compression algorithms supported : identity("identity"), 
> deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
>   Built with OpenSSL version : OpenSSL 1.0.2h  3 May 2016
>   Running on OpenSSL version : OpenSSL 1.0.2h  3 May 2016
>   OpenSSL library supports TLS extensions : yes
>   OpenSSL library supports SNI : yes
>   OpenSSL library supports prefer-server-ciphers : yes
>   Built with PCRE version : 8.38 2015-11-23
>   PCRE library supports JIT : yes
>   Built without Lua support
>   Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT 
> IP_FREEBIND
> 
>   Available polling systems :
> epoll : pref=300,  test result OK
>  poll : pref=200,  test result OK
>select : pref=150,  test result OK
>   Total: 3 (3 usable), will use epoll.
> 
>   Available filters :
>   [COMP] compression
>   [TRACE] trace
> 

Hi Bertrand,

I will investigate.

-- 
Christopher