From e553d2b67bd78b3bddb4ac5661c03ecfb9ce959d Mon Sep 17 00:00:00 2001 From: sxia-aviatrix Date: Wed, 19 Aug 2026 21:58:54 +0000 Subject: [PATCH 1/5] Fix use-after-free crash in when is called while a request transform plugin registered at is active. --- src/proxy/http/HttpSM.cc | 11 + .../slow_post/partial_post_client.py | 69 ++++ .../post_early_response_transform.test.py | 78 ++++ tests/tools/plugins/CMakeLists.txt | 1 + tests/tools/plugins/null_transform_request.cc | 332 ++++++++++++++++++ 5 files changed, 491 insertions(+) create mode 100644 tests/gold_tests/slow_post/partial_post_client.py create mode 100644 tests/gold_tests/slow_post/post_early_response_transform.test.py create mode 100644 tests/tools/plugins/null_transform_request.cc diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 0d54ad376d7..8a6cec744e2 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -2145,6 +2145,17 @@ HttpSM::state_read_server_response_header(int event, void *data) // If there is a post body in transit, give up on it if (tunnel.is_tunnel_alive()) { tunnel.abort_tunnel(); + // abort_tunnel() does not clean up vc_table entries. If a request + // transform is present, post_transform_info.entry still points at the + // TransformVConnection whose chain will be freed by the abort cascade. + // Clean it up now so cleanup_all() in kill_this() does not call + // do_io_close() on freed memory. + if (post_transform_info.entry != nullptr) { + post_transform_info.entry->in_tunnel = false; + vc_table.cleanup_entry(post_transform_info.entry); + post_transform_info.entry = nullptr; + } + // ink_release_assert(post_transform_info.entry == nullptr); // Make sure client connection is closed when we are done in case there is cruft left over t_state.client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE; // Similarly the server connection should also be closed diff --git a/tests/gold_tests/slow_post/partial_post_client.py b/tests/gold_tests/slow_post/partial_post_client.py new file mode 100644 index 00000000000..7cd5ab98fdf --- /dev/null +++ b/tests/gold_tests/slow_post/partial_post_client.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +"""Send a partial POST to trigger the abort_tunnel code path. + +Sends POST headers claiming a large Content-Length but only sends a small +chunk of body data. When a request transform plugin is active, this causes +ATS to call abort_tunnel() while the transform entry is still in the vc_table. +""" + +# 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 socket +import sys + + +def main() -> int: + """Run the client.""" + host = sys.argv[1] if len(sys.argv) > 1 else '127.0.0.1' + port = int(sys.argv[2]) if len(sys.argv) > 2 else 8080 + + request = ( + f'POST / HTTP/1.1\r\n' + f'Host: quick.server.com\r\n' + f'Content-Type: application/octet-stream\r\n' + f'Content-Length: 100000\r\n' + f'\r\n').encode() + + partial_body = b'x' * 4096 + + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(5) + try: + sock.connect((host, port)) + sock.sendall(request + partial_body) + print(f'Sent POST headers (Content-Length: 100000) + {len(partial_body)} bytes') + + # Read whatever response ATS sends back. + try: + response = sock.recv(4096) + if response: + first_line = response.split(b'\r\n')[0].decode(errors='replace') + print(f'Got response: {first_line}') + else: + print('Got response: connection closed') + except socket.timeout: + print('Got response: timeout (server may still be processing)') + except ConnectionError as e: + print(f'Got response: connection error ({e})') + finally: + sock.close() + + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/tests/gold_tests/slow_post/post_early_response_transform.test.py b/tests/gold_tests/slow_post/post_early_response_transform.test.py new file mode 100644 index 00000000000..cea4841fe17 --- /dev/null +++ b/tests/gold_tests/slow_post/post_early_response_transform.test.py @@ -0,0 +1,78 @@ +"""Verify ATS does not crash when a server replies before receiving the full POST body and a request transform plugin is active. + +When a POST request has a request transform and the origin responds before the +full body is forwarded through the transform chain, abort_tunnel() is called. +Without the fix, post_transform_info.entry is left stale in the vc_table, +causing a use-after-free in cleanup_all(). +""" + +# 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 +import sys + +Test.Summary = __doc__ + +_partial_post_client = 'partial_post_client.py' +_quick_server = 'quick_server.py' +_init_file = '__init__.py' +_http_utils = 'http_utils.py' + +# Set up DNS. +dns = Test.MakeDNServer('dns', default='127.0.0.1') + +# Set up the quick-responding origin server (responds before reading body). +server = Test.Processes.Process('server') +server_port = get_port(server, 'http_port') +server.Command = f'{sys.executable} {_quick_server} 127.0.0.1 {server_port}' +server.Ready = When.PortOpenv4(server_port) + +# Set up ATS with the null_transform_request plugin. +ts = Test.MakeATSProcess('ts') +ts.Disk.remap_config.AddLine(f'map / http://quick.server.com:{server_port}') +ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.dns.nameservers': f'127.0.0.1:{dns.Variables.Port}', + 'proxy.config.dns.resolv_conf': 'NULL', + }) +Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'null_transform_request.so'), ts) + +# The test: send a partial POST (large Content-Length, small body) through ATS +# with a request transform active. The quick server responds immediately. +# ATS must handle the abort_tunnel without crashing. +tr = Test.AddTestRun('Partial POST with request transform and early server response') + +tools_dir = ts.Variables.AtsTestToolsDir +http_utils = os.path.join(tools_dir, 'http_utils.py') +tr.Setup.CopyAs(_init_file, Test.RunDirectory) +tr.Setup.CopyAs(http_utils, Test.RunDirectory) +tr.Setup.CopyAs(_quick_server, Test.RunDirectory) +tr.Setup.CopyAs(_partial_post_client, Test.RunDirectory) + +p = tr.Processes.Default +p.Command = (f'{sys.executable} {_partial_post_client} ' + f'127.0.0.1 {ts.Variables.port}') +p.ReturnCode = 0 +p.Streams.All += Testers.ContainsExpression('Got response', 'Verify client received a response from ATS') + +ts.StartBefore(dns) +ts.StartBefore(server) +p.StartBefore(ts) +tr.Timeout = 10 diff --git a/tests/tools/plugins/CMakeLists.txt b/tests/tools/plugins/CMakeLists.txt index b7f18109ef0..856d00e6a9c 100644 --- a/tests/tools/plugins/CMakeLists.txt +++ b/tests/tools/plugins/CMakeLists.txt @@ -42,6 +42,7 @@ add_autest_plugin(hook_tunnel_plugin hook_tunnel_plugin.cc) add_autest_plugin(tunnel_transform tunnel_transform.cc) add_autest_plugin(http2_close_connection http2_close_connection.cc) add_autest_plugin(redirect_rearm redirect_rearm.cc) +add_autest_plugin(null_transform_request null_transform_request.cc) target_link_libraries(continuations_verify PRIVATE OpenSSL::SSL) target_link_libraries(ssl_client_verify_test PRIVATE OpenSSL::SSL) diff --git a/tests/tools/plugins/null_transform_request.cc b/tests/tools/plugins/null_transform_request.cc new file mode 100644 index 00000000000..7f20aa916d6 --- /dev/null +++ b/tests/tools/plugins/null_transform_request.cc @@ -0,0 +1,332 @@ +/** @file + + An example program that does a null transform of response body content. + + @section license License + + 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. + */ + +#include +#include +#include + +#include "ts/ts.h" + +#define PLUGIN_NAME "null_transform_request" +static const char PLUGIN_TAG[] = PLUGIN_NAME; +static DbgCtl plugin_ctl{PLUGIN_TAG}; + +static int stat_ua_bytes_sent = 0; // number of bytes seen by the transform from UA to OS +static int stat_os_bytes_sent = 0; // number of bytes seen by the transform from OS to UA +static int stat_error = 0; +static int stat_test_done = 0; + +typedef struct { + TSVIO output_vio; + TSIOBuffer output_buffer; + TSIOBufferReader output_reader; +} MyData; + +static MyData * +my_data_alloc() +{ + MyData *data; + + data = (MyData *)TSmalloc(sizeof(MyData)); + data->output_vio = nullptr; + data->output_buffer = nullptr; + data->output_reader = nullptr; + + return data; +} + +static void +my_data_destroy(MyData *data) +{ + if (data) { + if (data->output_buffer) { + TSIOBufferDestroy(data->output_buffer); + } + TSfree(data); + } +} + +static void +handle_transform(TSCont contp, bool forward) +{ + TSVConn output_conn; + TSIOBuffer buf_test; + TSVIO input_vio; + MyData *data; + int64_t towrite; + + Dbg(plugin_ctl, "Entering handle_transform()"); + /* Get the output (downstream) vconnection where we'll write data to. */ + + output_conn = TSTransformOutputVConnGet(contp); + + /* Get the write VIO for the write operation that was performed on + * ourself. This VIO contains the buffer that we are to read from + * as well as the continuation we are to call when the buffer is + * empty. This is the input VIO (the write VIO for the upstream + * vconnection). + */ + input_vio = TSVConnWriteVIOGet(contp); + + /* Get our data structure for this operation. The private data + * structure contains the output VIO and output buffer. If the + * private data structure pointer is null, then we'll create it + * and initialize its internals. + */ + data = static_cast(TSContDataGet(contp)); + if (!data) { + data = my_data_alloc(); + data->output_buffer = TSIOBufferCreate(); + data->output_reader = TSIOBufferReaderAlloc(data->output_buffer); + Dbg(plugin_ctl, "\tWriting %" PRId64 " bytes on VConn", TSVIONBytesGet(input_vio)); + data->output_vio = TSVConnWrite(output_conn, contp, data->output_reader, TSVIONBytesGet(input_vio)); + TSContDataSet(contp, data); + } + + /* We also check to see if the input VIO's buffer is non-null. A + * null buffer indicates that the write operation has been + * shutdown and that the upstream continuation does not want us to send any + * more WRITE_READY or WRITE_COMPLETE events. For this simplistic + * transformation that means we're done. In a more complex + * transformation we might have to finish writing the transformed + * data to our output connection. + */ + buf_test = TSVIOBufferGet(input_vio); + + if (!buf_test) { + TSVIONBytesSet(data->output_vio, TSVIONDoneGet(input_vio)); + TSVIOReenable(data->output_vio); + return; + } + + /* Determine how much data we have left to read. For this null + * transform plugin this is also the amount of data we have left + * to write to the output connection. + */ + towrite = TSVIONTodoGet(input_vio); + Dbg(plugin_ctl, "\ttoWrite is %" PRId64 "", towrite); + + if (towrite > 0) { + /* The amount of data left to read needs to be truncated by + * the amount of data actually in the read buffer. + */ + int64_t avail = TSIOBufferReaderAvail(TSVIOReaderGet(input_vio)); + Dbg(plugin_ctl, "\tavail is %" PRId64 "", avail); + if (towrite > avail) { + towrite = avail; + } + + if (towrite > 0) { + /* Copy the data from the read buffer to the output buffer. */ + TSIOBufferCopy(TSVIOBufferGet(data->output_vio), TSVIOReaderGet(input_vio), towrite, 0); + + /* Tell the read buffer that we have read the data and are no + * longer interested in it. + */ + TSIOBufferReaderConsume(TSVIOReaderGet(input_vio), towrite); + + /* Modify the input VIO to reflect how much data we've + * completed. + */ + TSVIONDoneSet(input_vio, TSVIONDoneGet(input_vio) + towrite); + if (forward) { + TSStatIntIncrement(stat_ua_bytes_sent, towrite); + } else { + TSStatIntIncrement(stat_os_bytes_sent, towrite); + } + } + } + + /* Now we check the input VIO to see if there is data left to + * read. + */ + if (TSVIONTodoGet(input_vio) > 0) { + if (towrite > 0) { + /* If there is data left to read, then we reenable the output + * connection by reenabling the output VIO. This will wake up + * the output connection and allow it to consume data from the + * output buffer. + */ + TSVIOReenable(data->output_vio); + + /* Call back the input VIO continuation to let it know that we + * are ready for more data. + */ + TSContCall(TSVIOContGet(input_vio), TS_EVENT_VCONN_WRITE_READY, input_vio); + } + } else { + /* If there is no data left to read, then we modify the output + * VIO to reflect how much data the output connection should + * expect. This allows the output connection to know when it + * is done reading. We then reenable the output connection so + * that it can consume the data we just gave it. + */ + TSVIONBytesSet(data->output_vio, TSVIONDoneGet(input_vio)); + TSVIOReenable(data->output_vio); + + /* Call back the input VIO continuation to let it know that we + * have completed the write operation. + */ + TSContCall(TSVIOContGet(input_vio), TS_EVENT_VCONN_WRITE_COMPLETE, input_vio); + } +} + +static int +null_transform(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */, bool forward) +{ + /* Check to see if the transformation has been closed by a call to + * TSVConnClose. + */ + Dbg(plugin_ctl, "Entering null_transform()"); + + if (TSVConnClosedGet(contp)) { + Dbg(plugin_ctl, "\tVConn is closed"); + my_data_destroy(static_cast(TSContDataGet(contp))); + TSContDestroy(contp); + return 0; + } else { + switch (event) { + case TS_EVENT_ERROR: { + TSVIO input_vio; + + TSStatIntIncrement(stat_error, 1); + + Dbg(plugin_ctl, "\tEvent is TS_EVENT_ERROR"); + /* Get the write VIO for the write operation that was + * performed on ourself. This VIO contains the continuation of + * our parent transformation. This is the input VIO. + */ + input_vio = TSVConnWriteVIOGet(contp); + + /* Call back the write VIO continuation to let it know that we + * have completed the write operation. + */ + TSContCall(TSVIOContGet(input_vio), TS_EVENT_ERROR, input_vio); + } break; + case TS_EVENT_VCONN_WRITE_COMPLETE: + Dbg(plugin_ctl, "\tEvent is TS_EVENT_VCONN_WRITE_COMPLETE"); + /* When our output connection says that it has finished + * reading all the data we've written to it then we should + * shutdown the write portion of its connection to + * indicate that we don't want to hear about it anymore. + */ + TSVConnShutdown(TSTransformOutputVConnGet(contp), 0, 1); + break; + + /* If we get a WRITE_READY event or any other type of + * event (sent, perhaps, because we were re-enabled) then + * we'll attempt to transform more data. + */ + case TS_EVENT_VCONN_WRITE_READY: + Dbg(plugin_ctl, "\tEvent is TS_EVENT_VCONN_WRITE_READY"); + handle_transform(contp, forward); + break; + default: + Dbg(plugin_ctl, "\t(event is %d)", event); + handle_transform(contp, forward); + break; + } + } + + return 0; +} + +static int +forward_null_transform(TSCont contp, TSEvent event, void *edata) +{ + return null_transform(contp, event, edata, true); +} + +static int +reverse_null_transform(TSCont contp, TSEvent event, void *edata) +{ + return null_transform(contp, event, edata, false); +} + +static void +transform_add(TSHttpTxn txnp) +{ + Dbg(plugin_ctl, "Entering transform_add()"); + TSVConn connp = TSTransformCreate(forward_null_transform, txnp); + TSVConn rev_connp = TSTransformCreate(reverse_null_transform, txnp); + TSHttpTxnHookAdd(txnp, TS_HTTP_REQUEST_TRANSFORM_HOOK, connp); + TSHttpTxnHookAdd(txnp, TS_HTTP_RESPONSE_TRANSFORM_HOOK, rev_connp); +} + +static int +transform_plugin(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) +{ + TSHttpTxn txnp = (TSHttpTxn)edata; + + Dbg(plugin_ctl, "Entering transform_plugin()"); + switch (event) { + case TS_EVENT_HTTP_READ_REQUEST_HDR: + Dbg(plugin_ctl, "\tEvent is TS_EVENT_HTTP_READ_REQUEST_HDR"); + transform_add(txnp); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; + default: + break; + } + + return 0; +} + +static int +handleMsg(TSCont /* cont ATS_UNUSED */, TSEvent event, void * /* edata ATS_UNUSED */) +{ + Dbg(plugin_ctl, "handleMsg event=%d", event); + TSStatIntIncrement(stat_test_done, 1); + return TS_SUCCESS; +} + +void +TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) +{ + TSPluginRegistrationInfo info; + + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + + goto Lerror; + } + + stat_ua_bytes_sent = + TSStatCreate("null_transform_request.ua.bytes_sent", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); + stat_os_bytes_sent = + TSStatCreate("null_transform_request.os.bytes_sent", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); + stat_error = TSStatCreate("null_transform_request.error", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); + stat_test_done = + TSStatCreate("null_transform_request.test.done", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); + + TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, TSContCreate(transform_plugin, nullptr)); + TSLifecycleHookAdd(TS_LIFECYCLE_MSG_HOOK, TSContCreate(handleMsg, TSMutexCreate())); + return; + +Lerror: + TSError("[%s] Unable to initialize plugin (disabled)", PLUGIN_NAME); +} From d51c9a7e0fdebda2d5ea7e82a49ed141e1708788 Mon Sep 17 00:00:00 2001 From: sxia-aviatrix Date: Thu, 20 Aug 2026 14:40:22 +0000 Subject: [PATCH 2/5] Remove unnecessary parts in null_transform_request plugin --- tests/tools/plugins/null_transform_request.cc | 254 +++--------------- 1 file changed, 42 insertions(+), 212 deletions(-) diff --git a/tests/tools/plugins/null_transform_request.cc b/tests/tools/plugins/null_transform_request.cc index 7f20aa916d6..63401cb73b3 100644 --- a/tests/tools/plugins/null_transform_request.cc +++ b/tests/tools/plugins/null_transform_request.cc @@ -1,6 +1,11 @@ /** @file - An example program that does a null transform of response body content. + Null request transform plugin hooked at TS_HTTP_READ_REQUEST_HDR_HOOK. + + Used by post_early_response_transform.test.py to reproduce a use-after-free + in HttpSM::state_read_server_response_header() when abort_tunnel() is called + while a request transform is active. The transform passes request body data + through unmodified. @section license License @@ -21,42 +26,31 @@ limitations under the License. */ -#include -#include -#include +#include +#include #include "ts/ts.h" #define PLUGIN_NAME "null_transform_request" -static const char PLUGIN_TAG[] = PLUGIN_NAME; -static DbgCtl plugin_ctl{PLUGIN_TAG}; - -static int stat_ua_bytes_sent = 0; // number of bytes seen by the transform from UA to OS -static int stat_os_bytes_sent = 0; // number of bytes seen by the transform from OS to UA -static int stat_error = 0; -static int stat_test_done = 0; typedef struct { TSVIO output_vio; TSIOBuffer output_buffer; TSIOBufferReader output_reader; -} MyData; +} TransformData; -static MyData * -my_data_alloc() +static TransformData * +transform_data_alloc() { - MyData *data; - - data = (MyData *)TSmalloc(sizeof(MyData)); + auto *data = static_cast(TSmalloc(sizeof(TransformData))); data->output_vio = nullptr; data->output_buffer = nullptr; data->output_reader = nullptr; - return data; } static void -my_data_destroy(MyData *data) +transform_data_destroy(TransformData *data) { if (data) { if (data->output_buffer) { @@ -67,225 +61,71 @@ my_data_destroy(MyData *data) } static void -handle_transform(TSCont contp, bool forward) +handle_transform(TSCont contp) { - TSVConn output_conn; - TSIOBuffer buf_test; - TSVIO input_vio; - MyData *data; - int64_t towrite; - - Dbg(plugin_ctl, "Entering handle_transform()"); - /* Get the output (downstream) vconnection where we'll write data to. */ - - output_conn = TSTransformOutputVConnGet(contp); - - /* Get the write VIO for the write operation that was performed on - * ourself. This VIO contains the buffer that we are to read from - * as well as the continuation we are to call when the buffer is - * empty. This is the input VIO (the write VIO for the upstream - * vconnection). - */ - input_vio = TSVConnWriteVIOGet(contp); + TSVConn output_conn = TSTransformOutputVConnGet(contp); + TSVIO input_vio = TSVConnWriteVIOGet(contp); + TransformData *data = static_cast(TSContDataGet(contp)); - /* Get our data structure for this operation. The private data - * structure contains the output VIO and output buffer. If the - * private data structure pointer is null, then we'll create it - * and initialize its internals. - */ - data = static_cast(TSContDataGet(contp)); if (!data) { - data = my_data_alloc(); + data = transform_data_alloc(); data->output_buffer = TSIOBufferCreate(); data->output_reader = TSIOBufferReaderAlloc(data->output_buffer); - Dbg(plugin_ctl, "\tWriting %" PRId64 " bytes on VConn", TSVIONBytesGet(input_vio)); - data->output_vio = TSVConnWrite(output_conn, contp, data->output_reader, TSVIONBytesGet(input_vio)); + data->output_vio = TSVConnWrite(output_conn, contp, data->output_reader, TSVIONBytesGet(input_vio)); TSContDataSet(contp, data); } - /* We also check to see if the input VIO's buffer is non-null. A - * null buffer indicates that the write operation has been - * shutdown and that the upstream continuation does not want us to send any - * more WRITE_READY or WRITE_COMPLETE events. For this simplistic - * transformation that means we're done. In a more complex - * transformation we might have to finish writing the transformed - * data to our output connection. - */ - buf_test = TSVIOBufferGet(input_vio); - - if (!buf_test) { + if (!TSVIOBufferGet(input_vio)) { TSVIONBytesSet(data->output_vio, TSVIONDoneGet(input_vio)); TSVIOReenable(data->output_vio); return; } - /* Determine how much data we have left to read. For this null - * transform plugin this is also the amount of data we have left - * to write to the output connection. - */ - towrite = TSVIONTodoGet(input_vio); - Dbg(plugin_ctl, "\ttoWrite is %" PRId64 "", towrite); - + int64_t towrite = TSVIONTodoGet(input_vio); if (towrite > 0) { - /* The amount of data left to read needs to be truncated by - * the amount of data actually in the read buffer. - */ int64_t avail = TSIOBufferReaderAvail(TSVIOReaderGet(input_vio)); - Dbg(plugin_ctl, "\tavail is %" PRId64 "", avail); if (towrite > avail) { towrite = avail; } - if (towrite > 0) { - /* Copy the data from the read buffer to the output buffer. */ TSIOBufferCopy(TSVIOBufferGet(data->output_vio), TSVIOReaderGet(input_vio), towrite, 0); - - /* Tell the read buffer that we have read the data and are no - * longer interested in it. - */ TSIOBufferReaderConsume(TSVIOReaderGet(input_vio), towrite); - - /* Modify the input VIO to reflect how much data we've - * completed. - */ TSVIONDoneSet(input_vio, TSVIONDoneGet(input_vio) + towrite); - if (forward) { - TSStatIntIncrement(stat_ua_bytes_sent, towrite); - } else { - TSStatIntIncrement(stat_os_bytes_sent, towrite); - } } } - /* Now we check the input VIO to see if there is data left to - * read. - */ if (TSVIONTodoGet(input_vio) > 0) { if (towrite > 0) { - /* If there is data left to read, then we reenable the output - * connection by reenabling the output VIO. This will wake up - * the output connection and allow it to consume data from the - * output buffer. - */ TSVIOReenable(data->output_vio); - - /* Call back the input VIO continuation to let it know that we - * are ready for more data. - */ TSContCall(TSVIOContGet(input_vio), TS_EVENT_VCONN_WRITE_READY, input_vio); } } else { - /* If there is no data left to read, then we modify the output - * VIO to reflect how much data the output connection should - * expect. This allows the output connection to know when it - * is done reading. We then reenable the output connection so - * that it can consume the data we just gave it. - */ TSVIONBytesSet(data->output_vio, TSVIONDoneGet(input_vio)); TSVIOReenable(data->output_vio); - - /* Call back the input VIO continuation to let it know that we - * have completed the write operation. - */ TSContCall(TSVIOContGet(input_vio), TS_EVENT_VCONN_WRITE_COMPLETE, input_vio); } } static int -null_transform(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */, bool forward) +null_transform(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */) { - /* Check to see if the transformation has been closed by a call to - * TSVConnClose. - */ - Dbg(plugin_ctl, "Entering null_transform()"); - if (TSVConnClosedGet(contp)) { - Dbg(plugin_ctl, "\tVConn is closed"); - my_data_destroy(static_cast(TSContDataGet(contp))); + transform_data_destroy(static_cast(TSContDataGet(contp))); TSContDestroy(contp); return 0; - } else { - switch (event) { - case TS_EVENT_ERROR: { - TSVIO input_vio; - - TSStatIntIncrement(stat_error, 1); - - Dbg(plugin_ctl, "\tEvent is TS_EVENT_ERROR"); - /* Get the write VIO for the write operation that was - * performed on ourself. This VIO contains the continuation of - * our parent transformation. This is the input VIO. - */ - input_vio = TSVConnWriteVIOGet(contp); - - /* Call back the write VIO continuation to let it know that we - * have completed the write operation. - */ - TSContCall(TSVIOContGet(input_vio), TS_EVENT_ERROR, input_vio); - } break; - case TS_EVENT_VCONN_WRITE_COMPLETE: - Dbg(plugin_ctl, "\tEvent is TS_EVENT_VCONN_WRITE_COMPLETE"); - /* When our output connection says that it has finished - * reading all the data we've written to it then we should - * shutdown the write portion of its connection to - * indicate that we don't want to hear about it anymore. - */ - TSVConnShutdown(TSTransformOutputVConnGet(contp), 0, 1); - break; - - /* If we get a WRITE_READY event or any other type of - * event (sent, perhaps, because we were re-enabled) then - * we'll attempt to transform more data. - */ - case TS_EVENT_VCONN_WRITE_READY: - Dbg(plugin_ctl, "\tEvent is TS_EVENT_VCONN_WRITE_READY"); - handle_transform(contp, forward); - break; - default: - Dbg(plugin_ctl, "\t(event is %d)", event); - handle_transform(contp, forward); - break; - } } - return 0; -} - -static int -forward_null_transform(TSCont contp, TSEvent event, void *edata) -{ - return null_transform(contp, event, edata, true); -} - -static int -reverse_null_transform(TSCont contp, TSEvent event, void *edata) -{ - return null_transform(contp, event, edata, false); -} - -static void -transform_add(TSHttpTxn txnp) -{ - Dbg(plugin_ctl, "Entering transform_add()"); - TSVConn connp = TSTransformCreate(forward_null_transform, txnp); - TSVConn rev_connp = TSTransformCreate(reverse_null_transform, txnp); - TSHttpTxnHookAdd(txnp, TS_HTTP_REQUEST_TRANSFORM_HOOK, connp); - TSHttpTxnHookAdd(txnp, TS_HTTP_RESPONSE_TRANSFORM_HOOK, rev_connp); -} - -static int -transform_plugin(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) -{ - TSHttpTxn txnp = (TSHttpTxn)edata; - - Dbg(plugin_ctl, "Entering transform_plugin()"); switch (event) { - case TS_EVENT_HTTP_READ_REQUEST_HDR: - Dbg(plugin_ctl, "\tEvent is TS_EVENT_HTTP_READ_REQUEST_HDR"); - transform_add(txnp); - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); - return 0; + case TS_EVENT_ERROR: { + TSVIO input_vio = TSVConnWriteVIOGet(contp); + TSContCall(TSVIOContGet(input_vio), TS_EVENT_ERROR, input_vio); + break; + } + case TS_EVENT_VCONN_WRITE_COMPLETE: + TSVConnShutdown(TSTransformOutputVConnGet(contp), 0, 1); + break; default: + handle_transform(contp); break; } @@ -293,11 +133,15 @@ transform_plugin(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) } static int -handleMsg(TSCont /* cont ATS_UNUSED */, TSEvent event, void * /* edata ATS_UNUSED */) +transform_plugin(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) { - Dbg(plugin_ctl, "handleMsg event=%d", event); - TSStatIntIncrement(stat_test_done, 1); - return TS_SUCCESS; + if (event == TS_EVENT_HTTP_READ_REQUEST_HDR) { + TSHttpTxn txnp = static_cast(edata); + TSVConn connp = TSTransformCreate(null_transform, txnp); + TSHttpTxnHookAdd(txnp, TS_HTTP_REQUEST_TRANSFORM_HOOK, connp); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + } + return 0; } void @@ -310,23 +154,9 @@ TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) info.support_email = "dev@trafficserver.apache.org"; if (TSPluginRegister(&info) != TS_SUCCESS) { - TSError("[%s] Plugin registration failed", PLUGIN_NAME); - - goto Lerror; + TSError("[%s] Unable to initialize plugin (disabled)", PLUGIN_NAME); + return; } - stat_ua_bytes_sent = - TSStatCreate("null_transform_request.ua.bytes_sent", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); - stat_os_bytes_sent = - TSStatCreate("null_transform_request.os.bytes_sent", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); - stat_error = TSStatCreate("null_transform_request.error", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); - stat_test_done = - TSStatCreate("null_transform_request.test.done", TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); - TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, TSContCreate(transform_plugin, nullptr)); - TSLifecycleHookAdd(TS_LIFECYCLE_MSG_HOOK, TSContCreate(handleMsg, TSMutexCreate())); - return; - -Lerror: - TSError("[%s] Unable to initialize plugin (disabled)", PLUGIN_NAME); } From c784d87bbada5bf99afc5bbca572ba613afd6401 Mon Sep 17 00:00:00 2001 From: sxia-aviatrix Date: Thu, 20 Aug 2026 21:31:13 +0000 Subject: [PATCH 3/5] Comment out the assert --- src/proxy/http/HttpSM.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 8a6cec744e2..a1673b58715 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -2155,7 +2155,7 @@ HttpSM::state_read_server_response_header(int event, void *data) vc_table.cleanup_entry(post_transform_info.entry); post_transform_info.entry = nullptr; } - // ink_release_assert(post_transform_info.entry == nullptr); + ink_release_assert(post_transform_info.entry == nullptr); // Make sure client connection is closed when we are done in case there is cruft left over t_state.client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE; // Similarly the server connection should also be closed From 706f70676b24361dfcff2d5e12541f035f78d930 Mon Sep 17 00:00:00 2001 From: sxia-aviatrix Date: Thu, 20 Aug 2026 21:32:57 +0000 Subject: [PATCH 4/5] Update the test cases to use test class --- .../post_early_response_transform.test.py | 130 +++++++++++------- 1 file changed, 82 insertions(+), 48 deletions(-) diff --git a/tests/gold_tests/slow_post/post_early_response_transform.test.py b/tests/gold_tests/slow_post/post_early_response_transform.test.py index cea4841fe17..8ed47ac83a6 100644 --- a/tests/gold_tests/slow_post/post_early_response_transform.test.py +++ b/tests/gold_tests/slow_post/post_early_response_transform.test.py @@ -28,51 +28,85 @@ Test.Summary = __doc__ -_partial_post_client = 'partial_post_client.py' -_quick_server = 'quick_server.py' -_init_file = '__init__.py' -_http_utils = 'http_utils.py' - -# Set up DNS. -dns = Test.MakeDNServer('dns', default='127.0.0.1') - -# Set up the quick-responding origin server (responds before reading body). -server = Test.Processes.Process('server') -server_port = get_port(server, 'http_port') -server.Command = f'{sys.executable} {_quick_server} 127.0.0.1 {server_port}' -server.Ready = When.PortOpenv4(server_port) - -# Set up ATS with the null_transform_request plugin. -ts = Test.MakeATSProcess('ts') -ts.Disk.remap_config.AddLine(f'map / http://quick.server.com:{server_port}') -ts.Disk.records_config.update( - { - 'proxy.config.diags.debug.enabled': 1, - 'proxy.config.diags.debug.tags': 'http', - 'proxy.config.dns.nameservers': f'127.0.0.1:{dns.Variables.Port}', - 'proxy.config.dns.resolv_conf': 'NULL', - }) -Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'null_transform_request.so'), ts) - -# The test: send a partial POST (large Content-Length, small body) through ATS -# with a request transform active. The quick server responds immediately. -# ATS must handle the abort_tunnel without crashing. -tr = Test.AddTestRun('Partial POST with request transform and early server response') - -tools_dir = ts.Variables.AtsTestToolsDir -http_utils = os.path.join(tools_dir, 'http_utils.py') -tr.Setup.CopyAs(_init_file, Test.RunDirectory) -tr.Setup.CopyAs(http_utils, Test.RunDirectory) -tr.Setup.CopyAs(_quick_server, Test.RunDirectory) -tr.Setup.CopyAs(_partial_post_client, Test.RunDirectory) - -p = tr.Processes.Default -p.Command = (f'{sys.executable} {_partial_post_client} ' - f'127.0.0.1 {ts.Variables.port}') -p.ReturnCode = 0 -p.Streams.All += Testers.ContainsExpression('Got response', 'Verify client received a response from ATS') - -ts.StartBefore(dns) -ts.StartBefore(server) -p.StartBefore(ts) -tr.Timeout = 10 + +class PostEarlyResponseTransformTest: + """Verify abort_tunnel with a request transform does not crash ATS.""" + + _partial_post_client = 'partial_post_client.py' + _quick_server = 'quick_server.py' + _init_file = '__init__.py' + _http_utils = 'http_utils.py' + + def __init__(self): + """Configure and run the test.""" + tr = Test.AddTestRun('Partial POST with request transform and early server response') + self._configure_dns(tr) + self._configure_server(tr) + self._configure_traffic_server(tr) + self._configure_client(tr) + + def _configure_dns(self, tr: 'TestRun') -> None: + """Configure the DNS process. + + :param tr: The test run to associate the DNS process with. + """ + self._dns = tr.MakeDNServer('dns', default='127.0.0.1') + + def _configure_server(self, tr: 'TestRun') -> None: + """Configure the quick-responding origin server. + + The server responds immediately after receiving the request headers, + before the full POST body arrives. + + :param tr: The test run to associate the server process with. + """ + server = tr.Processes.Process('server') + server_port = get_port(server, 'http_port') + server.Command = f'{sys.executable} {self._quick_server} 127.0.0.1 {server_port}' + server.Ready = When.PortOpenv4(server_port) + self._server = server + + def _configure_traffic_server(self, tr: 'TestRun') -> None: + """Configure ATS with the null_transform_request plugin. + + :param tr: The test run to associate the ATS process with. + """ + self._ts = tr.MakeATSProcess('ts') + self._ts.Disk.remap_config.AddLine(f'map / http://quick.server.com:{self._server.Variables.http_port}') + self._ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.dns.nameservers': f'127.0.0.1:{self._dns.Variables.Port}', + 'proxy.config.dns.resolv_conf': 'NULL', + }) + Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'null_transform_request.so'), self._ts) + + def _configure_client(self, tr: 'TestRun') -> None: + """Configure the partial POST client. + + Sends a POST with a large Content-Length but only a small body, + triggering abort_tunnel when the origin responds early. + + :param tr: The test run to associate the client process with. + """ + tools_dir = self._ts.Variables.AtsTestToolsDir + http_utils = os.path.join(tools_dir, 'http_utils.py') + tr.Setup.CopyAs(self._init_file, Test.RunDirectory) + tr.Setup.CopyAs(http_utils, Test.RunDirectory) + tr.Setup.CopyAs(self._quick_server, Test.RunDirectory) + tr.Setup.CopyAs(self._partial_post_client, Test.RunDirectory) + + p = tr.Processes.Default + p.Command = (f'{sys.executable} {self._partial_post_client} ' + f'127.0.0.1 {self._ts.Variables.port}') + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('Got response', 'Verify client received a response from ATS') + + self._ts.StartBefore(self._dns) + self._ts.StartBefore(self._server) + p.StartBefore(self._ts) + tr.Timeout = 10 + + +PostEarlyResponseTransformTest() From 588f0d727cdcb9ad8cd979747a1a082a2091fda9 Mon Sep 17 00:00:00 2001 From: sxia-aviatrix Date: Fri, 21 Aug 2026 21:24:11 +0000 Subject: [PATCH 5/5] Explicit free the leaking vc --- src/proxy/http/HttpSM.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index a1673b58715..7fa59ac9f2c 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -2145,17 +2145,16 @@ HttpSM::state_read_server_response_header(int event, void *data) // If there is a post body in transit, give up on it if (tunnel.is_tunnel_alive()) { tunnel.abort_tunnel(); - // abort_tunnel() does not clean up vc_table entries. If a request - // transform is present, post_transform_info.entry still points at the - // TransformVConnection whose chain will be freed by the abort cascade. - // Clean it up now so cleanup_all() in kill_this() does not call - // do_io_close() on freed memory. + // abort_tunnel() cancels I/O but does not close VCs or clean up + // vc_table entries. When a request transform is active the + // post_transform_info entry still references the TransformVConnection + // with in_tunnel=true, which causes cleanup_entry() to skip + // do_io_close() — leaking the VC. Close it explicitly here. if (post_transform_info.entry != nullptr) { - post_transform_info.entry->in_tunnel = false; + post_transform_info.vc->do_io_close(); vc_table.cleanup_entry(post_transform_info.entry); post_transform_info.entry = nullptr; } - ink_release_assert(post_transform_info.entry == nullptr); // Make sure client connection is closed when we are done in case there is cruft left over t_state.client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE; // Similarly the server connection should also be closed