diff --git a/doc/developer-guide/api/functions/TSHttpTxnServerAddrSet.en.rst b/doc/developer-guide/api/functions/TSHttpTxnServerAddrSet.en.rst index 16d48449dce..1eb9bbaa431 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 ba0c5d6b6fa..f7c3cf19de3 100644 --- a/include/proxy/http/HttpTransact.h +++ b/include/proxy/http/HttpTransact.h @@ -683,6 +683,7 @@ class HttpTransact 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 a911d82c0c2..aba3e30b7e5 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 e0f1e0030fe..516d89e514d 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 00000000000..fcb3547fa6e --- /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 + +#include +#include +#include + +#include +#include + +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(reinterpret_cast(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(static_cast(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(&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 = "dev@trafficserver.apache.org"; + 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 00000000000..c9588abd03d --- /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()