From: Willy Tarreau <w@1wt.eu>
Date: Sat, 28 Aug 2010 16:57:20 +0000 (+0200)
Subject: [BUG] http: don't set auto_close if more data are expected
X-Git-Tag: v1.5-dev8~480
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=92aa1fac0a45fe4b6a834d8e3cf24ec24e793ac2;p=thirdparty%2Fhaproxy.git

[BUG] http: don't set auto_close if more data are expected

Fix 4fe41902789d188ee4c23b14a7cdbf075463b158 was a bit too strong. It
has caused some chunked-encoded responses to be truncated when a recv()
call could return multiple chunks followed by a close. The reason is
that when a chunk is parsed, only its contents are scheduled to be
forwarded. Thus, the reader sees auto_close+shutr and sets shutw_now.
The sender in turn sends the last scheduled data and does shutw().

Another nasty effect is that it has reduced the keep-alive rate. If
a response did not completely fit into the buffer, then the auto_close
bit was left on and the sender would close upon completion.

The fix consists in not making use of auto_close when chunked encoding
is used nor when keep-alive is used, which makes sense. However it is
maintained on error processing.

Thanks to Cyril Bonté for reporting the issue early.
---

diff --git a/src/proto_http.c b/src/proto_http.c
index d737cfe543..865eaa4945 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4248,7 +4248,9 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit)
 		else {
 			/* other states, DONE...TUNNEL */
 			/* for keep-alive we don't want to forward closes on DONE */
-			buffer_dont_close(req);
+			if ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||
+			    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL)
+				buffer_dont_close(req);
 			if (http_resync_states(s)) {
 				/* some state changes occurred, maybe the analyser
 				 * was disabled too.
@@ -4292,6 +4294,12 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit)
 		goto return_bad_req;
 	}
 
+	/* When TE: chunked is used, we need to get there again to parse remaining
+	 * chunks even if the client has closed, so we don't want to set BF_DONTCLOSE.
+	 */
+	if (txn->flags & TX_REQ_TE_CHNK)
+		buffer_dont_close(req);
+
 	http_silent_debug(__LINE__, s);
 	return 0;
 
@@ -5172,7 +5180,9 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
 		else {
 			/* other states, DONE...TUNNEL */
 			/* for keep-alive we don't want to forward closes on DONE */
-			buffer_dont_close(res);
+			if ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||
+			    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL)
+				buffer_dont_close(res);
 			if (http_resync_states(s)) {
 				http_silent_debug(__LINE__, s);
 				/* some state changes occurred, maybe the analyser
@@ -5208,6 +5218,16 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
 		msg->som = msg->sov;
 	}
 
+	/* When TE: chunked is used, we need to get there again to parse remaining
+	 * chunks even if the server has closed, so we don't want to set BF_DONTCLOSE.
+	 * Similarly, with keep-alive on the client side, we don't want to forward a
+	 * close.
+	 */
+	if ((txn->flags & TX_RES_TE_CHNK) ||
+	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||
+	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL)
+		buffer_dont_close(res);
+
 	/* the session handler will take care of timeouts and errors */
 	http_silent_debug(__LINE__, s);
 	return 0;