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