From: Christopher Faulet <christopher.faulet@capflam.org>
Date: Tue, 28 Jun 2016 13:54:44 +0000 (+0200)
Subject: BUG/MINOR: Rework slightly commit 9962f8fc to clean code and avoid mistakes
X-Git-Tag: v1.7-dev4~47
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a9300a3d5a068c6e6c0a95f2a2640df557aa0a4c;p=thirdparty%2Fhaproxy.git

BUG/MINOR: Rework slightly commit 9962f8fc to clean code and avoid mistakes

In commit 9962f8fc (BUG/MEDIUM: http: unbreak uri/header/url_param hashing), we
take care to update 'msg->sov' value when the parser changes to state
HTTP_MSG_DONE. This works when no filter is used. But, if a filter is used and
if it loops on 'http_end' callback, the following block is evaluated two times
consecutively:

    if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
            msg->sov -= ret;

Today, in practice, because this happens when all data are parsed and forwarded,
the second test always fails (after the first update, msg->sov is always lower
or equal to 0). But it is useless and error prone. So to avoid misunderstanding
the code has been slightly changed. Now, in all cases, we try to update msg->sov
only once per iteration.

No backport is needed.
---

diff --git a/src/proto_http.c b/src/proto_http.c
index ef5a15ccbc..7d1b678727 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -6850,11 +6850,10 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg)
 			       /* on_error    */ goto error);
 	b_adv(chn->buf, ret);
 	msg->next -= ret;
-	if (msg->next)
-		goto waiting;
-
 	if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
 		msg->sov -= ret;
+	if (msg->next)
+		goto waiting;
 
 	FLT_STRM_DATA_CB(s, chn, flt_http_end(s, msg),
 			 /* default_ret */ 1,
@@ -6870,12 +6869,11 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg)
 			       /* on_error    */ goto error);
 	b_adv(chn->buf, ret);
 	msg->next -= ret;
-
-  waiting:
 	if (!(chn->flags & CF_WROTE_DATA) || msg->sov > 0)
 		msg->sov -= ret;
 	if (!HAS_DATA_FILTERS(s, chn))
 		msg->chunk_len -= channel_forward(chn, msg->chunk_len);
+  waiting:
 	return 0;
   error:
 	return -1;
@@ -6968,11 +6966,10 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
 			  /* on_error    */ goto error);
 	b_adv(chn->buf, ret);
 	msg->next -= ret;
-	if (msg->next)
-		goto waiting;
-
 	if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
 		msg->sov -= ret;
+	if (msg->next)
+		goto waiting;
 
 	FLT_STRM_DATA_CB(s, chn, flt_http_end(s, msg),
 		    /* default_ret */ 1,
@@ -6988,12 +6985,11 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
 			  /* on_error    */ goto error);
 	b_adv(chn->buf, ret);
 	msg->next -= ret;
-
-  waiting:
 	if (!(chn->flags & CF_WROTE_DATA) || msg->sov > 0)
 		msg->sov -= ret;
 	if (!HAS_DATA_FILTERS(s, chn))
 		msg->chunk_len -= channel_forward(chn, msg->chunk_len);
+  waiting:
 	return 0;
 
   chunk_parsing_error: