This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new bb34d9eec5 Fix retry logic for TSHttpTxnServerAddrSet (issue #12611) 
(#12733)
bb34d9eec5 is described below

commit bb34d9eec506e82db6394dc7df52b404c0c32bdb
Author: Bryan Call <[email protected]>
AuthorDate: Wed Dec 10 09:38:58 2025 -0800

    Fix retry logic for TSHttpTxnServerAddrSet (issue #12611) (#12733)
    
    When a plugin uses TSHttpTxnServerAddrSet() to set a server address and the
    connection fails, ATS should retry by calling the OS_DNS hook again to allow
    the plugin to provide an alternative address. However, the retry logic was
    missing handling for the USE_API case.
    
    Changes:
    - Added retry handling for ResolveInfo::OS_Addr::USE_API in 
handle_response_from_server()
    - Clears dns_info.resolved_p to trigger OS_DNS hook re-execution
    - Resets os_addr_style to TRY_DEFAULT to allow normal flow
    - Clears server_request to allow it to be rebuilt for new destination
    - Added api_server_addr_set_retried flag to prevent infinite retry loops
    - Added test plugin and autest to verify the fix
---
 .../api/functions/TSHttpTxnServerAddrSet.en.rst    |   8 +-
 include/proxy/http/HttpTransact.h                  |   1 +
 src/proxy/http/HttpTransact.cc                     |  12 ++
 tests/gold_tests/pluginTest/tsapi/CMakeLists.txt   |   1 +
 .../tsapi/test_TSHttpTxnServerAddrSet_retry.cc     | 169 +++++++++++++++++++++
 .../test_TSHttpTxnServerAddrSet_retry.test.py      |  87 +++++++++++
 6 files changed, 274 insertions(+), 4 deletions(-)

diff --git a/doc/developer-guide/api/functions/TSHttpTxnServerAddrSet.en.rst 
b/doc/developer-guide/api/functions/TSHttpTxnServerAddrSet.en.rst
index 16d48449dc..1eb9bbaa43 100644
--- a/doc/developer-guide/api/functions/TSHttpTxnServerAddrSet.en.rst
+++ b/doc/developer-guide/api/functions/TSHttpTxnServerAddrSet.en.rst
@@ -51,10 +51,10 @@ Notes
 =====
 
 If |TS| is configured to retry connections to origin servers and
-:func:`TSHttpTxnServerAddrGet` has been called, |TS| will return
-to TS_HTTP_OS_DNS_HOOK so to let the plugin set a different server
-address. Plugins should be prepared for TS_HTTP_OS_DNS_HOOK and any
-subsequent hooks to be called multiple times.
+:func:`TSHttpTxnServerAddrSet` has been called, |TS| will return
+to TS_HTTP_OS_DNS_HOOK once to let the plugin set a different server
+address. Plugins should be prepared for TS_HTTP_OS_DNS_HOOK to be
+called one additional time on connection failure.
 
 Once a plugin calls :func:`TSHttpTxnServerAddrGet` any prior DNS
 resolution results are lost. The plugin should use
diff --git a/include/proxy/http/HttpTransact.h 
b/include/proxy/http/HttpTransact.h
index 7034740c9f..64e13a991d 100644
--- a/include/proxy/http/HttpTransact.h
+++ b/include/proxy/http/HttpTransact.h
@@ -683,6 +683,7 @@ public:
     bool api_server_request_body_set  = false;
     bool api_req_cacheable            = false;
     bool api_resp_cacheable           = false;
+    bool api_server_addr_set_retried  = false;
     bool reverse_proxy                = false;
     bool url_remap_success            = false;
     bool api_skip_all_remapping       = false;
diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc
index a911d82c0c..aba3e30b7e 100644
--- a/src/proxy/http/HttpTransact.cc
+++ b/src/proxy/http/HttpTransact.cc
@@ -3770,6 +3770,18 @@ HttpTransact::handle_response_from_server(State *s)
         // families - that is locked in by the client source address.
         ats_force_order_by_family(s->current.server->dst_addr.family(), 
s->my_txn_conf().host_res_data.order);
         return CallOSDNSLookup(s);
+      } else if (ResolveInfo::OS_Addr::USE_API == s->dns_info.os_addr_style && 
!s->api_server_addr_set_retried) {
+        // Plugin set the server address via TSHttpTxnServerAddrSet(). Clear 
resolution
+        // state to allow the OS_DNS hook to be called again, giving the 
plugin a chance
+        // to set a different server address for retry (issue #12611).
+        // Only retry once to avoid infinite loops if the plugin keeps setting 
failing addresses.
+        s->api_server_addr_set_retried = true;
+        s->dns_info.resolved_p         = false;
+        s->dns_info.os_addr_style      = ResolveInfo::OS_Addr::TRY_DEFAULT;
+        // Clear the server request so it can be rebuilt for the new 
destination
+        s->hdr_info.server_request.destroy();
+        TxnDbg(dbg_ctl_http_trans, "Retrying with plugin-set address, 
returning to OS_DNS hook");
+        return CallOSDNSLookup(s);
       } else {
         if ((s->txn_conf->connect_attempts_rr_retries > 0) &&
             ((s->current.retry_attempts.get() + 1) % 
s->txn_conf->connect_attempts_rr_retries == 0)) {
diff --git a/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt 
b/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt
index e0f1e0030f..516d89e514 100644
--- a/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt
+++ b/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt
@@ -20,3 +20,4 @@ add_autest_plugin(test_TSHttpTxnServerAddrSet 
test_TSHttpTxnServerAddrSet.cc)
 add_autest_plugin(test_TSHttpSsnInfo test_TSHttpSsnInfo.cc)
 add_autest_plugin(test_TSVConnPPInfo test_TSVConnPPInfo.cc)
 add_autest_plugin(test_TSHttpTxnVerifiedAddr test_TSHttpTxnVerifiedAddr.cc)
+add_autest_plugin(test_TSHttpTxnServerAddrSet_retry 
test_TSHttpTxnServerAddrSet_retry.cc)
diff --git 
a/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.cc 
b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.cc
new file mode 100644
index 0000000000..fcb3547fa6
--- /dev/null
+++ b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.cc
@@ -0,0 +1,169 @@
+/*
+ * 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.
+ */
+
+/**
+ * Test plugin to reproduce issue #12611
+ *
+ * This plugin sets server addresses via TSHttpTxnServerAddrSet() in the 
OS_DNS hook.
+ * On first call, it sets a non-routable address that will fail to connect.
+ * On retry (if OS_DNS is called again), it sets a working address.
+ *
+ * BUG: On master, the OS_DNS hook is NOT called again on retry, so the 
connection
+ * keeps failing with the bad address.
+ */
+
+#include <ts/ts.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <cstdint>
+#include <cstdlib>
+
+namespace
+{
+
+DbgCtl dbg_ctl{"test_TSHttpTxnServerAddrSet_retry"};
+
+// Transaction argument index for per-transaction call count
+int g_txn_arg_idx = -1;
+
+/** Get the OS_DNS call count for this transaction */
+int
+get_txn_call_count(TSHttpTxn txnp)
+{
+  return static_cast<int>(reinterpret_cast<intptr_t>(TSUserArgGet(txnp, 
g_txn_arg_idx)));
+}
+
+/** Set the OS_DNS call count for this transaction */
+void
+set_txn_call_count(TSHttpTxn txnp, int count)
+{
+  TSUserArgSet(txnp, g_txn_arg_idx, reinterpret_cast<void 
*>(static_cast<intptr_t>(count)));
+}
+
+/** Handler for OS_DNS hook */
+int
+handle_os_dns(TSCont /* cont */, TSEvent event, void *edata)
+{
+  if (event != TS_EVENT_HTTP_OS_DNS) {
+    TSError("[TSHttpTxnServerAddrSet_retry] Unexpected event in OS_DNS 
handler: %d", event);
+    return TS_ERROR;
+  }
+
+  TSHttpTxn txnp = (TSHttpTxn)edata;
+
+  // Increment per-transaction call count
+  int call_count = get_txn_call_count(txnp) + 1;
+  set_txn_call_count(txnp, call_count);
+
+  Dbg(dbg_ctl, "OS_DNS hook called, count=%d", call_count);
+  TSError("[TSHttpTxnServerAddrSet_retry] OS_DNS hook called, count=%d", 
call_count);
+
+  // First call: set a bad address (192.0.2.1 is TEST-NET-1, non-routable)
+  // Second call: set a good address (localhost)
+  const char *addr_str;
+  int         port;
+
+  if (call_count == 1) {
+    addr_str = "192.0.2.1"; // Non-routable - will fail
+    port     = 80;
+    TSError("[TSHttpTxnServerAddrSet_retry] Attempt 1: Setting BAD address 
%s:%d (will fail)", addr_str, port);
+  } else {
+    addr_str = "127.0.0.1"; // Localhost - should work
+    port     = 8080;
+    TSError("[TSHttpTxnServerAddrSet_retry] Attempt %d: Setting GOOD address 
%s:%d (should work)", call_count, addr_str, port);
+  }
+
+  // Create sockaddr
+  struct sockaddr_in sa;
+  sa.sin_family = AF_INET;
+  sa.sin_port   = htons(port);
+  if (inet_pton(AF_INET, addr_str, &sa.sin_addr) != 1) {
+    TSError("[TSHttpTxnServerAddrSet_retry] Invalid address %s", addr_str);
+    TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR);
+    return TS_ERROR;
+  }
+
+  // Set the server address
+  if (TSHttpTxnServerAddrSet(txnp, reinterpret_cast<struct sockaddr const 
*>(&sa)) != TS_SUCCESS) {
+    TSError("[TSHttpTxnServerAddrSet_retry] Failed to set server address to 
%s:%d", addr_str, port);
+    TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR);
+    return TS_ERROR;
+  }
+
+  Dbg(dbg_ctl, "Set server address to %s:%d", addr_str, port);
+
+  TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+  return TS_SUCCESS;
+}
+
+/** Handler for TXN_CLOSE hook - report results */
+int
+handle_txn_close(TSCont /* cont */, TSEvent event, void *edata)
+{
+  if (event != TS_EVENT_HTTP_TXN_CLOSE) {
+    TSError("[TSHttpTxnServerAddrSet_retry] Unexpected event in TXN_CLOSE 
handler: %d", event);
+    return TS_ERROR;
+  }
+
+  TSHttpTxn txnp = (TSHttpTxn)edata;
+
+  // Get the per-transaction call count
+  int call_count = get_txn_call_count(txnp);
+
+  TSError("[TSHttpTxnServerAddrSet_retry] Transaction closing. OS_DNS was 
called %d time(s)", call_count);
+
+  if (call_count == 1) {
+    TSError("[TSHttpTxnServerAddrSet_retry] *** BUG CONFIRMED: OS_DNS hook was 
only called ONCE. "
+            "Plugin could not retry with different address. This is issue 
#12611. ***");
+  } else if (call_count >= 2) {
+    TSError("[TSHttpTxnServerAddrSet_retry] SUCCESS: OS_DNS hook was called %d 
times. "
+            "Plugin was able to retry with different address.",
+            call_count);
+  }
+
+  TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+  return TS_SUCCESS;
+}
+
+} // anonymous namespace
+
+void
+TSPluginInit(int /* argc */, char const * /* argv */[])
+{
+  Dbg(dbg_ctl, "Initializing plugin to reproduce issue #12611");
+  TSError("[TSHttpTxnServerAddrSet_retry] Plugin initialized - will test 
TSHttpTxnServerAddrSet retry behavior");
+
+  TSPluginRegistrationInfo info;
+  info.plugin_name   = "test_TSHttpTxnServerAddrSet_retry";
+  info.vendor_name   = "Apache Software Foundation";
+  info.support_email = "[email protected]";
+  TSReleaseAssert(TSPluginRegister(&info) == TS_SUCCESS);
+
+  // Reserve a transaction argument slot for per-transaction call count
+  TSReleaseAssert(TSUserArgIndexReserve(TS_USER_ARGS_TXN, 
"test_TSHttpTxnServerAddrSet_retry", "OS_DNS call count",
+                                        &g_txn_arg_idx) == TS_SUCCESS);
+
+  TSCont os_dns_cont = TSContCreate(handle_os_dns, nullptr);
+  TSHttpHookAdd(TS_HTTP_OS_DNS_HOOK, os_dns_cont);
+
+  TSCont close_cont = TSContCreate(handle_txn_close, nullptr);
+  TSHttpHookAdd(TS_HTTP_TXN_CLOSE_HOOK, close_cont);
+}
diff --git 
a/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.test.py 
b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.test.py
new file mode 100644
index 0000000000..c9588abd03
--- /dev/null
+++ 
b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.test.py
@@ -0,0 +1,87 @@
+#  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.
+'''
+Test to reproduce issue #12611 - retry fails when TSHttpTxnServerAddrSet is 
used
+'''
+
+import os
+from ports import get_port
+
+Test.Summary = '''
+Reproduce issue #12611: OS_DNS hook is not called again on retry when using 
TSHttpTxnServerAddrSet
+'''
+
+plugin_name = "test_TSHttpTxnServerAddrSet_retry"
+
+
+class TestIssue12611:
+    """Reproduce issue #12611: retry fails when TSHttpTxnServerAddrSet is 
used."""
+
+    def _configure_dns(self, tr: 'TestRun') -> 'Process':
+        """Configure the DNS process for a TestRun."""
+        self._dns = tr.MakeDNServer("dns", default=['127.0.0.1'])
+        return self._dns
+
+    def _configure_traffic_server(self, tr: 'TestRun') -> 'Process':
+        """Configure the Traffic Server process for a TestRun."""
+        self._ts = tr.MakeATSProcess("ts", enable_cache=False)
+        ts = self._ts
+
+        plugin_path = os.path.join(Test.Variables.AtsBuildGoldTestsDir, 
'pluginTest', 'tsapi', '.libs', f'{plugin_name}.so')
+        ts.Setup.Copy(plugin_path, ts.Env['PROXY_CONFIG_PLUGIN_PLUGIN_DIR'])
+        ts.Disk.plugin_config.AddLine(f"{plugin_path}")
+
+        ts.Disk.records_config.update(
+            {
+                'proxy.config.dns.nameservers': 
f'127.0.0.1:{self._dns.Variables.Port}',
+                'proxy.config.dns.resolv_conf': 'NULL',
+                'proxy.config.diags.debug.enabled': 1,
+                'proxy.config.diags.debug.tags': 
'http|dns|test_TSHttpTxnServerAddrSet_retry',
+                # Enable retries so we can see if OS_DNS is called multiple 
times
+                'proxy.config.http.connect_attempts_max_retries': 3,
+                'proxy.config.http.connect_attempts_timeout': 1,
+            })
+
+        # Remap to a nonexisting server - the plugin will set addresses
+        bogus_port = get_port(ts, "bogus_port")
+        ts.Disk.remap_config.AddLine(f'map / 
http://non.existent.server.com:{bogus_port}')
+
+    def run(self):
+        """Configure the TestRun."""
+        tr = Test.AddTestRun("Reproduce issue #12611")
+        self._configure_dns(tr)
+        self._configure_traffic_server(tr)
+
+        # Make a simple request - it should fail since first address is 
non-routable
+        # but the plugin should log whether OS_DNS was called multiple times
+        tr.Processes.Default.Command = f'curl -vs --connect-timeout 5 
http://127.0.0.1:{self._ts.Variables.port}/ -o /dev/null 2>&1; true'
+        tr.Processes.Default.ReturnCode = 0
+
+        tr.Processes.Default.StartBefore(self._dns)
+        tr.Processes.Default.StartBefore(self._ts)
+
+        # Override the default diags.log error check - we use TSError() for 
test output
+        # Using '=' instead of '+=' replaces the default "no errors" check
+        self._ts.Disk.diags_log.Content = Testers.ContainsExpression("OS_DNS 
hook called, count=1", "First OS_DNS call logged")
+
+        # This message indicates the fix works - OS_DNS was called multiple 
times
+        # Test will FAIL on master (bug exists), PASS after fix is applied
+        self._ts.Disk.diags_log.Content += Testers.ContainsExpression(
+            "SUCCESS: OS_DNS hook was called", "Plugin was able to retry with 
different address")
+
+
+test = TestIssue12611()
+test.run()

Reply via email to