Hello HAProxy maintainers,

I would appreciate your feedback on a reproducible raw TCP data
truncation issue in the data-filter shutdown path. I am sending this as
an RFC rather than requesting an immediate merge because the shutdown
scope and backport range would benefit from maintainer guidance.

Tested on HAProxy master at commit
46376cfc20bfa62b872dedc58aa566f57729d1f9, version 3.5-dev3.

Summary
-------

When a raw TCP data filter forwards data in partial chunks, the current
output and fast-forward buffers can become temporarily empty while the
filter still retains channel input. If the producer has already sent
FIN, the current shutdown checks may forward FIN before that retained
input becomes sendable. The peer then receives a truncated payload.

Evidence
--------

With the unmodified tree, both raw TCP request bwlim-in and response
bwlim-out tests receive 29768 of 32768 bytes before EOF. A larger 1 MiB
response reproducer receives 1033388 of 1048576 bytes. The same topology
without bwlim delivers the complete payload.

Changing only the two process_stream() shutdown checks fixes the simple
request and response cases, but a full-duplex asynchronous case still
fails in the stconn shutdown path. The complete change passes all three
arms of the new regression test.

Proposed change
---------------

The patch waits for active-filter channel input to drain at four normal
write-shutdown gates:

  - request processing in process_stream();
  - response processing in process_stream();
  - sc_chk_snd();
  - sc_notify().

Existing output and fast-forward drain checks remain in place. Write
timeouts, producer-side errors, and aborts remain forced-shutdown
exceptions.

Validation
----------

  - TARGET=linux-glibc ERR=1 build: passed;
  - make unit-tests: 1 passed, 4 skipped, 0 failed;
  - bwlim-fin.vtc -n 20: 20 passed, 0 skipped, 0 failed;
  - filters selection: 2 passed, 1 skipped, 0 failed;
  - bug/default selection: 133 passed, 47 skipped, 0 failed.

Strict Linux checkpatch.pl --no-signoff reports two CODE_INDENT errors
for continuation alignment that follows HAProxy's documented coding
style, plus one generic new-file MAINTAINERS warning. The
HAProxy-documented checkpatch profile reports no code errors and
retains only the generic MAINTAINERS warning.

Known limitation
----------------

This patch is intentionally limited to raw TCP data filters. An HTTP
request using bwlim-in remains truncated in the same way before and
after this change. I believe that HTX/request-analyzer behavior should
be investigated separately rather than mixed into this raw TCP fix.

Questions
---------

I would particularly appreciate guidance on:

  1. whether these four shutdown gates are the correct scope;
  2. whether BUG/MEDIUM and the stconn area are appropriate;
  3. how far the fix should be backported;
  4. whether the HTTP request/HTX behavior should be reported as a
     separate issue.

If this direction is acceptable, I can follow up with a revised,
merge-ready patch based on the review feedback.

Patch
-----

The exact standard git format-patch is attached as a text/x-patch file,
and the same patch is also included inline below this cover body for
convenient review.

Thank you for your time and feedback.

Ylang

>From 5616a706afaacb145429b9560f63945d25d2c866 Mon Sep 17 00:00:00 2001
From: Ylang <[email protected]>
Date: Thu, 30 Jul 2026 17:35:41 +0800
Subject: [RFC PATCH] BUG/MEDIUM: stconn: defer FIN until filter input drains

In raw TCP mode, a data filter may forward data in partial chunks, so
the currently forwarded output and fast-forward data can be drained
while the filter still retains channel input. If the producer has
already sent FIN, the existing shutdown checks may forward FIN before
that retained input becomes sendable, and the peer receives a truncated
payload.

Wait for the retained channel input to drain whenever an active data
filter is present, at the four normal write-shutdown gates: the request
and response shutdown checks in process_stream(), sc_chk_snd(), and
sc_notify(). Write timeouts, producer-side errors, and aborts remain
forced-shutdown exceptions so a stuck or failed producer cannot block
the close path.

Add a regression test covering raw TCP response bwlim-out, raw TCP
request bwlim-in, and a full-duplex asynchronous trace-filter path that
exercises the stconn shutdown checks. The unmodified tree truncates the
raw payloads, a change limited to process_stream() still fails the
asynchronous case, and the complete change delivers the expected byte
counts before EOF.

This change is validated for raw TCP data filters only. An HTTP request
using bwlim-in remains truncated in the same way before and after this
change; the HTX/request-analyzer case is not addressed here.

Tests:
  - make -j"$(nproc)" TARGET=linux-glibc ERR=1: passed
  - make unit-tests: 1 passed, 4 skipped, 0 failed
  - bwlim-fin.vtc -n 20: 20 passed, 0 skipped, 0 failed
  - filters selection: 2 passed, 1 skipped, 0 failed
  - bug/default selection: 133 passed, 47 skipped, 0 failed

The appropriate backport range has not been established yet. Maintainer
guidance on how far this fix should be backported would be appreciated.
---
 reg-tests/filters/bwlim-fin.vtc | 76 +++++++++++++++++++++++++++++++++
 src/stconn.c                    |  9 +++-
 src/stream.c                    | 10 ++++-
 3 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 reg-tests/filters/bwlim-fin.vtc

diff --git a/reg-tests/filters/bwlim-fin.vtc b/reg-tests/filters/bwlim-fin.vtc
new file mode 100644
index 000000000..36dee375b
--- /dev/null
+++ b/reg-tests/filters/bwlim-fin.vtc
@@ -0,0 +1,76 @@
+varnishtest "Data filters must drain channel input before forwarding FIN"
+#REGTEST_TYPE=bug
+
+feature ignore_unknown_macro
+
+server s_response {
+        recv 1
+        send_n 2048 "0123456789abcdef"
+} -start
+
+server s_request {
+        recv 32768
+        expect_close
+} -start
+
+# Keep request traffic flowing while the server half-closes its response. This
+# makes the response shutdown run through the asynchronous stconn paths while
+# the trace filter still holds response input.
+server s_async {
+        recv 64
+        send_n 256 "response-unique-"
+        shutdown -write
+        recv 32704
+} -start
+
+haproxy h1 -conf {
+    global
+        tune.bufsize 4096
+    .if feature(THREAD)
+        nbthread 1
+    .endif
+
+    defaults
+        mode tcp
+        timeout connect    "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client     "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server     "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client-fin "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server-fin "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen response
+        bind "fd@${response}"
+        filter bwlim-out output default-limit 8k default-period 100ms
+        tcp-response content set-bandwidth-limit output
+        server response ${s_response_addr}:${s_response_port}
+
+    listen request
+        bind "fd@${request}"
+        filter bwlim-in input default-limit 8k default-period 100ms
+        tcp-request content set-bandwidth-limit input
+        server request ${s_request_addr}:${s_request_port}
+
+    listen async
+        bind "fd@${async}"
+        filter trace name async max-fwd 1 quiet
+        server async ${s_async_addr}:${s_async_port}
+} -start
+
+client c_response -connect ${h1_response_sock} {
+        send "X"
+        recv 32768
+        expect_close
+} -run
+
+client c_request -connect ${h1_request_sock} {
+        send_n 2048 "0123456789abcdef"
+} -run
+
+client c_async -connect ${h1_async_sock} {
+        send_n 2048 "request-traffic-"
+        recv 4096
+        expect_close
+} -run
+
+server s_request -wait
+server s_async -wait
diff --git a/src/stconn.c b/src/stconn.c
index 8dcdd4970..7d257b1b8 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -793,6 +793,9 @@ static inline void sc_chk_snd(struct stconn *sc)
                         */
                        if ((oc->flags & CF_AUTO_CLOSE) &&
                            ((sc->flags &
(SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED) &&
+                           (!HAS_DATA_FILTERS(__sc_strm(sc), oc) ||
channel_input_data(oc) == 0 ||
+                            (oc->flags & CF_WRITE_TIMEOUT) ||
+                            (sc_opposite(sc)->flags &
(SC_FL_ERROR|SC_FL_ABRT_WANTED))) &&
                            sc_state_in(sc->state, SC_SB_RDY|SC_SB_EST)) {
                                sc_shutdown(sc);
                                goto out_wakeup;
@@ -939,7 +942,11 @@ void sc_notify(struct stconn *sc)
                struct connection *conn = sc_conn(sc);

                if (((sc->flags & (SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED))
== SC_FL_SHUT_WANTED) &&
-                   (sc->state == SC_ST_EST) && (!conn ||
!(conn->flags & (CO_FL_WAIT_XPRT | CO_FL_EARLY_SSL_HS))))
+                   (sc->state == SC_ST_EST) &&
+                   (!HAS_DATA_FILTERS(__sc_strm(sc), oc) ||
channel_input_data(oc) == 0 ||
+                    (oc->flags & CF_WRITE_TIMEOUT) ||
+                    (sco->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED))) &&
+                   (!conn || !(conn->flags & (CO_FL_WAIT_XPRT |
CO_FL_EARLY_SSL_HS))))
                        sc_shutdown(sc);
        }

diff --git a/src/stream.c b/src/stream.c
index 76414565d..d11a478df 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -2576,7 +2576,10 @@ struct task *process_stream(struct task *t,
void *context, unsigned int state)

        /* shutdown(write) pending */
        if (unlikely((scb->flags &
(SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED &&
-                    ((!co_data(req) && !sc_ep_have_ff_data(scb)) ||
(req->flags & CF_WRITE_TIMEOUT)))) {
+                    (((!co_data(req) && !sc_ep_have_ff_data(scb)) &&
+                      (!HAS_DATA_FILTERS(s, req) ||
channel_input_data(req) == 0 ||
+                       (scf->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED)))) ||
+                     (req->flags & CF_WRITE_TIMEOUT)))) {
                if (scf->flags & SC_FL_ERROR)
                        scb->flags |= SC_FL_NOLINGER;
                sc_shutdown(scb);
@@ -2684,7 +2687,10 @@ struct task *process_stream(struct task *t,
void *context, unsigned int state)

        /* shutdown(write) pending */
        if (unlikely((scf->flags &
(SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED &&
-                    ((!co_data(res) && !sc_ep_have_ff_data(scf)) ||
(res->flags & CF_WRITE_TIMEOUT)))) {
+                    (((!co_data(res) && !sc_ep_have_ff_data(scf)) &&
+                      (!HAS_DATA_FILTERS(s, res) ||
channel_input_data(res) == 0 ||
+                       (scb->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED)))) ||
+                     (res->flags & CF_WRITE_TIMEOUT)))) {
                sc_shutdown(scf);
        }

--
2.34.1
From 5616a706afaacb145429b9560f63945d25d2c866 Mon Sep 17 00:00:00 2001
From: Ylang <[email protected]>
Date: Thu, 30 Jul 2026 17:35:41 +0800
Subject: [RFC PATCH] BUG/MEDIUM: stconn: defer FIN until filter input drains

In raw TCP mode, a data filter may forward data in partial chunks, so
the currently forwarded output and fast-forward data can be drained
while the filter still retains channel input. If the producer has
already sent FIN, the existing shutdown checks may forward FIN before
that retained input becomes sendable, and the peer receives a truncated
payload.

Wait for the retained channel input to drain whenever an active data
filter is present, at the four normal write-shutdown gates: the request
and response shutdown checks in process_stream(), sc_chk_snd(), and
sc_notify(). Write timeouts, producer-side errors, and aborts remain
forced-shutdown exceptions so a stuck or failed producer cannot block
the close path.

Add a regression test covering raw TCP response bwlim-out, raw TCP
request bwlim-in, and a full-duplex asynchronous trace-filter path that
exercises the stconn shutdown checks. The unmodified tree truncates the
raw payloads, a change limited to process_stream() still fails the
asynchronous case, and the complete change delivers the expected byte
counts before EOF.

This change is validated for raw TCP data filters only. An HTTP request
using bwlim-in remains truncated in the same way before and after this
change; the HTX/request-analyzer case is not addressed here.

Tests:
  - make -j"$(nproc)" TARGET=linux-glibc ERR=1: passed
  - make unit-tests: 1 passed, 4 skipped, 0 failed
  - bwlim-fin.vtc -n 20: 20 passed, 0 skipped, 0 failed
  - filters selection: 2 passed, 1 skipped, 0 failed
  - bug/default selection: 133 passed, 47 skipped, 0 failed

The appropriate backport range has not been established yet. Maintainer
guidance on how far this fix should be backported would be appreciated.
---
 reg-tests/filters/bwlim-fin.vtc | 76 +++++++++++++++++++++++++++++++++
 src/stconn.c                    |  9 +++-
 src/stream.c                    | 10 ++++-
 3 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 reg-tests/filters/bwlim-fin.vtc

diff --git a/reg-tests/filters/bwlim-fin.vtc b/reg-tests/filters/bwlim-fin.vtc
new file mode 100644
index 000000000..36dee375b
--- /dev/null
+++ b/reg-tests/filters/bwlim-fin.vtc
@@ -0,0 +1,76 @@
+varnishtest "Data filters must drain channel input before forwarding FIN"
+#REGTEST_TYPE=bug
+
+feature ignore_unknown_macro
+
+server s_response {
+        recv 1
+        send_n 2048 "0123456789abcdef"
+} -start
+
+server s_request {
+        recv 32768
+        expect_close
+} -start
+
+# Keep request traffic flowing while the server half-closes its response. This
+# makes the response shutdown run through the asynchronous stconn paths while
+# the trace filter still holds response input.
+server s_async {
+        recv 64
+        send_n 256 "response-unique-"
+        shutdown -write
+        recv 32704
+} -start
+
+haproxy h1 -conf {
+    global
+        tune.bufsize 4096
+    .if feature(THREAD)
+        nbthread 1
+    .endif
+
+    defaults
+        mode tcp
+        timeout connect    "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client     "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server     "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client-fin "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server-fin "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen response
+        bind "fd@${response}"
+        filter bwlim-out output default-limit 8k default-period 100ms
+        tcp-response content set-bandwidth-limit output
+        server response ${s_response_addr}:${s_response_port}
+
+    listen request
+        bind "fd@${request}"
+        filter bwlim-in input default-limit 8k default-period 100ms
+        tcp-request content set-bandwidth-limit input
+        server request ${s_request_addr}:${s_request_port}
+
+    listen async
+        bind "fd@${async}"
+        filter trace name async max-fwd 1 quiet
+        server async ${s_async_addr}:${s_async_port}
+} -start
+
+client c_response -connect ${h1_response_sock} {
+        send "X"
+        recv 32768
+        expect_close
+} -run
+
+client c_request -connect ${h1_request_sock} {
+        send_n 2048 "0123456789abcdef"
+} -run
+
+client c_async -connect ${h1_async_sock} {
+        send_n 2048 "request-traffic-"
+        recv 4096
+        expect_close
+} -run
+
+server s_request -wait
+server s_async -wait
diff --git a/src/stconn.c b/src/stconn.c
index 8dcdd4970..7d257b1b8 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -793,6 +793,9 @@ static inline void sc_chk_snd(struct stconn *sc)
 			 */
 			if ((oc->flags & CF_AUTO_CLOSE) &&
 			    ((sc->flags & (SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED) &&
+			    (!HAS_DATA_FILTERS(__sc_strm(sc), oc) || channel_input_data(oc) == 0 ||
+			     (oc->flags & CF_WRITE_TIMEOUT) ||
+			     (sc_opposite(sc)->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED))) &&
 			    sc_state_in(sc->state, SC_SB_RDY|SC_SB_EST)) {
 				sc_shutdown(sc);
 				goto out_wakeup;
@@ -939,7 +942,11 @@ void sc_notify(struct stconn *sc)
 		struct connection *conn = sc_conn(sc);
 
 		if (((sc->flags & (SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED) &&
-		    (sc->state == SC_ST_EST) && (!conn || !(conn->flags & (CO_FL_WAIT_XPRT | CO_FL_EARLY_SSL_HS))))
+		    (sc->state == SC_ST_EST) &&
+		    (!HAS_DATA_FILTERS(__sc_strm(sc), oc) || channel_input_data(oc) == 0 ||
+		     (oc->flags & CF_WRITE_TIMEOUT) ||
+		     (sco->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED))) &&
+		    (!conn || !(conn->flags & (CO_FL_WAIT_XPRT | CO_FL_EARLY_SSL_HS))))
 			sc_shutdown(sc);
 	}
 
diff --git a/src/stream.c b/src/stream.c
index 76414565d..d11a478df 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -2576,7 +2576,10 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
 
 	/* shutdown(write) pending */
 	if (unlikely((scb->flags & (SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED &&
-		     ((!co_data(req) && !sc_ep_have_ff_data(scb)) || (req->flags & CF_WRITE_TIMEOUT)))) {
+		     (((!co_data(req) && !sc_ep_have_ff_data(scb)) &&
+		       (!HAS_DATA_FILTERS(s, req) || channel_input_data(req) == 0 ||
+		        (scf->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED)))) ||
+		      (req->flags & CF_WRITE_TIMEOUT)))) {
 		if (scf->flags & SC_FL_ERROR)
 			scb->flags |= SC_FL_NOLINGER;
 		sc_shutdown(scb);
@@ -2684,7 +2687,10 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
 
 	/* shutdown(write) pending */
 	if (unlikely((scf->flags & (SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED &&
-		     ((!co_data(res) && !sc_ep_have_ff_data(scf)) || (res->flags & CF_WRITE_TIMEOUT)))) {
+		     (((!co_data(res) && !sc_ep_have_ff_data(scf)) &&
+		       (!HAS_DATA_FILTERS(s, res) || channel_input_data(res) == 0 ||
+		        (scb->flags & (SC_FL_ERROR|SC_FL_ABRT_WANTED)))) ||
+		      (res->flags & CF_WRITE_TIMEOUT)))) {
 		sc_shutdown(scf);
 	}
 
-- 
2.34.1

Reply via email to