Le 12/12/2018 à 10:52, Christopher Faulet a écrit :
Le 12/12/2018 à 02:08, PiBa-NL a écrit :
Hi List,

Didn't have time yet to bisect when it went wrong. But attached testfile
produces the following output after 3 curl requests at different speeds,
this seems to trigger a problem as the hash of the downloaded content is
nolonger the same as it should be, (in my actual environment its a 2MB
javascript file that comes from a iis server behind haproxy.). Took
already a few hours more than desired to come up with a seemingly
reliable reproduction.
1.9-dev10 is the first one i put on my production environment as i think
release is imminent so it 'should' be pretty stable ;), (yes i know..i
shouldn't assume..) before it was using 1.8.14.. So was quick to revert
to that 1.8 again :).

Using these settings:

       compression algo gzip
       compression type text/html text/plain application/json
application/javascript
       compression offload
When these compression settings are disabled, it completes successfully..

**** top   2.4 shell_cmd|      exit 1
**** top   2.4 shell_cmd|    fi
**** top   2.5 shell_out|File1 all OK
**** top   2.5 shell_out|File2 not OK 7798551c02a37ce89c77fc18fc415e5b
**** top   2.5 shell_out|File3 not OK 3146c4c9fce4da750558bfd9387ffc3b
**** top   2.5 shell_status = 0x0001
---- top   2.5 shell_exit not as expected: got 0x0001 wanted 0x0000
*    top   2.5 RESETTING after ./PB-TEST/ulticompres/b00005.vtc
**   h1    2.5 Reset and free h1 haproxy 51853
**   h1    2.5 Wait
**   h1    2.5 Stop HAproxy pid=51853
**** h1    2.5 STDOUT poll 0x11
**** h1    2.5 Kill(2)=0: No error: 0
**   h1    2.6 WAIT4 pid=51853 status=0x0002 (user 0.253496 sys 0.000000)
*    top   2.6 TEST ./PB-TEST/ulticompres/b00005.vtc FAILED
#    top  TEST ./PB-TEST/ulticompres/b00005.vtc FAILED (2.581) exit=2

haproxy -v
HA-Proxy version 1.9-dev10-3815b22 2018/12/11
Copyright 2000-2018 Willy Tarreau <[email protected]>

Can anyone confirm? Or perhaps even fix ;) Ill try and dig a little more
tomorrow evening :).

Hi Pieter,

There is a bug with the commit 3815b22 when the message is chunked. I
need to see with Willy how to fix it. BTW, if you revert this commit, it
should work fine.


Ok, Finally it was not too hard to fix (hoping it does not break anything else...). Here is a patch. Pieter, could you try it to see if it works for you ?

--
Christopher
>From 00ebd09575fd7451033b78c394f04476b79b9aa4 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Wed, 12 Dec 2018 10:32:09 +0100
Subject: [PATCH] BUG/MEDIUM: mux-h1: Fix the zero-copy on output for chunked
 messages

The commit 3815b227f ("MEDIUM: mux-h1: implement true zero-copy of DATA blocks")
broke the output of chunked messages. When the zero-copy was performed on such
messages, no chunk size was emitted nor ending CRLF.

Now, the chunked envelope is added when necessary. We have at least the size of
the struct htx to emit it. So 40 bytes for now. It should be enough.
---
 src/mux_h1.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/src/mux_h1.c b/src/mux_h1.c
index baaf0dfed..da5d5c17e 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -839,6 +839,34 @@ static void h1_capture_bad_message(struct h1c *h1c, struct h1s *h1s,
 			    &ctx, h1_show_error_snapshot);
 }
 
+/* Emit the chunksize followed by a CRLF in front of data of the buffer
+ * <buf>. It goes backwards and starts with the byte before the buffer's
+ * head. The caller is responsible for ensuring there is enough room left before
+ * the buffer's head for the string.
+ */
+static void h1_emit_chunk_size(struct buffer *buf, size_t chksz)
+{
+	char *beg, *end;
+
+	beg = end = b_head(buf);
+	*--beg = '\n';
+	*--beg = '\r';
+	do {
+		*--beg = hextab[chksz & 0xF];
+	} while (chksz >>= 4);
+	buf->head -= (end - beg);
+	b_add(buf, end - beg);
+}
+
+/* Emit a CRLF after the data of the buffer <buf>. The caller is responsible for
+ * ensuring there is enough room left in the buffer for the string. */
+static void h1_emit_chunk_crlf(struct buffer *buf)
+{
+	*(b_peek(buf, b_data(buf)))     = '\r';
+	*(b_peek(buf, b_data(buf) + 1)) = '\n';
+	b_add(buf, 2);
+}
+
 /*
  * Parse HTTP/1 headers. It returns the number of bytes parsed if > 0, or 0 if
  * it couldn't proceed. Parsing errors are reported by setting H1S_F_*_ERROR
@@ -1389,6 +1417,16 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 
 			buf->area = old_area;
 			buf->data = buf->head = 0;
+
+			/* The message is chunked. We need to emit the chunk
+			 * size. We have at least the size of the struct htx to
+			 * write the chunk envelope. It should be enough.
+			 */
+			if (h1m->flags & H1_MF_CHNK) {
+				h1_emit_chunk_size(&h1c->obuf, count);
+				h1_emit_chunk_crlf(&h1c->obuf);
+			}
+
 			total += count;
 			goto out;
 		}
-- 
2.19.2

Reply via email to