diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index b77d3dfbc1e..1cc02b6bc34 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -7832,7 +7832,8 @@ HttpSM::set_next_state() t_state.dns_info.resolved_p = true; // seems dangerous - where's the IP address? call_transact_and_set_next_state(nullptr); break; - } else if (t_state.parent_result.result == PARENT_UNDEFINED && t_state.dns_info.resolve_immediate()) { + } else if (t_state.dns_info.resolved_p) { + SMDebug("dns", "Skipping DNS lookup because the address is already set."); call_transact_and_set_next_state(nullptr); break; } diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index b98fe2db371..3f851a271ca 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -21,6 +21,7 @@ limitations under the License. */ +#include "tscore/ink_inet.h" #include "tscpp/util/ts_bw_format.h" #include "ts/parentselectdefs.h" @@ -1829,8 +1830,8 @@ HttpTransact::PPDNSLookup(State *s) s->parent_info.dst_addr.network_order_port() = htons(s->parent_result.port); get_ka_info_from_host_db(s, &s->parent_info, &s->client_info, s->dns_info.active); - char addrbuf[INET6_ADDRSTRLEN]; - TxnDebug("http_trans", "DNS lookup for successful IP: %s", ats_ip_ntop(&s->parent_info.dst_addr.sa, addrbuf, sizeof(addrbuf))); + ip_port_text_buffer addrbuf; + TxnDebug("http_trans", "DNS lookup for successful IP: %s", ats_ip_nptop(&s->parent_info.dst_addr.sa, addrbuf, sizeof(addrbuf))); } // Since this function can be called several times while retrying @@ -1961,9 +1962,9 @@ HttpTransact::OSDNSLookup(State *s) ats_ip_copy(&s->request_data.dest_ip, &s->server_info.dst_addr); get_ka_info_from_host_db(s, &s->server_info, &s->client_info, s->dns_info.active); - char addrbuf[INET6_ADDRSTRLEN]; + ip_port_text_buffer addrbuf; TxnDebug("http_trans", "DNS lookup for O.S. successful IP: %s", - ats_ip_ntop(&s->server_info.dst_addr.sa, addrbuf, sizeof(addrbuf))); + ats_ip_nptop(&s->server_info.dst_addr.sa, addrbuf, sizeof(addrbuf))); if (s->redirect_info.redirect_in_process) { // If dns lookup was not successful, the code below will handle the error. @@ -1989,7 +1990,7 @@ HttpTransact::OSDNSLookup(State *s) } else { // Invalid server response, since we can't copy it we are going to reject TxnDebug("http_trans", "Invalid server response. Rejecting."); - Error("Invalid server response. Rejecting. IP: %s", ats_ip_ntop(&s->server_info.dst_addr.sa, addrbuf, sizeof(addrbuf))); + Error("Invalid server response. Rejecting. IP: %s", ats_ip_nptop(&s->server_info.dst_addr.sa, addrbuf, sizeof(addrbuf))); } build_error_response(s, HTTP_STATUS_FORBIDDEN, nullptr, "request#syntax_error"); SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD); @@ -3628,9 +3629,9 @@ HttpTransact::handle_response_from_parent(State *s) s->state_machine->do_hostdb_update_if_necessary(); } - char addrbuf[INET6_ADDRSTRLEN]; + ip_port_text_buffer addrbuf; TxnDebug("http_trans", "[%d] failed to connect to parent %s", s->current.retry_attempts.get(), - ats_ip_ntop(&s->current.server->dst_addr.sa, addrbuf, sizeof(addrbuf))); + ats_ip_nptop(&s->current.server->dst_addr.sa, addrbuf, sizeof(addrbuf))); // If the request is not retryable, just give up! if (!is_request_retryable(s)) { @@ -3815,9 +3816,9 @@ HttpTransact::handle_response_from_server(State *s) void HttpTransact::error_log_connection_failure(State *s, ServerState_t conn_state) { - char addrbuf[INET6_ADDRSTRLEN]; + ip_port_text_buffer addrbuf; TxnDebug("http_trans", "[%d] failed to connect [%d] to %s", s->current.retry_attempts.get(), conn_state, - ats_ip_ntop(&s->current.server->dst_addr.sa, addrbuf, sizeof(addrbuf))); + ats_ip_nptop(&s->current.server->dst_addr.sa, addrbuf, sizeof(addrbuf))); if (s->current.server->had_connect_fail()) { char *url_str = s->hdr_info.client_request.url_string_get(&s->arena); diff --git a/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt b/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt index 85799a39c8b..b0ee883557f 100644 --- a/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt +++ b/tests/gold_tests/pluginTest/tsapi/CMakeLists.txt @@ -16,3 +16,4 @@ ####################### add_autest_plugin(test_tsapi test_tsapi.cc) +add_autest_plugin(test_TSHttpTxnServerAddrSet test_TSHttpTxnServerAddrSet.cc) diff --git a/tests/gold_tests/pluginTest/tsapi/Makefile.inc b/tests/gold_tests/pluginTest/tsapi/Makefile.inc index c3fa39cc960..134371be93b 100644 --- a/tests/gold_tests/pluginTest/tsapi/Makefile.inc +++ b/tests/gold_tests/pluginTest/tsapi/Makefile.inc @@ -16,3 +16,7 @@ noinst_LTLIBRARIES += gold_tests/pluginTest/tsapi/test_tsapi.la gold_tests_pluginTest_tsapi_test_tsapi_la_SOURCES = gold_tests/pluginTest/tsapi/test_tsapi.cc + +noinst_LTLIBRARIES += gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.la +gold_tests_pluginTest_tsapi_test_TSHttpTxnServerAddrSet_la_SOURCES = \ + gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.cc diff --git a/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.cc b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.cc new file mode 100644 index 00000000000..96217ec9f39 --- /dev/null +++ b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.cc @@ -0,0 +1,176 @@ +/* + * 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 + +#include +#include +#include + +#include + +#include +#include +#include + +namespace +{ + +std::string_view PNAME = "test_plugin"; + +/** Represents the origin address and port that the user specified to which to + * redirect requests. */ +class TargetAddress +{ +public: + /** Constructs a TargetAddress object from the given address and port. + * + * @param address The address to which to redirect requests. + * @param port The port to which to redirect requests. + */ + TargetAddress(char const *address, int port) : _address(address), _port(port) + { + _sockaddr.sin_family = AF_INET; + _sockaddr.sin_port = htons(port); + if (inet_pton(AF_INET, address, &_sockaddr.sin_addr) != 1) { + TSError("Invalid address %s", address); + _is_valid = false; + } else { + _is_valid = true; + } + } + + /** Returns the address to which to redirect requests. + * + * @return The address to which to redirect requests. + */ + std::string_view + get_address() const + { + return _address; + } + + /** Returns the port to which to redirect requests. + * + * @return The port to which to redirect requests. + */ + int + get_port() const + { + return _port; + } + + /** Returns whether the address and port are valid. + * + * @return Whether the address and port are valid. + */ + bool + is_valid() const + { + return _is_valid; + } + + /** Returns the sockaddr representing the user's specified origin address and + * port. + * + * @return The sockaddr representing the user's specified origin address and + * port. + */ + sockaddr const * + get_sockaddr() const + { + return reinterpret_cast(&_sockaddr); + } + +private: + std::string _address; + int _port; + sockaddr_in _sockaddr; + bool _is_valid; +}; + +/** The user specified origin to which requests are redirected. */ +std::unique_ptr g_target_address{nullptr}; + +/** Parse the plugin's command line arguments. */ +bool +parse_arguments(int argc, char const *argv[]) +{ + if (argc != 3) { + TSError("Must provide the address and port for TSHttpTxnServerAddrSet."); + return false; + } + char const *address = argv[1]; + int port = atoi(argv[2]); + if (port <= 0 || port >= 65536) { + TSError("Invalid port number %s", argv[2]); + return false; + } + g_target_address = std::make_unique(address, port); + if (!g_target_address->is_valid()) { + TSError("Invalid address %s:%d", address, port); + return false; + } + + return true; +} + +/** The handler which sets the user-specified origin. */ +int +set_origin(TSCont cont, TSEvent event, void *edata) +{ + if (event != TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE) { + TSError("Unexpected event: %d", event); + return TS_ERROR; + } + + TSHttpTxn txnp = (TSHttpTxn)edata; + if (TSHttpTxnServerAddrSet(txnp, g_target_address->get_sockaddr()) != TS_SUCCESS) { + TSError("Failed to set a transaction's origin to %s:%d", g_target_address->get_address().data(), g_target_address->get_port()); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR); + return TS_ERROR; + } + TSDebug(PNAME.data(), "Successfully set a transaction's origin to %s:%d", g_target_address->get_address().data(), + g_target_address->get_port()); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return TS_SUCCESS; +} + +} // anonymous namespace + +void +TSPluginInit(int argc, char const *argv[]) +{ + TSDebug(PNAME.data(), "Initializing plugin."); + + TSPluginRegistrationInfo info; + info.plugin_name = PNAME.data(); + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + TSReleaseAssert(TSPluginRegister(&info) == TS_SUCCESS); + + if (!parse_arguments(argc, argv)) { + TSError("Failed to parse arguments."); + return; + } + + TSDebug(PNAME.data(), "Redirecting all requests to %s:%s", argv[1], argv[2]); + + TSCont contp = TSContCreate(set_origin, nullptr); + TSHttpHookAdd(TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, contp); +} diff --git a/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.replay.yaml b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.replay.yaml new file mode 100644 index 00000000000..df25fcd98e1 --- /dev/null +++ b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.replay.yaml @@ -0,0 +1,34 @@ +meta: + version: '1.0' + +sessions: + +- transactions: + + - client-request: + method: GET + url: /pictures/flower.jpeg + version: '1.1' + headers: + fields: + - [ Host, www.example.com ] + - [ uuid, first-request ] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Date, "Sat, 16 Mar 2019 03:11:36 GMT" ] + - [ Content-Type, image/jpeg ] + - [ Transfer-Encoding, chunked ] + - [ Connection, keep-alive ] + - [ X-Response, redirect-succeeded ] + content: + size: 45 + + proxy-response: + status: 200 + headers: + fields: + - [ X-Response, { value: redirect-succeeded, as: equal } ] diff --git a/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.test.py b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.test.py new file mode 100644 index 00000000000..8bd59d1428e --- /dev/null +++ b/tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet.test.py @@ -0,0 +1,102 @@ +# 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 +from ports import get_port + + +Test.Summary = ''' +Verify TSHttpTxnServerAddrSet() works as expected. +''' + +plugin_name = "test_TSHttpTxnServerAddrSet" + + +class TestTSHttpTxnServerAddrSet: + """Verify that TSHttpTxnServerAddrSet() works as expected.""" + _replay_file = "test_TSHttpTxnServerAddrSet.replay.yaml" + + def _configure_server(self, tr: 'TestRun') -> 'Process': + """Configure the server process for a TestRun. + + :param tr: The TestRun for which to configure the server process. + """ + self._server = tr.AddVerifierServerProcess("server", self._replay_file) + self._server.Streams.stdout += Testers.ContainsExpression( + "redirect-succeeded", + "Verify that the server sent the response.") + return self._server + + def _configure_dns(self, tr: 'TestRun') -> 'Process': + """Configure the DNS process for a TestRun. + + :param tr: The TestRun for which to configure the DNS process. + """ + 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. + + :param tr: The TestRun for which to configure the Traffic Server process. + """ + self._ts = tr.MakeATSProcess("ts") + 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} 127.0.0.1 {self._server.Variables.http_port}" + ) + + 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|hostdb|test_plugin', + }) + + # Remap to a nonexisting server and port. The plugin should use + # TSHttpTxnServerAddrSet() to set an actual server address and port. We + # use get_port to guarantee that no one is listing on this port. + bogus_port = get_port(self._server, "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("Verify TSHttpTxnServerAddrSet() works as expected.") + self._configure_dns(tr) + self._configure_server(tr) + self._configure_traffic_server(tr) + tr.AddVerifierClientProcess( + "client", + self._replay_file, + http_ports=[self._ts.Variables.port]) + + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + "redirect-succeeded", + "Verify that the client received the response.") + + tr.Processes.Default.StartBefore(self._dns) + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + + +test = TestTSHttpTxnServerAddrSet() +test.run()