From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 20 Feb 2025 09:00:31 +0000 (+0100)
Subject: BUG/MEDIUM: applet: Don't handle EOI/EOS/ERROR is applet is waiting for room
X-Git-Tag: v3.2-dev7~87
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=efc46de29457b93f75ae4b581f70a9df8362d6ca;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: applet: Don't handle EOI/EOS/ERROR is applet is waiting for room

The commit 7214dcd52 ("BUG/MEDIUM: applet: Don't pretend to have more data
to handle EOI/EOS/ERROR") introduced a regression. Because of this patch, it
was possible to handle EOI/EOS/ERROR applet flags too early while the applet
was waiting for more room to transfer the last output data.

This bug can be encountered with any applet using its own buffers (cache and
stats for instance). And depending on the configuration and the timing, the
data may be truncated or the stream may be blocked, infinitely or not.
Streams blocked infinitely were observed with the cache applet and the HTTP
compression enabled.

For the record, it is important to detect EOI/EOS/ERROR applet flags to be
able to report the corresponding event on the SE and by transitivity on the
SC. Most of time, this happens when some data should be transferred to the
stream. The .rcv_buf callback function is called and these flags are
properly handled. However, some applets may also report them spontaneously,
outside of any data transfer. In that case, the .rcv_buf callback is not
called.

It is the purpose of this patch (and the one above). Being able to detect
pending EOI/EOS/ERROR applet flags. However, we must be sure to not handle
them too early at this place. When these flags are set, it means no more
data will be produced by the applet. So we must only wait to have
transferred everything to the stream. And this happens when the applet is no
longer waiting for more room.

This patch must be backported to 3.1 with the one above.
---

diff --git a/src/applet.c b/src/applet.c
index 4f3601ccb..fafd7a6d1 100644
--- a/src/applet.c
+++ b/src/applet.c
@@ -926,17 +926,22 @@ struct task *task_process_applet(struct task *t, void *context, unsigned int sta
 
 	sc_applet_sync_recv(sc);
 
-	if (applet_fl_test(app, APPCTX_FL_EOI) && !se_fl_test(app->sedesc, SE_FL_EOI)) {
-		se_fl_set(app->sedesc, SE_FL_EOI);
-		TRACE_STATE("report EOI to SE", APPLET_EV_RECV|APPLET_EV_BLK, app);
-	}
-	if (applet_fl_test(app, APPCTX_FL_EOS) && !se_fl_test(app->sedesc, SE_FL_EOS)) {
-		se_fl_set(app->sedesc, SE_FL_EOS);
-		TRACE_STATE("report EOS to SE", APPLET_EV_RECV|APPLET_EV_BLK, app);
-	}
-	if (applet_fl_test(app, APPCTX_FL_ERROR) && !se_fl_test(app->sedesc, SE_FL_ERROR)) {
-		se_fl_set(app->sedesc, SE_FL_ERROR);
-		TRACE_STATE("report ERROR to SE", APPLET_EV_RECV|APPLET_EV_BLK, app);
+	if (!se_fl_test(app->sedesc, SE_FL_WANT_ROOM)) {
+		/* Handle EOI/EOS/ERROR outside of data transfer. But take care
+		 * there are no pending data. Otherwise, we must wait.
+		 */
+		if (applet_fl_test(app, APPCTX_FL_EOI) && !se_fl_test(app->sedesc, SE_FL_EOI)) {
+			se_fl_set(app->sedesc, SE_FL_EOI);
+			TRACE_STATE("report EOI to SE", APPLET_EV_RECV|APPLET_EV_BLK, app);
+		}
+		if (applet_fl_test(app, APPCTX_FL_EOS) && !se_fl_test(app->sedesc, SE_FL_EOS)) {
+			se_fl_set(app->sedesc, SE_FL_EOS);
+			TRACE_STATE("report EOS to SE", APPLET_EV_RECV|APPLET_EV_BLK, app);
+		}
+		if (applet_fl_test(app, APPCTX_FL_ERROR) && !se_fl_test(app->sedesc, SE_FL_ERROR)) {
+			se_fl_set(app->sedesc, SE_FL_ERROR);
+			TRACE_STATE("report ERROR to SE", APPLET_EV_RECV|APPLET_EV_BLK, app);
+		}
 	}
 
 	/* TODO: May be move in appctx_rcv_buf or sc_applet_process ? */