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

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



Your message dated Wed, 05 Apr 2017 16:58:00 +0000
with message-id <086799bb-38c2-29db-d83d-f83c4152f236@thykier.net>
and subject line Re: Bug#859490: unblock: haproxy/1.7.5-1 (pre-approval)
has caused the Debian Bug report #859490,
regarding unblock: haproxy/1.7.5-1 (pre-approval)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
859490: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=859490
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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;
 

--- End Message ---
--- Begin Message ---
Apollon Oikonomopoulos:
> Control: tags -1 - moreinfo
> 
> Hi,
> 
> On 13:40 Wed 05 Apr     , Niels Thykier wrote:
>> Ack, please go ahead and remove the moreinfo tag once the upload has
>> been built successfully on all relevant release architectures.
> 
> Uploaded and built everywhere, thanks!
> 
> I'm also attaching the full source debdiff for reference.
> 
> Regards,
> Apollon
> 

Unblocked, thanks.

~Niels

--- End Message ---

Reply to: