Fixing TSHttpTxnServerAddrSet

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

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

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)

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

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 <ts/ts.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <cstdlib>
+
+#include <memory>
+#include <string_view>
+#include <string>
+
+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 const *>(&_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<TargetAddress> 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<TargetAddress>(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);
+}

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 } ]