From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 20 Apr 2020 12:58:38 +0000 (+0200)
Subject: BUG/MINOR: http-ana: Throw a 500 error if after-response ruleset fails on errors
X-Git-Tag: v2.2-dev7~188
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8d945d6dd94fe6d46a9720b0be6a8a15ec5fe549;p=thirdparty%2Fhaproxy.git

BUG/MINOR: http-ana: Throw a 500 error if after-response ruleset fails on errors

It is the intended behaviour. But because of a bug, the 500 error resulting of a
rewrite failure during http-after-response ruleset evaluation is also
rewritten. So if at this step, if there is also a rewrite error, the session is
closed and no error message is returned.

Instead, we must be sure to not evaluate the http-after-response rules on an
error message if it is was thrown because of a rewrite failure on a previous
error message.

It is a 2.2-dev2+ bug. No need to backport. This patch should fix the issue
---

diff --git a/src/http_ana.c b/src/http_ana.c
index e0fe67e248..d475cd24de 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -4557,8 +4557,9 @@ static void http_end_response(struct stream *s)
 /* Forward a response generated by HAProxy (error/redirect/return). This
  * function forwards all pending incoming data. If <final> is set to 0, nothing
  * more is performed. It is used for 1xx informational messages. Otherwise, the
- * transaction is terminated and the request is emptied. On success 1 is
- * returned. If an error occurred, 0 is returned.
+ * transaction is terminated and the request is emptied. if <final> is greater
+ * than 1, it means after-response ruleset must not be evaluated. On success 1
+ * is returned. If an error occurred, 0 is returned.
  */
 int http_forward_proxy_resp(struct stream *s, int final)
 {
@@ -4569,7 +4570,7 @@ int http_forward_proxy_resp(struct stream *s, int final)
 
 	if (final) {
 		htx->flags |= HTX_FL_PROXY_RESP;
-		if (!http_eval_after_res_rules(s))
+		if (final == 1 && !http_eval_after_res_rules(s))
 			return 0;
 
 		channel_auto_read(req);
@@ -4602,6 +4603,9 @@ void http_server_error(struct stream *s, struct stream_interface *si, int err,
 
 void http_reply_and_close(struct stream *s, short status, const struct buffer *msg)
 {
+	int final = 1;
+
+  retry:
 	channel_auto_read(&s->req);
 	channel_abort(&s->req);
 	channel_auto_close(&s->req);
@@ -4623,9 +4627,16 @@ void http_reply_and_close(struct stream *s, short status, const struct buffer *m
 		FLT_STRM_CB(s, flt_http_reply(s, s->txn->status, msg));
 		htx = htx_from_buf(&chn->buf);
 		if (channel_htx_copy_msg(chn, htx, msg)) {
-			if (!http_forward_proxy_resp(s, 1) && s->txn->status != 500) {
+			if (!http_forward_proxy_resp(s, final)) {
+				/* On error, return a 500 error message, but
+				 * don't rewrite it if it is already an internal
+				 * error.
+				 */
+				if (s->txn->status == 500)
+					final++;
 				s->txn->status = 500;
-				http_reply_and_close(s, s->txn->status, http_error_message(s));
+				msg = http_error_message(s);
+				goto retry;
 			}
 		}
 	}