Add port descriptor destroy API

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,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 <ts/ts.h>
+
+.. 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)`

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
 ======================
 

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

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 */

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<TSPortDescriptor>(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<const HttpProxyPort *>(descp);
+
+  if (!is_usable_port_descriptor(port)) {
+    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()) {
@@ -7804,6 +7823,12 @@ TSPortDescriptorAccept(TSPortDescriptor descp, TSCont contp)
   return action ? TS_SUCCESS : TS_ERROR;
 }
 
+void
+TSPortDescriptorDestroy(TSPortDescriptor descp)
+{
+  delete reinterpret_cast<HttpProxyPort *>(descp);
+}
+
 TSReturnCode
 TSPluginDescriptorAccept(TSCont contp)
 {

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));

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()

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)