You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ez...@apache.org on 2026/10/19 22:03:34 UTC
(trafficserver) 30/30: [9.2.x] Fix a SIGSEGV in UnixNetVConnection::net_write_io on dechunked POST responses
This is an automated email from the ASF dual-hosted git repository.
ezelkow1 pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit b21b71a127effa270248a1f517d7313eaad08865
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Oct 7 13:51:55 2026 +0900
[9.2.x] Fix a SIGSEGV in UnixNetVConnection::net_write_io on dechunked POST responses
Co-authored-by: mattyw <gh...@mattyw.net>
---
iocore/net/UnixNetVConnection.cc | 12 +++
.../dechunk_post_half_close.test.py | 106 ++++++++++++++++++++
.../dechunk_post_half_close_client.py | 79 +++++++++++++++
.../dechunk_post_half_close_origin.py | 108 +++++++++++++++++++++
4 files changed, 305 insertions(+)
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 15e041aa2a..c73b9b4cdd 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -360,6 +360,7 @@ void
write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
{
NetState *s = &vc->write;
+ Continuation *c = s->vio.cont;
ProxyMutex *mutex = thread->mutex.get();
MUTEX_TRY_LOCK(lock, s->vio.mutex, thread);
@@ -544,6 +545,17 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
read_reschedule(nh, vc);
}
+ // A POST half-close in the handler shut down the write side; nothing is left to write.
+ if (!buf.reader()) {
+ return;
+ }
+
+ // The handler replaced the write VIO while writes were enabled, so nothing else requeues this VC.
+ if (c != s->vio.cont) {
+ write_reschedule(nh, vc);
+ return;
+ }
+
if (!(buf.reader()->is_read_avail_more_than(0))) {
write_disable(nh, vc);
return;
diff --git a/tests/gold_tests/chunked_encoding/dechunk_post_half_close.test.py b/tests/gold_tests/chunked_encoding/dechunk_post_half_close.test.py
new file mode 100644
index 0000000000..404261644b
--- /dev/null
+++ b/tests/gold_tests/chunked_encoding/dechunk_post_half_close.test.py
@@ -0,0 +1,106 @@
+'''
+Regression test: a nested half-close must not leave net_write_io on a cleared write VIO.
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+from ports import get_port
+
+Test.Summary = __doc__
+# Each method drives its own ATS, so a crash in one does not mask the other.
+Test.ContinueOnFail = True
+
+# Larger than the client socket buffers, yet read by ATS in one 32KB block.
+BODY_SIZE = 24000
+# The last-chunk must arrive in its own read while the client still has a backlog;
+# sent with the body, it is parsed together with body bytes and the crash path is skipped.
+LAST_CHUNK_HOLD = 0.5
+CLIENT_STALL = 1.0
+
+
+class DechunkHalfCloseTest:
+ '''Drive a dechunked response whose last-chunk is parsed from a client WRITE_READY.
+
+ The origin sends the whole body in one read, then the last-chunk on its own.
+ The client stalls with a small receive buffer, so the last-chunk arrives while
+ the user-agent consumer still has a backlog. With water_mark 1 the chunked
+ throttle parks it unparsed. When the client drains, net_write_io signals
+ WRITE_READY, and HttpTunnel::consumer_handler synthesizes a READ_READY that
+ parses the last-chunk. That emits no body bytes, so local_finish_all sees
+ nbytes == ndone and delivers WRITE_COMPLETE to tunnel_handler_ua on the same
+ stack. For a POST on a closing connection that half-closes the client, and
+ do_io_shutdown(IO_SHUTDOWN_WRITE) clears the write VIO net_write_io is
+ still running on.
+
+ GET takes a full do_io_close instead of the half-close, so it is the control.
+ '''
+
+ def __init__(self, method, expect_half_close):
+ self.method = method
+ self.name = f"dechunk-{method.lower()}-half-close"
+ self.expect_half_close = expect_half_close
+ self.setupOrigin()
+ self.setupTS()
+
+ def setupOrigin(self):
+ script = os.path.join(Test.TestDirectory, "dechunk_post_half_close_origin.py")
+ self.server = Test.Processes.Process(f"{self.name}-server")
+ self.port = get_port(self.server, "Port")
+ self.server.Command = f"python3 {script} {self.port} --body-size {BODY_SIZE} --hold {LAST_CHUNK_HOLD}"
+ self.server.Ready = When.PortOpen(self.port)
+ self.server.ReturnCode = Any(None, 0, -2, -9, -15)
+
+ def setupTS(self):
+ self.ts = Test.MakeATSProcess(f"ts-{self.name}")
+ self.ts.Disk.records_config.update(
+ {
+ "proxy.config.diags.debug.enabled": 1,
+ "proxy.config.diags.debug.tags": "http_tunnel|http_cs",
+ # Dechunk for the client, which also closes the connection.
+ "proxy.config.http.chunking_enabled": 0,
+ # Any backlog trips the chunked throttle.
+ "proxy.config.http.default_buffer_water_mark": 1,
+ # One 32KB block, so the whole body is read at once.
+ "proxy.config.http.default_buffer_size": 8,
+ # Keep the client socket small enough that the body backs up.
+ "proxy.config.net.sock_send_buffer_size_in": 4096,
+ })
+ self.ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self.port}/")
+
+ if self.expect_half_close:
+ self.ts.Disk.traffic_out.Content = Testers.ContainsExpression(
+ "session half close", "the POST must take the client half-close path")
+ else:
+ self.ts.Disk.traffic_out.Content = Testers.ExcludesExpression(
+ "session half close", "the GET must not take the client half-close path")
+
+ def run(self):
+ client = os.path.join(Test.TestDirectory, "dechunk_post_half_close_client.py")
+ tr = Test.AddTestRun(f"{self.name}: dechunked response with a withheld last-chunk")
+ tr.Processes.Default.StartBefore(self.server)
+ tr.Processes.Default.StartBefore(self.ts)
+ tr.Command = f"python3 {client} {self.ts.Variables.port} --method {self.method} --stall {CLIENT_STALL} --sip 512 --interval 0.02"
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.TimeOut = 90
+ tr.Processes.Default.Streams.stdout = Testers.ContainsExpression("status: HTTP/1.1 200", "the response must be served")
+ tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
+ f"body bytes: {BODY_SIZE}", "the full dechunked body must reach the client")
+ tr.StillRunningAfter = self.ts
+
+
+DechunkHalfCloseTest("POST", expect_half_close=True).run()
+DechunkHalfCloseTest("GET", expect_half_close=False).run()
diff --git a/tests/gold_tests/chunked_encoding/dechunk_post_half_close_client.py b/tests/gold_tests/chunked_encoding/dechunk_post_half_close_client.py
new file mode 100644
index 0000000000..c14de4e815
--- /dev/null
+++ b/tests/gold_tests/chunked_encoding/dechunk_post_half_close_client.py
@@ -0,0 +1,79 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+'''
+Client that sends a request, then drains the response one small sip at a time.
+
+Reading a few bytes per interval keeps continuous backpressure on ATS's
+client-side buffer, so it stays above the water mark while the origin's
+last-chunk arrives and the chunked throttle parks it unparsed. Prints the status
+line and the body length it received.
+'''
+
+import argparse
+import socket
+import sys
+import time
+
+
+def main():
+ ap = argparse.ArgumentParser()
+ ap.add_argument("port", type=int)
+ ap.add_argument("--ip", default="127.0.0.1")
+ ap.add_argument("--method", default="POST")
+ ap.add_argument("--stall", type=float, default=1.0, help="seconds to wait before the first read")
+ ap.add_argument("--sip", type=int, default=512, help="bytes to read per interval")
+ ap.add_argument("--interval", type=float, default=0.02, help="seconds between sips")
+ ap.add_argument("--rcvbuf", type=int, default=2048)
+ args = ap.parse_args()
+
+ sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, args.rcvbuf)
+ sock.settimeout(30)
+ sock.connect((args.ip, args.port))
+
+ body = b"data" if args.method == "POST" else b""
+ request = (
+ f"{args.method} /dechunk HTTP/1.1\r\n"
+ f"Host: www.example.com\r\n"
+ f"Content-Length: {len(body)}\r\n"
+ f"Connection: close\r\n"
+ f"\r\n").encode() + body
+ sock.sendall(request)
+
+ time.sleep(args.stall)
+
+ response = b""
+ try:
+ while True:
+ chunk = sock.recv(args.sip)
+ if not chunk:
+ break
+ response += chunk
+ time.sleep(args.interval)
+ except (ConnectionResetError, socket.timeout) as e:
+ print(f"read error: {e!r}")
+ sock.close()
+
+ head, sep, payload = response.partition(b"\r\n\r\n")
+ status = head.split(b"\r\n", 1)[0].decode(errors="replace") if head else "<no response>"
+ print(f"status: {status}")
+ print(f"body bytes: {len(payload) if sep else 0}")
+ return 0
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/tests/gold_tests/chunked_encoding/dechunk_post_half_close_origin.py b/tests/gold_tests/chunked_encoding/dechunk_post_half_close_origin.py
new file mode 100644
index 0000000000..266bae2210
--- /dev/null
+++ b/tests/gold_tests/chunked_encoding/dechunk_post_half_close_origin.py
@@ -0,0 +1,108 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+'''
+Origin that answers with one chunk of body, then withholds the last-chunk.
+
+The body arrives in a single read so ATS dechunks all of it at once. The
+terminal "0\\r\\n\\r\\n" is sent --hold seconds later, on its own, so ATS reads
+it while the stalled client still has a backlog and parks it unparsed. Proxy
+Verifier cannot delay part of an HTTP/1.1 body, hence this custom origin.
+'''
+
+import argparse
+import socket
+import sys
+import threading
+import time
+
+
+def read_request(conn):
+ buf = b""
+ while b"\r\n\r\n" not in buf:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return None
+ buf += chunk
+ head, _, body = buf.partition(b"\r\n\r\n")
+ content_length = 0
+ for line in head.split(b"\r\n")[1:]:
+ name, _, value = line.partition(b":")
+ if name.strip().lower() == b"content-length":
+ content_length = int(value.strip())
+ while len(body) < content_length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return None
+ body += chunk
+ return head
+
+
+def handle(conn, body_size, hold):
+ try:
+ conn.settimeout(60)
+ if read_request(conn) is None:
+ return
+ # Header and the one data chunk arrive together so ATS parses the whole
+ # chunk into the dechunked buffer, filling it past the water mark while
+ # the stalled client holds the consumer above high water.
+ conn.sendall(
+ b"HTTP/1.1 200 OK\r\n"
+ b"Content-Type: application/octet-stream\r\n"
+ b"Transfer-Encoding: chunked\r\n"
+ b"\r\n" + b"%x\r\n" % body_size + b"x" * body_size + b"\r\n")
+ # The last-chunk arrives as its own read while the consumer is throttled,
+ # so the chunked throttle parks it unparsed in chunked_reader.
+ time.sleep(hold)
+ conn.sendall(b"0\r\n\r\n")
+ sys.stderr.write("origin: sent last-chunk\n")
+ sys.stderr.flush()
+ while conn.recv(4096):
+ pass
+ except (OSError, socket.timeout):
+ return
+ finally:
+ try:
+ conn.close()
+ except OSError:
+ pass
+
+
+def main():
+ ap = argparse.ArgumentParser()
+ ap.add_argument("port", type=int)
+ ap.add_argument("--ip", default="127.0.0.1")
+ ap.add_argument("--body-size", type=int, default=24000)
+ ap.add_argument("--hold", type=float, default=0.5, help="seconds between the body and the last-chunk")
+ args = ap.parse_args()
+
+ srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ srv.bind((args.ip, args.port))
+ srv.listen(128)
+ sys.stderr.write(f"dechunk half-close origin listening on {args.ip}:{args.port}\n")
+ sys.stderr.flush()
+
+ while True:
+ try:
+ conn, _ = srv.accept()
+ except OSError:
+ break
+ threading.Thread(target=handle, args=(conn, args.body_size, args.hold), daemon=True).start()
+
+
+if __name__ == "__main__":
+ main()