Make port descriptors caller-owned

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

doc/developer-guide/api/functions/TSPortDescriptorParse.en.rst

@@ -0,0 +1,96 @@
+.. 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 <ts/ts.h>
+
+.. class:: TSPortDescriptor
+.. function:: TSReturnCode TSPortDescriptorParse(const char *descriptor, TSPortDescriptor *result)
+.. function:: TSReturnCode TSPortDescriptorAccept(const TSPortDescriptor *descriptor, TSCont contp)
+
+Description
+===========
+
+:func:`TSPortDescriptorParse` parses the same descriptor syntax used by
+:ts:cv:`proxy.config.http.server_ports`. The parsed representation is written
+to caller-owned :type:`TSPortDescriptor` storage. A descriptor must be
+successfully parsed before it is passed to :func:`TSPortDescriptorAccept`.
+
+The API does not allocate or free the :type:`TSPortDescriptor` object. No
+separate destruction function is required. Its memory is released according
+to its normal C++ storage duration: an automatic object is released when it
+leaves scope, and a dynamically allocated object is released when the plugin
+deletes it. Calling :func:`TSPortDescriptorParse` again reuses the same
+descriptor storage.
+
+:func:`TSPortDescriptorAccept` copies the information it needs from
+:arg:`descriptor` and does not retain a pointer to it. The descriptor can
+therefore leave scope immediately after :func:`TSPortDescriptorAccept`
+returns, regardless of whether the listener remains active.
+
+For example, the descriptor in this function is released when the function
+returns while the listener continues to accept connections:
+
+.. code-block:: cpp
+
+    TSReturnCode
+    listen_on_descriptor(TSCont contp, const char *spec)
+    {
+      TSPortDescriptor descriptor;
+
+      if (TSPortDescriptorParse(spec, &descriptor) != TS_SUCCESS) {
+        return TS_ERROR;
+      }
+
+      return TSPortDescriptorAccept(&descriptor, contp);
+    }
+
+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 :enumerator:`TS_SUCCESS` when
+:arg:`descriptor` was parsed successfully. It returns :enumerator:`TS_ERROR`
+for a null argument or invalid descriptor. After a failed parse,
+:arg:`result` remains invalid until it is parsed successfully.
+
+:func:`TSPortDescriptorAccept` returns :enumerator:`TS_SUCCESS` when the
+listener was opened. It returns :enumerator:`TS_ERROR` for a null argument, a
+descriptor that was not successfully parsed, an unusable listen endpoint, or
+an error opening the listener.
+
+See Also
+========
+
+:manpage:`TSAPI(3ts)`,
+:manpage:`TSNetAccept(3ts)`,
+:manpage:`records.yaml(5)`

example/plugins/c-api/passthru/passthru.cc

@@ -296,17 +296,16 @@ PassthruAccept(TSCont /* cont */, TSEvent event, void *edata)
 static TSReturnCode
 PassthruListen()
 {
-  TSMgmtString     ports      = nullptr;
-  TSPortDescriptor descriptor = nullptr;
-  TSCont           cont       = nullptr;
+  TSMgmtString     ports = nullptr;
+  TSPortDescriptor descriptor;
+  TSCont           cont = 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);
-  if (descriptor == nullptr) {
+  if (TSPortDescriptorParse(ports, &descriptor) != TS_SUCCESS) {
     TSError("[%s] failed to parse config.plugin.passthru.server_ports", PLUGIN_NAME);
     TSfree(ports);
     return TS_ERROR;
@@ -316,7 +315,7 @@ PassthruListen()
   TSfree(ports);
 
   cont = TSContCreate(PassthruAccept, nullptr);
-  return TSPortDescriptorAccept(descriptor, cont);
+  return TSPortDescriptorAccept(&descriptor, cont);
 }
 
 void

include/ts/apidefs.h.in

@@ -42,6 +42,7 @@
  *
  */
 
+#include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -1139,7 +1140,21 @@ using TSCacheHttpInfo = struct tsapi_cachehttpinfo *;
 using TSCacheTxn      = struct tsapi_cachetxn *;
 using TSSslVerifyCTX  = struct tsapi_x509_store_ctx *;
 
-using TSPortDescriptor   = struct tsapi_port *;
+/** Caller-owned storage for a parsed proxy port descriptor.
+ *
+ * The contents are opaque and must only be initialized and accessed through
+ * the TSPortDescriptor API.
+ */
+class alignas(std::uint64_t) TSPortDescriptor
+{
+  friend TSReturnCode TSPortDescriptorParse(const char *, TSPortDescriptor *);
+  friend TSReturnCode TSPortDescriptorAccept(const TSPortDescriptor *, struct tsapi_cont *);
+
+private:
+  std::byte _opaque[216];
+  bool      _is_valid{false};
+};
+
 using TSVIO              = struct tsapi_vio *;
 using TSThread           = struct tsapi_thread *;
 using TSEventThread      = struct tsapi_event_thread *;

include/ts/ts.h

@@ -2187,19 +2187,32 @@ 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.
+/** Parse a port descriptor.
+ *
+ * Parse the string specification used by the
+ * @c proxy.config.http.server_ports configuration value into caller-owned
+ * storage. The API does not allocate or free this storage, and no separate
+ * destruction function is required.
+ *
+ * @param[in] descriptor Port descriptor string to parse.
+ * @param[out] result Storage for the parsed port descriptor.
+ * @return @c TS_SUCCESS if @a descriptor was parsed, @c TS_ERROR otherwise.
  */
-TSPortDescriptor TSPortDescriptorParse(const char *descriptor);
+TSReturnCode TSPortDescriptorParse(const char *descriptor, TSPortDescriptor *result);
 
-/**
-   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. Neither the descriptor nor its storage is retained
+ * after this function returns. The descriptor can therefore be destroyed or
+ * reused 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, TSCont);
+TSReturnCode TSPortDescriptorAccept(const TSPortDescriptor *descriptor, TSCont contp);
 
 /* --------------------------------------------------------------------------
    DNS Lookups */

src/api/InkAPI.cc

@@ -23,10 +23,12 @@
 
 #include <atomic>
 #include <charconv>
+#include <new>
 #include <tuple>
 #include <unordered_map>
 #include <string_view>
 #include <string>
+#include <type_traits>
 #include <utility>
 
 #include "iocore/net/NetVConnection.h"
@@ -7775,24 +7777,43 @@ TSHttpTxnCloseAfterResponse(TSHttpTxn txnp, int should_close)
 }
 
 // Parse a port descriptor for the proxy.config.http.server_ports descriptor format.
-TSPortDescriptor
-TSPortDescriptorParse(const char *descriptor)
+TSReturnCode
+TSPortDescriptorParse(const char *descriptor, TSPortDescriptor *result)
 {
-  HttpProxyPort *port = new HttpProxyPort();
+  static_assert(sizeof(result->_opaque) >= sizeof(HttpProxyPort));
+  static_assert(alignof(TSPortDescriptor) >= alignof(HttpProxyPort));
+  static_assert(std::is_trivially_destructible_v<HttpProxyPort>);
 
-  if (descriptor && port->processOptions(descriptor)) {
-    return reinterpret_cast<TSPortDescriptor>(port);
+  if (result == nullptr) {
+    return TS_ERROR;
   }
 
-  delete port;
-  return nullptr;
+  result->_is_valid = false;
+  if (descriptor == nullptr) {
+    return TS_ERROR;
+  }
+
+  auto *port = new (result->_opaque) HttpProxyPort();
+
+  result->_is_valid = port->processOptions(descriptor);
+  return result->_is_valid ? TS_SUCCESS : TS_ERROR;
 }
 
 TSReturnCode
-TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
+TSPortDescriptorAccept(const TSPortDescriptor *descp, TSCont contp)
 {
+  if (descp == nullptr || contp == nullptr || !descp->_is_valid) {
+    return TS_ERROR;
+  }
+
+  const HttpProxyPort *port = std::launder(reinterpret_cast<const HttpProxyPort *>(descp->_opaque));
+
+  if ((port->m_family != AF_INET && port->m_family != AF_INET6 && port->m_family != AF_UNIX) ||
+      (port->m_family != AF_UNIX && port->m_port == 0)) {
+    return TS_ERROR;
+  }
+
   Action                     *action = nullptr;
-  HttpProxyPort              *port   = reinterpret_cast<HttpProxyPort *>(descp);
   NetProcessor::AcceptOptions net(make_net_accept_options(port, -1 /* nthreads */));
 
   if (port->isSSL()) {

src/api/InkAPITest.cc

@@ -1579,23 +1579,27 @@ 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 (TSPortDescriptorParse(nullptr, &port) != TS_ERROR) {
+    SDK_RPRINT(test, "TSPortDescriptorParse", "NULL port descriptor", TC_FAIL, "TSPortDescriptorParse(NULL) returned TS_SUCCESS");
     *pstatus = REGRESSION_TEST_FAILED;
     return;
   }
 
   snprintf(desc, sizeof(desc), "%u", params->port);
-  port = TSPortDescriptorParse(desc);
-
-  if (TSPortDescriptorAccept(port, server_cont) == TS_ERROR) {
+  if (TSPortDescriptorParse(desc, &port) != TS_SUCCESS) {
     SDK_RPRINT(test, "TSPortDescriptorParse", "Basic port descriptor", TC_FAIL, "TSPortDescriptorParse(%s) returned TS_ERROR",
                desc);
     *pstatus = REGRESSION_TEST_FAILED;
     return;
   }
 
+  if (TSPortDescriptorAccept(&port, server_cont) == TS_ERROR) {
+    SDK_RPRINT(test, "TSPortDescriptorAccept", "Basic port descriptor", TC_FAIL, "TSPortDescriptorAccept(%s) returned TS_ERROR",
+               desc);
+    *pstatus = REGRESSION_TEST_FAILED;
+    return;
+  }
+
   IpEndpoint addr;
   ats_ip4_set(&addr, htonl(INADDR_LOOPBACK), htons(params->port));
   TSNetConnect(client_cont, &addr.sa);

tests/gold_tests/pluginTest/port_descriptor/port_descriptor.test.py

@@ -0,0 +1,58 @@
+'''
+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')
+        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()

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)