[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#859490: unblock: haproxy/1.7.5-1 (pre-approval)



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Dear Release Team,

We would like to upload HAProxy 1.7.5 to unstable and have it migrate to 
testing. Upstream says:

    Released version 1.7.5 with the following main changes :
        - BUG/MEDIUM: peers: fix buffer overflow control in intdecode.
        - BUG/MEDIUM: buffers: Fix how input/output data are injected into buffers
        - BUG/MEDIUM: http: Fix blocked HTTP/1.0 responses when compression is enabled
        - BUG/MINOR: filters: Don't force the stream's wakeup when we wait in flt_end_analyze
        - DOC: fix parenthesis and add missing "Example" tags
        - DOC: update the contributing file
        - DOC: log-format/tcplog/httplog update
        - MINOR: config parsing: add warning when log-format/tcplog/httplog is overriden in "defaults" sections

Some background on the bugs fixed can be found at 
https://www.mail-archive.com/haproxy@formilux.org/msg25574.html

The code (src/ + include/) diffstat is as follows:

 include/common/buffer.h | 60 ++++++++++++++++++++++++++++++++++++++----------------------
 src/cfgparse.c          | 38 ++++++++++++++++++++++++++++++++++++++
 src/channel.c           |  6 +++---
 src/filters.c           |  7 +------
 src/peers.c             | 49 +++++++++++++++++++++++++++----------------------
 src/proto_http.c        | 24 +++++++++++++++---------
 6 files changed, 122 insertions(+), 62 deletions(-)

See also the attached diff.

Can we proceed with the upload?

Regards,
Apollon

unblock haproxy/1.7.5-1
diff --git a/include/common/buffer.h b/include/common/buffer.h
index ce3eb40a..3a6dfd7f 100644
--- a/include/common/buffer.h
+++ b/include/common/buffer.h
@@ -156,6 +156,41 @@ static inline int bo_contig_data(const struct buffer *b)
 	return b->o;
 }
 
+/* Return the amount of bytes that can be written into the input area at once
+ * including reserved space which may be overwritten (this is the caller
+ * responsibility to know if the reserved space is protected or not).
+*/
+static inline int bi_contig_space(const struct buffer *b)
+{
+	const char *left, *right;
+
+	left  = bi_end(b);
+	right = bo_ptr(b);
+
+	if (left >= right)
+		right = b->data + b->size;
+
+	return (right - left);
+}
+
+/* Return the amount of bytes that can be written into the output area at once
+ * including reserved space which may be overwritten (this is the caller
+ * responsibility to know if the reserved space is protected or not). Input data
+ * are assumed to not exist.
+*/
+static inline int bo_contig_space(const struct buffer *b)
+{
+	const char *left, *right;
+
+	left  = bo_end(b);
+	right = bo_ptr(b);
+
+	if (left >= right)
+		right = b->data + b->size;
+
+	return (right - left);
+}
+
 /* Return the buffer's length in bytes by summing the input and the output */
 static inline int buffer_len(const struct buffer *buf)
 {
@@ -226,21 +261,6 @@ static inline int buffer_contig_area(const struct buffer *buf, const char *start
 	return count;
 }
 
-/* Return the amount of bytes that can be written into the buffer at once,
- * including reserved space which may be overwritten.
- */
-static inline int buffer_contig_space(const struct buffer *buf)
-{
-	const char *left, *right;
-
-	if (buf->data + buf->o <= buf->p)
-		right = buf->data + buf->size;
-	else
-		right = buf->p + buf->size - buf->o;
-
-	left = buffer_wrap_add(buf, buf->p + buf->i);
-	return right - left;
-}
 
 /* Returns the amount of byte that can be written starting from <p> into the
  * input buffer at once, including reserved space which may be overwritten.
@@ -340,17 +360,13 @@ static inline void bi_fast_delete(struct buffer *buf, int n)
 	buf->p += n;
 }
 
-/*
- * Tries to realign the given buffer, and returns how many bytes can be written
- * there at once without overwriting anything.
- */
-static inline int buffer_realign(struct buffer *buf)
+/* Tries to realign the given buffer. */
+static inline void buffer_realign(struct buffer *buf)
 {
 	if (!(buf->i | buf->o)) {
 		/* let's realign the buffer to optimize I/O */
 		buf->p = buf->data;
 	}
-	return buffer_contig_space(buf);
 }
 
 /* Schedule all remaining buffer data to be sent. ->o is not touched if it
@@ -402,7 +418,7 @@ static inline int bo_putblk(struct buffer *b, const char *blk, int len)
 	if (!len)
 		return 0;
 
-	half = buffer_contig_space(b);
+	half = bo_contig_space(b);
 	if (half > len)
 		half = len;
 
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 074d5e67..95e95f62 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -4792,6 +4792,21 @@ stats_error_parsing:
 				if (alertif_too_many_args_idx(1, 1, file, linenum, args, &err_code))
 					goto out;
 			}
+			if (curproxy->conf.logformat_string && curproxy == &defproxy) {
+				char *oldlogformat = "log-format";
+				char *clflogformat = "";
+
+				if (curproxy->conf.logformat_string == default_http_log_format)
+					oldlogformat = "option httplog";
+				else if (curproxy->conf.logformat_string == default_tcp_log_format)
+					oldlogformat = "option tcplog";
+				else if (curproxy->conf.logformat_string == clf_http_log_format)
+					oldlogformat = "option httplog clf";
+				if (logformat == clf_http_log_format)
+					clflogformat = " clf";
+				Warning("parsing [%s:%d]: 'option httplog%s' overrides previous '%s' in 'defaults' section.\n",
+				        file, linenum, clflogformat, oldlogformat);
+			}
 			if (curproxy->conf.logformat_string != default_http_log_format &&
 			    curproxy->conf.logformat_string != default_tcp_log_format &&
 			    curproxy->conf.logformat_string != clf_http_log_format)
@@ -4803,6 +4818,18 @@ stats_error_parsing:
 			curproxy->conf.lfs_line = curproxy->conf.args.line;
 		}
 		else if (!strcmp(args[1], "tcplog")) {
+			if (curproxy->conf.logformat_string && curproxy == &defproxy) {
+				char *oldlogformat = "log-format";
+
+				if (curproxy->conf.logformat_string == default_http_log_format)
+					oldlogformat = "option httplog";
+				else if (curproxy->conf.logformat_string == default_tcp_log_format)
+					oldlogformat = "option tcplog";
+				else if (curproxy->conf.logformat_string == clf_http_log_format)
+					oldlogformat = "option httplog clf";
+				Warning("parsing [%s:%d]: 'option tcplog' overrides previous '%s' in 'defaults' section.\n",
+				        file, linenum, oldlogformat);
+			}
 			/* generate a detailed TCP log */
 			if (curproxy->conf.logformat_string != default_http_log_format &&
 			    curproxy->conf.logformat_string != default_tcp_log_format &&
@@ -6046,7 +6073,18 @@ stats_error_parsing:
 			err_code |= ERR_ALERT | ERR_FATAL;
 			goto out;
 		}
+		if (curproxy->conf.logformat_string && curproxy == &defproxy) {
+			char *oldlogformat = "log-format";
 
+			if (curproxy->conf.logformat_string == default_http_log_format)
+				oldlogformat = "option httplog";
+			else if (curproxy->conf.logformat_string == default_tcp_log_format)
+				oldlogformat = "option tcplog";
+			else if (curproxy->conf.logformat_string == clf_http_log_format)
+				oldlogformat = "option httplog clf";
+			Warning("parsing [%s:%d]: 'log-format' overrides previous '%s' in 'defaults' section.\n",
+			        file, linenum, oldlogformat);
+		}
 		if (curproxy->conf.logformat_string != default_http_log_format &&
 		    curproxy->conf.logformat_string != default_tcp_log_format &&
 		    curproxy->conf.logformat_string != clf_http_log_format)
diff --git a/src/channel.c b/src/channel.c
index fe65895c..02280728 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -91,8 +91,8 @@ int bo_inject(struct channel *chn, const char *msg, int len)
 		return -2;
 	}
 
-	max = buffer_realign(chn->buf);
-
+	buffer_realign(chn->buf);
+	max = bo_contig_space(chn->buf);
 	if (len > max)
 		return max;
 
@@ -166,7 +166,7 @@ int bi_putblk(struct channel *chn, const char *blk, int len)
 		return 0;
 
 	/* OK so the data fits in the buffer in one or two blocks */
-	max = buffer_contig_space(chn->buf);
+	max = bi_contig_space(chn->buf);
 	memcpy(bi_end(chn->buf), blk, MIN(len, max));
 	if (len > max)
 		memcpy(chn->buf->data, blk + max, len - max);
diff --git a/src/filters.c b/src/filters.c
index 3d556454..8be16636 100644
--- a/src/filters.c
+++ b/src/filters.c
@@ -843,12 +843,7 @@ flt_end_analyze(struct stream *s, struct channel *chn, unsigned int an_bit)
 		/* Remove backend filters from the list */
 		flt_stream_release(s, 1);
 	}
-	else {
-		/* This analyzer ends only for one channel. So wake up the
-		 * stream to be sure to process it for the other side as soon as
-		 * possible. */
-		task_wakeup(s->task, TASK_WOKEN_MSG);
-	}
+
 	return ret;
 }
 
diff --git a/src/peers.c b/src/peers.c
index 1a280a57..ed6f7362 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -174,6 +174,12 @@ enum {
 struct peers *peers = NULL;
 static void peer_session_forceshutdown(struct appctx *appctx);
 
+/* This function encode an uint64 to 'dynamic' length format.
+   The encoded value is written at address *str, and the
+   caller must assure that size after *str is large enought.
+   At return, the *str is set at the next Byte after then
+   encoded integer. The function returns then length of the
+   encoded integer in Bytes */
 int intencode(uint64_t i, char **str) {
 	int idx = 0;
 	unsigned char *msg;
@@ -205,36 +211,35 @@ int intencode(uint64_t i, char **str) {
    *str point on the beginning of the integer to decode
    at the end of decoding *str point on the end of the
    encoded integer or to null if end is reached */
-uint64_t intdecode(char **str, char *end) {
-	uint64_t i;
-	int idx = 0;
+uint64_t intdecode(char **str, char *end)
+{
 	unsigned char *msg;
+	uint64_t i;
+	int shift;
 
 	if (!*str)
 		return 0;
 
 	msg = (unsigned char *)*str;
-	if (msg >= (unsigned char *)end) {
-		*str = NULL;
-		return 0;
+	if (msg >= (unsigned char *)end)
+		goto fail;
+
+	i = *(msg++);
+	if (i >= 240) {
+		shift = 4;
+		do {
+			if (msg >= (unsigned char *)end)
+				goto fail;
+			i += (uint64_t)*msg << shift;
+			shift += 7;
+		} while (*(msg++) >= 128);
 	}
+        *str = (char *)msg;
+        return i;
 
-	if (msg[idx] < 240) {
-		*str = (char *)&msg[idx+1];
-		return msg[idx];
-	}
-	i = msg[idx];
-	do {
-		idx++;
-		if (msg >= (unsigned char *)end) {
-			*str = NULL;
-			return 0;
-		}
-		i += (uint64_t)msg[idx] <<  (4 + 7*(idx-1));
-	}
-	while (msg[idx] >= 128);
-	*str = (char *)&msg[idx+1];
-	return i;
+  fail:
+        *str = NULL;
+        return 0;
 }
 
 /* Set the stick-table UPDATE message type byte at <msg_type> address,
diff --git a/src/proto_http.c b/src/proto_http.c
index f4a57e97..bfc56604 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -6827,11 +6827,9 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s
 	}
 
  skip_header_mangling:
-	if ((msg->flags & HTTP_MSGF_XFER_LEN) || HAS_DATA_FILTERS(s, rep) ||
-	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_TUN) {
-		rep->analysers &= ~AN_RES_FLT_XFER_DATA;
-		rep->analysers |= AN_RES_HTTP_XFER_BODY;
-	}
+	/* Always enter in the body analyzer */
+	rep->analysers &= ~AN_RES_FLT_XFER_DATA;
+	rep->analysers |= AN_RES_HTTP_XFER_BODY;
 
 	/* if the user wants to log as soon as possible, without counting
 	 * bytes from the server, then this is the right moment. We have
@@ -6974,11 +6972,11 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit
 
 	/* 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 CF_DONTCLOSE. Similarly, if the body length is undefined, if
-	 * keep-alive is set on the client side or if there are filters
-	 * registered on the stream, we don't want to forward a close
+	 * set CF_DONTCLOSE. Similarly, if keep-alive is set on the client side
+	 * or if there are filters registered on the stream, we don't want to
+	 * forward a close
 	 */
-	if ((msg->flags & HTTP_MSGF_TE_CHNK) || !(msg->flags & HTTP_MSGF_XFER_LEN) ||
+	if ((msg->flags & HTTP_MSGF_TE_CHNK) ||
 	    HAS_DATA_FILTERS(s, res) ||
 	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||
 	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL)
@@ -6995,6 +6993,14 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit
 	if ((msg->flags & HTTP_MSGF_TE_CHNK) || (msg->flags & HTTP_MSGF_COMPRESSING))
 		res->flags |= CF_EXPECT_MORE;
 
+	/* If there is neither content-length, nor transfer-encoding header
+	 * _AND_ there is no data filtering, we can safely forward all data
+	 * indefinitely. */
+	if (!(msg->flags & HTTP_MSGF_XFER_LEN) && !HAS_DATA_FILTERS(s, res)) {
+		buffer_flush(res->buf);
+		channel_forward_forever(res);
+	}
+
 	/* the stream handler will take care of timeouts and errors */
 	return 0;
 

Reply to: