BUG/MAJOR: mux-h2: Preserve raw data on aborted frontend tunnels

Since the commit 523112b72 ("MEDIUM: mux-h2: Use RAW_DATA blocks to store
tunneled data"), raw data received after a tunnel abort on client side were
not properly handled. These data were added as plain HTTP data, triggering a
BUG_ON() because it is invalid by definition. Instead they must be handled
as true raw data from the HTX point of view.

So now, we properly use RAW_DATA for aborted tunnels on the frontend. The
backend must continue to use regular DATA blocks for the rejection response,
whose payload and trailers remain valid. This matches the existing handling
of trailers and END_STREAM on aborted tunnels.

A regression test was added with early tunnel data left pending, then more
data received after the rejection headers while the response is still
pending.  It reproduces the HTX assertion without this fix.

It is a 3.5-specific issue. No backport is needed.

diff --git a/reg-tests/http-messaging/connect_aborted_inflight.vtc b/reg-tests/http-messaging/connect_aborted_inflight.vtc
new file mode 100644 (file)
index 0000000..a21152f
--- /dev/null
+++ b/reg-tests/http-messaging/connect_aborted_inflight.vtc
@@ -0,0 +1,71 @@
+# A CONNECT request ends at its headers, even when the tunnel is subsequently
+# rejected. DATA frames received after the rejection must still carry raw
+# tunnel data, not HTTP payload. Otherwise, transferring them into a request
+# buffer whose message already ended triggers an HTX assertion.
+
+varnishtest "In-flight H2 tunnel data after a CONNECT rejection"
+feature ignore_unknown_macro
+
+barrier b0 cond 2
+barrier b1 cond 2
+
+server s1 {
+       rxreq
+       expect req.method == "CONNECT"
+       barrier b0 sync
+       # Keep the response unfinished while the client sends more tunnel data.
+       txresp -status 403 -hdr "content-length: 6" -nolen
+       barrier b1 sync
+       send "denied"
+       expect_close
+} -start
+
+haproxy hap -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+       mode http
+       timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+       timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+       timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen fe
+       bind "fd@${fe}" proto h2
+       server s1 "${s1_addr}:${s1_port}"
+} -start
+
+client c1 -connect ${hap_fe_sock} {
+       txpri
+       stream 0 {
+               txsettings
+               rxsettings
+               txsettings -ack
+               rxsettings
+       } -run
+
+       stream 1 {
+               txreq -noadd \
+                 -litHdr inc plain ":method" plain "CONNECT" \
+                 -litHdr inc plain ":authority" plain "example.com:443" \
+                 -nostrend
+               # Leave raw data pending in the request buffer, preserving its EOM.
+               txdata -data "early tunnel data" -nostrend
+               barrier b0 sync
+               rxwinup
+               rxwinup
+               rxhdrs
+               expect resp.status == 403
+
+               # Simulate data already in flight when the rejection was sent.
+               # Wait for its window update before allowing the response to end.
+               txdata -data "late tunnel data" -nostrend
+               rxwinup
+               delay 0.1
+               barrier b1 sync
+               rxdata
+               expect resp.body == "denied"
+       } -run
+} -run

diff --git a/src/mux_h2.c b/src/mux_h2.c
index ce246353e31d2aec392d011e984533bb30fb07be..09a1012635e12be720376f0ca094e5eca7451891 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -6612,9 +6612,14 @@ try_again:
 
        /* Tunneled data must never be mixed with HTTP data. A dedicated block
         * type is used for them. This way, the end of the HTTP message, reported
-        * just after the headers for a tunneled stream, is preserved.
+        * just after the headers for a tunneled stream, is preserved. On the
+        * request side, this remains true after a tunnel attempt was aborted:
+        * some tunneled data may still be in flight after the early end of the
+        * message was reported. On the response side, an aborted tunnel carries
+        * a regular HTTP response instead.
         */
-       if (h2s->flags & H2_SF_BODY_TUNNEL)
+       if ((h2s->flags & H2_SF_BODY_TUNNEL) ||
+           (!(h2c->flags & H2_CF_IS_BACK) && (h2s->flags & H2_SF_TUNNEL_ABRT)))
                sent = htx_add_raw_data(htx, ist2(b_head(&h2c->dbuf), flen));
        else
                sent = htx_add_data(htx, ist2(b_head(&h2c->dbuf), flen));