From 114a16e9a23f3611c1e6e9783c0fe1bcbd1e3763 Mon Sep 17 00:00:00 2001 From: bneradt Date: Fri, 7 Aug 2026 14:44:24 -0500 Subject: [PATCH] Add port descriptor destroy API TSPortDescriptorParse allocates an HttpProxyPort that plugins cannot release, so every parsed descriptor leaks for the lifetime of Traffic Server. The API also lacks end-to-end coverage that verifies the accept callback. This patch adds TSPortDescriptorDestroy while preserving the opaque handle ABI, validates unusable descriptors during parsing, and documents the ownership contract. It updates API users and adds an AuTest that opens a dynamically selected port and observes the accept callback. Fixes: #6894 --- ci/asan_leak_suppression/regression.txt | 1 - .../functions/TSPortDescriptorParse.en.rst | 97 +++++++++++++++++++ doc/release-notes/upgrading.en.rst | 11 +++ example/plugins/c-api/passthru/passthru.cc | 17 ++-- include/ts/ts.h | 38 ++++++-- src/api/InkAPI.cc | 31 +++++- src/api/InkAPITest.cc | 16 ++- .../port_descriptor/port_descriptor.test.py | 62 ++++++++++++ tests/tools/plugins/CMakeLists.txt | 1 + tests/tools/plugins/port_descriptor.cc | 92 ++++++++++++++++++ 10 files changed, 342 insertions(+), 24 deletions(-) create mode 100644 doc/developer-guide/api/functions/TSPortDescriptorParse.en.rst create mode 100644 tests/gold_tests/pluginTest/port_descriptor/port_descriptor.test.py create mode 100644 tests/tools/plugins/port_descriptor.cc diff --git a/ci/asan_leak_suppression/regression.txt b/ci/asan_leak_suppression/regression.txt index 3bcd60f553a..1513f1a6fd1 100644 --- a/ci/asan_leak_suppression/regression.txt +++ b/ci/asan_leak_suppression/regression.txt @@ -11,7 +11,6 @@ leak:RegressionTest_HttpTransact_handle_trace_and_options_requests leak:CRYPTO_malloc leak:RegressionTest_SDK_API_TSMgmtGet leak:RegressionTest_SDK_API_TSCache -leak:RegressionTest_SDK_API_TSPortDescriptor leak:RegressionTest_HostDBProcessor leak:RegressionTest_DNS leak:RegressionTest_UDPNet_echo diff --git a/doc/developer-guide/api/functions/TSPortDescriptorParse.en.rst b/doc/developer-guide/api/functions/TSPortDescriptorParse.en.rst new file mode 100644 index 00000000000..b557d56d71b --- /dev/null +++ b/doc/developer-guide/api/functions/TSPortDescriptorParse.en.rst @@ -0,0 +1,97 @@ +.. 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:: ../../../common.defs + +.. default-domain:: cpp + +TSPortDescriptorParse +********************* + +Parse and listen on a proxy port descriptor. + +Synopsis +======== + +.. code-block:: cpp + + #include + +.. type:: TSPortDescriptor +.. function:: TSPortDescriptor TSPortDescriptorParse(const char *descriptor) +.. function:: TSReturnCode TSPortDescriptorAccept(TSPortDescriptor descriptor, TSCont contp) +.. function:: void TSPortDescriptorDestroy(TSPortDescriptor descriptor) + +Description +=========== + +:func:`TSPortDescriptorParse` parses the same descriptor syntax used by +:ts:cv:`proxy.config.http.server_ports` and returns an allocated, opaque +:type:`TSPortDescriptor` handle. Each successful call must be paired with +exactly one call to :func:`TSPortDescriptorDestroy`. + +:func:`TSPortDescriptorAccept` copies the information it needs from +:arg:`descriptor` and does not retain a pointer to it. The descriptor can +therefore be destroyed immediately after :func:`TSPortDescriptorAccept` +returns, regardless of whether the listener remains active. + +A descriptor containing only an ``fd=`` option is not supported because this +API requires an explicit listen endpoint. The ``quic`` option is also not +supported by this API and must not be used; it does not create a QUIC listener. + +For example, this function destroys the descriptor after opening the listener: + +.. code-block:: cpp + + TSReturnCode + listen_on_descriptor(TSCont contp, const char *spec) + { + TSPortDescriptor descriptor = TSPortDescriptorParse(spec); + if (descriptor == nullptr) { + return TS_ERROR; + } + + TSReturnCode result = TSPortDescriptorAccept(descriptor, contp); + TSPortDescriptorDestroy(descriptor); + return result; + } + +When a connection is accepted, :arg:`contp` receives +:enumerator:`TS_EVENT_NET_ACCEPT`. The event data is a :type:`TSVConn` for the +accepted connection. + +Return Values +============= + +:func:`TSPortDescriptorParse` returns a new descriptor handle when +:arg:`descriptor` was parsed successfully. It returns ``nullptr`` for a null +argument, invalid descriptor, or descriptor that cannot be used by +:func:`TSPortDescriptorAccept`. + +:func:`TSPortDescriptorAccept` returns :enumerator:`TS_SUCCESS` when the +listener was opened. It returns :enumerator:`TS_ERROR` for a null argument, a +descriptor with an unusable listen endpoint, or an error opening the listener. + +:func:`TSPortDescriptorDestroy` releases :arg:`descriptor`. Passing +``nullptr`` has no effect. Destroying a descriptor does not stop a listener +previously opened from it. + +See Also +======== + +:manpage:`TSAPI(3ts)`, +:manpage:`TSNetAccept(3ts)`, +:manpage:`records.yaml(5)` diff --git a/doc/release-notes/upgrading.en.rst b/doc/release-notes/upgrading.en.rst index 0548c19b721..fd9c145f7e7 100644 --- a/doc/release-notes/upgrading.en.rst +++ b/doc/release-notes/upgrading.en.rst @@ -19,6 +19,17 @@ .. _upgrading: +Upgrading to ATS v11.x +====================== + +API Changes +----------- + +The handle returned by :cpp:func:`TSPortDescriptorParse` must now be released +with :cpp:func:`TSPortDescriptorDestroy`. The descriptor can be destroyed +immediately after :cpp:func:`TSPortDescriptorAccept` returns because the +listener does not retain it. + Upgrading to ATS v10.x ====================== diff --git a/example/plugins/c-api/passthru/passthru.cc b/example/plugins/c-api/passthru/passthru.cc index 95c78fd6b30..b79733fc39d 100644 --- a/example/plugins/c-api/passthru/passthru.cc +++ b/example/plugins/c-api/passthru/passthru.cc @@ -296,16 +296,15 @@ PassthruAccept(TSCont /* cont */, TSEvent event, void *edata) static TSReturnCode PassthruListen() { - TSMgmtString ports = nullptr; - TSPortDescriptor descriptor = nullptr; - TSCont cont = nullptr; + TSMgmtString ports = nullptr; if (TSMgmtStringGet("config.plugin.passthru.server_ports", &ports) == TS_ERROR) { TSError("[%s] missing config.plugin.passthru.server_ports configuration", PLUGIN_NAME); return TS_ERROR; } - descriptor = TSPortDescriptorParse(ports); + TSPortDescriptor descriptor = TSPortDescriptorParse(ports); + if (descriptor == nullptr) { TSError("[%s] failed to parse config.plugin.passthru.server_ports", PLUGIN_NAME); TSfree(ports); @@ -315,8 +314,14 @@ PassthruListen() Dbg(dbg_ctl, "listening on port '%s'", ports); TSfree(ports); - cont = TSContCreate(PassthruAccept, nullptr); - return TSPortDescriptorAccept(descriptor, cont); + TSCont cont = TSContCreate(PassthruAccept, nullptr); + TSReturnCode result = TSPortDescriptorAccept(descriptor, cont); + + TSPortDescriptorDestroy(descriptor); + if (result != TS_SUCCESS) { + TSContDestroy(cont); + } + return result; } void diff --git a/include/ts/ts.h b/include/ts/ts.h index cff3a73b6f8..5a3de2e585d 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -2187,19 +2187,39 @@ TSReturnCode TSPluginDescriptorAccept(TSCont contp); */ TSReturnCode TSNetAcceptNamedProtocol(TSCont contp, const char *protocol); -/** - Create a new port from the string specification used by the - proxy.config.http.server_ports configuration value. +/** Create a port descriptor. + * + * Parse the string specification used by the + * @c proxy.config.http.server_ports configuration value. The returned handle + * must be released with TSPortDescriptorDestroy(). + * + * @param[in] descriptor Port descriptor string to parse. + * @return A port descriptor handle, or @c nullptr if @a descriptor is invalid + * or cannot be used by TSPortDescriptorAccept(). */ TSPortDescriptor TSPortDescriptorParse(const char *descriptor); -/** - Start listening on the given port descriptor. If a connection is - successfully accepted, the TS_EVENT_NET_ACCEPT is delivered to the - continuation. The event data will be a valid TSVConn bound to the accepted - connection. +/** Start listening on a parsed port descriptor. + * + * If a connection is successfully accepted, @c TS_EVENT_NET_ACCEPT is + * delivered to @a contp. The event data will be a valid @c TSVConn bound to + * the accepted connection. The descriptor is not retained and can be + * destroyed immediately after this function returns. + * + * @param[in] descriptor Parsed port descriptor. + * @param[in] contp Continuation that accepts connections on the port. + * @return @c TS_SUCCESS if the port was opened, @c TS_ERROR otherwise. + */ +TSReturnCode TSPortDescriptorAccept(TSPortDescriptor descriptor, TSCont contp); + +/** Destroy a port descriptor. + * + * This does not stop a listener previously opened with + * TSPortDescriptorAccept(). Passing @c nullptr has no effect. + * + * @param[in] descriptor Port descriptor to destroy. */ -TSReturnCode TSPortDescriptorAccept(TSPortDescriptor, TSCont); +void TSPortDescriptorDestroy(TSPortDescriptor descriptor); /* -------------------------------------------------------------------------- DNS Lookups */ diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 581dae89982..ddd6a2c4e1d 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -7774,13 +7774,23 @@ TSHttpTxnCloseAfterResponse(TSHttpTxn txnp, int should_close) return TS_SUCCESS; } +namespace +{ +bool +is_usable_port_descriptor(const HttpProxyPort *port) +{ + return port != nullptr && + (port->m_family == AF_UNIX || ((port->m_family == AF_INET || port->m_family == AF_INET6) && port->m_port != 0)); +} +} // namespace + // Parse a port descriptor for the proxy.config.http.server_ports descriptor format. TSPortDescriptor TSPortDescriptorParse(const char *descriptor) { - HttpProxyPort *port = new HttpProxyPort(); + auto *port = new HttpProxyPort(); - if (descriptor && port->processOptions(descriptor)) { + if (descriptor != nullptr && port->processOptions(descriptor) && is_usable_port_descriptor(port)) { return reinterpret_cast(port); } @@ -7791,8 +7801,17 @@ TSPortDescriptorParse(const char *descriptor) TSReturnCode TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp) { + if (descp == nullptr || contp == nullptr) { + return TS_ERROR; + } + + const auto *port = reinterpret_cast(descp); + + if (!is_usable_port_descriptor(port)) { + return TS_ERROR; + } + Action *action = nullptr; - HttpProxyPort *port = reinterpret_cast(descp); NetProcessor::AcceptOptions net(make_net_accept_options(port, -1 /* nthreads */)); if (port->isSSL()) { @@ -7804,6 +7823,12 @@ TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp) return action ? TS_SUCCESS : TS_ERROR; } +void +TSPortDescriptorDestroy(TSPortDescriptor descp) +{ + delete reinterpret_cast(descp); +} + TSReturnCode TSPluginDescriptorAccept(TSCont contp) { diff --git a/src/api/InkAPITest.cc b/src/api/InkAPITest.cc index eac60ccd3fc..0c9d05cfdb3 100644 --- a/src/api/InkAPITest.cc +++ b/src/api/InkAPITest.cc @@ -1579,22 +1579,28 @@ REGRESSION_TEST(SDK_API_TSPortDescriptor)(RegressionTest *test, int /* atype ATS TSContDataSet(server_cont, params); TSContDataSet(client_cont, params); - port = TSPortDescriptorParse(nullptr); - if (port) { - SDK_RPRINT(test, "TSPortDescriptorParse", "NULL port descriptor", TC_FAIL, "TSPortDescriptorParse(NULL) returned %s", port); + if ((port = TSPortDescriptorParse(nullptr)) != nullptr) { + SDK_RPRINT(test, "TSPortDescriptorParse", "NULL port descriptor", TC_FAIL, "TSPortDescriptorParse(NULL) returned a descriptor"); + TSPortDescriptorDestroy(port); *pstatus = REGRESSION_TEST_FAILED; return; } snprintf(desc, sizeof(desc), "%u", params->port); - port = TSPortDescriptorParse(desc); + if ((port = TSPortDescriptorParse(desc)) == nullptr) { + SDK_RPRINT(test, "TSPortDescriptorParse", "Basic port descriptor", TC_FAIL, "TSPortDescriptorParse(%s) returned NULL", desc); + *pstatus = REGRESSION_TEST_FAILED; + return; + } if (TSPortDescriptorAccept(port, server_cont) == TS_ERROR) { - SDK_RPRINT(test, "TSPortDescriptorParse", "Basic port descriptor", TC_FAIL, "TSPortDescriptorParse(%s) returned TS_ERROR", + SDK_RPRINT(test, "TSPortDescriptorAccept", "Basic port descriptor", TC_FAIL, "TSPortDescriptorAccept(%s) returned TS_ERROR", desc); + TSPortDescriptorDestroy(port); *pstatus = REGRESSION_TEST_FAILED; return; } + TSPortDescriptorDestroy(port); IpEndpoint addr; ats_ip4_set(&addr, htonl(INADDR_LOOPBACK), htons(params->port)); diff --git a/tests/gold_tests/pluginTest/port_descriptor/port_descriptor.test.py b/tests/gold_tests/pluginTest/port_descriptor/port_descriptor.test.py new file mode 100644 index 00000000000..05efa1ae47f --- /dev/null +++ b/tests/gold_tests/pluginTest/port_descriptor/port_descriptor.test.py @@ -0,0 +1,62 @@ +''' +Verify that a plugin can listen on a port described by TSPortDescriptor. +''' +# 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 + +Test.Summary = 'Test the TSPortDescriptor API.' +Test.SkipUnless(Condition.HasProgram('nc', 'nc is required to connect to the plugin port')) + + +class TestPortDescriptor: + '''Verify that a plugin can accept connections on a parsed port.''' + + def __init__(self) -> None: + '''Configure the Traffic Server and client processes.''' + Test.GetTcpPort('descriptor_port') + tr = Test.AddTestRun('Connect to the plugin port') + self._ts = self._configure_traffic_server(tr) + self._configure_client(tr) + + def _configure_traffic_server(self, tr: 'TestRun') -> 'Process': + '''Configure Traffic Server with the port descriptor test plugin. + + :return: The Traffic Server process. + ''' + ts = tr.MakeATSProcess('ts', enable_cache=False) + plugin_path = os.path.join(Test.Variables.AtsTestPluginsDir, 'port_descriptor.so') + Test.PrepareTestPlugin(plugin_path, ts, f'{ts.Variables.descriptor_port}:ipv4') + ts.Disk.diags_log.Content += Testers.ContainsExpression( + r'port_descriptor.*accepted connection', 'Verify the plugin handled the accepted connection.') + ts.Disk.diags_log.Content += Testers.ExcludesExpression( + r'port_descriptor.*unexpected accept event', 'Verify the plugin received the expected accept event.') + return ts + + def _configure_client(self, tr: 'TestRun') -> 'Process': + '''Configure the client that connects to the plugin port. + + :return: The client process. + ''' + client = tr.Processes.Default + client.Command = f'nc -z 127.0.0.1 {self._ts.Variables.descriptor_port}' + client.ReturnCode = 0 + client.StartBefore(self._ts) + return client + + +TestPortDescriptor() diff --git a/tests/tools/plugins/CMakeLists.txt b/tests/tools/plugins/CMakeLists.txt index b7f18109ef0..fcad5aad972 100644 --- a/tests/tools/plugins/CMakeLists.txt +++ b/tests/tools/plugins/CMakeLists.txt @@ -27,6 +27,7 @@ add_autest_plugin(hook_add_plugin hook_add_plugin.cc) add_autest_plugin(http_alt_info_quality http_alt_info_quality.cc) add_autest_plugin(missing_mangled_definition missing_mangled_definition_c.c missing_mangled_definition_cpp.cc) add_autest_plugin(missing_ts_plugin_init missing_ts_plugin_init.cc) +add_autest_plugin(port_descriptor port_descriptor.cc) add_autest_plugin(server_packet_mark server_packet_mark.cc packet_mark_common.cc) add_autest_plugin(ssl_client_verify_test ssl_client_verify_test.cc) add_autest_plugin(ssl_hook_test ssl_hook_test.cc) diff --git a/tests/tools/plugins/port_descriptor.cc b/tests/tools/plugins/port_descriptor.cc new file mode 100644 index 00000000000..0f50c1b26a2 --- /dev/null +++ b/tests/tools/plugins/port_descriptor.cc @@ -0,0 +1,92 @@ +/** @file + + Test the TSPortDescriptor API. + + @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 + +namespace +{ +constexpr char PLUGIN_NAME[] = "port_descriptor"; + +int +accept_connection(TSCont /* contp */, TSEvent event, void *edata) +{ + if (event != TS_EVENT_NET_ACCEPT) { + TSError("[%s] unexpected accept event: %d", PLUGIN_NAME, event); + return TS_EVENT_ERROR; + } + + TSStatus("[%s] accepted connection", PLUGIN_NAME); + TSVConnClose(static_cast(edata)); + return TS_EVENT_NONE; +} +} // namespace + +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info{PLUGIN_NAME, "Apache Software Foundation", "dev@trafficserver.apache.org"}; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] plugin registration failed", PLUGIN_NAME); + return; + } + if (argc != 2) { + TSError("[%s] expected a single port descriptor argument", PLUGIN_NAME); + return; + } + + TSCont contp = TSContCreate(accept_connection, TSMutexCreate()); + + if (TSPortDescriptor descriptor = TSPortDescriptorParse(nullptr); descriptor != nullptr) { + TSError("[%s] parsing a null descriptor unexpectedly succeeded", PLUGIN_NAME); + TSPortDescriptorDestroy(descriptor); + TSContDestroy(contp); + return; + } + if (TSPortDescriptor descriptor = TSPortDescriptorParse("fd=5"); descriptor != nullptr) { + TSError("[%s] parsing an fd-only descriptor unexpectedly succeeded", PLUGIN_NAME); + TSPortDescriptorDestroy(descriptor); + TSContDestroy(contp); + return; + } + if (TSPortDescriptorAccept(nullptr, contp) != TS_ERROR) { + TSError("[%s] accepting a null descriptor unexpectedly succeeded", PLUGIN_NAME); + TSContDestroy(contp); + return; + } + + TSPortDescriptor descriptor = TSPortDescriptorParse(argv[1]); + if (descriptor == nullptr) { + TSError("[%s] failed to parse descriptor '%s'", PLUGIN_NAME, argv[1]); + TSContDestroy(contp); + return; + } + + if (TSPortDescriptorAccept(descriptor, nullptr) != TS_ERROR || TSPortDescriptorAccept(descriptor, contp) != TS_SUCCESS) { + TSError("[%s] failed to listen on descriptor '%s'", PLUGIN_NAME, argv[1]); + TSPortDescriptorDestroy(descriptor); + TSContDestroy(contp); + return; + } + TSPortDescriptorDestroy(descriptor); +}