From 0158bb23d75482f7827ff3bd0a73a6da90881ca5 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfaulet@haproxy.com>
Date: Wed, 1 Jun 2022 17:08:19 +0200
Subject: [PATCH] BUG/MEDIUM: httpclient: Rework CLI I/O handler to handle full
 buffer cases

'httpclient' command does not properly handle full buffer cases. When the
response buffer is full, we exit to retry later. However, the context flags
are updated. It means when this happens, we may loose a part of the
response.

So now, flags are preserved when we fail to push data into the response
buffer. In addition, instead of dumping one part per call, we now try to
dump as much data as possible.

Finally, when there is no more data, because everything was dumped or
because we are waiting for more data from the HTTP client, the applet is
updated accordingly by calling applet_have_no_more_data(). Otherwise, when
some data are blocked, applet_putchk() already takes care to update the SE
flags. So, it is useless to call sc_need_room().

This patch should fix the issue #1723. It must be backported as far as
2.5. But a massive refactoring was performed in 2.6. So, for the 2.5 and
below, the patch will have to be adapted.
---
 src/http_client.c | 46 +++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/http_client.c b/src/http_client.c
index bf01c1b85..677161beb 100644
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -196,32 +196,29 @@ static int hc_cli_io_handler(struct appctx *appctx)
 {
 	struct hcli_svc_ctx *ctx = appctx->svcctx;
 	struct stconn *sc = appctx_sc(appctx);
-	struct buffer *trash = alloc_trash_chunk();
 	struct httpclient *hc = ctx->hc;
 	struct http_hdr *hdrs, *hdr;
 
-	if (!trash)
-		goto out;
-
 	if (ctx->flags & HC_CLI_F_RES_STLINE) {
-		chunk_appendf(trash, "%.*s %d %.*s\n", (unsigned int)istlen(hc->res.vsn), istptr(hc->res.vsn),
-			      hc->res.status, (unsigned int)istlen(hc->res.reason), istptr(hc->res.reason));
-		applet_putchk(appctx, trash);
+		chunk_printf(&trash, "%.*s %d %.*s\n", (unsigned int)istlen(hc->res.vsn), istptr(hc->res.vsn),
+			     hc->res.status, (unsigned int)istlen(hc->res.reason), istptr(hc->res.reason));
+		if (applet_putchk(appctx, &trash) == -1)
+			goto more;
 		ctx->flags &= ~HC_CLI_F_RES_STLINE;
-		goto out;
 	}
 
 	if (ctx->flags & HC_CLI_F_RES_HDR) {
+		chunk_reset(&trash);
 		hdrs = hc->res.hdrs;
 		for (hdr = hdrs; isttest(hdr->v); hdr++) {
-			if (!h1_format_htx_hdr(hdr->n, hdr->v, trash))
-				goto out;
+			if (!h1_format_htx_hdr(hdr->n, hdr->v, &trash))
+				goto too_many_hdrs;
 		}
-		if (!chunk_memcat(trash, "\r\n", 2))
-			goto out;
-		applet_putchk(appctx, trash);
+		if (!chunk_memcat(&trash, "\r\n", 2))
+			goto too_many_hdrs;
+		if (applet_putchk(appctx, &trash) == -1)
+			goto more;
 		ctx->flags &= ~HC_CLI_F_RES_HDR;
-		goto out;
 	}
 
 	if (ctx->flags & HC_CLI_F_RES_BODY) {
@@ -230,10 +227,10 @@ static int hc_cli_io_handler(struct appctx *appctx)
 		ret = httpclient_res_xfer(hc, sc_ib(sc));
 		channel_add_input(sc_ic(sc), ret); /* forward what we put in the buffer channel */
 
-		if (!httpclient_data(hc)) {/* remove the flag if the buffer was emptied */
-			ctx->flags &= ~HC_CLI_F_RES_BODY;
-		}
-		goto out;
+		/* remove the flag if the buffer was emptied */
+		if (httpclient_data(hc))
+			goto more;
+		ctx->flags &= ~HC_CLI_F_RES_BODY;
 	}
 
 	/* we must close only if F_END is the last flag */
@@ -241,16 +238,15 @@ static int hc_cli_io_handler(struct appctx *appctx)
 		sc_shutw(sc);
 		sc_shutr(sc);
 		ctx->flags &= ~HC_CLI_F_RES_END;
-		goto out;
 	}
 
-out:
-	/* we didn't clear every flags, we should come back to finish things */
-	if (ctx->flags)
-		sc_need_room(sc);
-
-	free_trash_chunk(trash);
+more:
+	if (!ctx->flags)
+		applet_have_no_more_data(appctx);
 	return 0;
+
+too_many_hdrs:
+	return cli_err(appctx, "Too many headers.\n");
 }
 
 static void hc_cli_release(struct appctx *appctx)