diff --git a/include/tscpp/util/TextView.h b/include/tscpp/util/TextView.h index d2c31ca47a4..637dfa10d8b 100644 --- a/include/tscpp/util/TextView.h +++ b/include/tscpp/util/TextView.h @@ -1265,6 +1265,32 @@ namespace literals constexpr ts::TextView operator"" _tv(const char *s, size_t n) { return {s, n}; } } // namespace literals +/** Functor for STL containers that need caseless comparisons of standard string types. + * + * For example a @c std::set of strings with caseless comparison would be + * + * @code + * std::set strings; + * @endcode + */ +struct caseless_compare { + bool + operator()(std::string_view const &lhs, std::string_view const &rhs) const + { + return strcasecmp(lhs, rhs); + } + bool + operator()(TextView const &lhs, TextView const &rhs) const + { + return strcasecmp(lhs, rhs); + } + bool + operator()(std::string const &lhs, std::string const &rhs) const + { + return strcasecmp(lhs, rhs); + } +}; + } // namespace ts namespace std diff --git a/iocore/utils/I_Machine.h b/iocore/utils/I_Machine.h index c744cd78cf4..caf64290cb0 100644 --- a/iocore/utils/I_Machine.h +++ b/iocore/utils/I_Machine.h @@ -33,9 +33,10 @@ #include "tscore/ink_inet.h" #include "tscore/ink_uuid.h" -#include #include #include +#include +#include /** The Machine is a simple place holder for the hostname and the ip @@ -52,21 +53,14 @@ */ struct Machine { - typedef Machine self; ///< Self reference type. - - char *hostname; // name of the internet host - int hostname_len; // size of the string pointed to by hostname + using self_type = Machine; ///< Self reference type. IpEndpoint ip; ///< Preferred IP address of the host (network order) IpEndpoint ip4; ///< IPv4 address if present. IpEndpoint ip6; ///< IPv6 address if present. - ip_text_buffer ip_string; // IP address of the host as a string. - int ip_string_len; - - char ip_hex_string[TS_IP6_SIZE * 2 + 1]; ///< IP address as hex string - int ip_hex_string_len; - + std::string host_name; + std::string ip_hex_string; ///< IP address as hex string ATSUuid uuid; ~Machine(); @@ -77,22 +71,22 @@ struct Machine { @note This must be called before called @c instance so that the singleton is not @em inadvertently default initialized. */ - static self *init(char const *name = nullptr, ///< Host name of the machine. - sockaddr const *addr = nullptr ///< Primary IP address of the machine. + static self_type *init(char const *name = nullptr, ///< Host name of the machine. + sockaddr const *addr = nullptr ///< Primary IP address of the machine. ); /// @return The global instance of this class. - static self *instance(); - bool is_self(const char *name); - bool is_self(const char *name, int name_len); - bool is_self(const IpAddr *ipaddr); + static self_type *instance(); + bool is_self(std::string_view name); + bool is_self(std::string const &name); + bool is_self(IpAddr const &ipaddr); bool is_self(struct sockaddr const *addr); - void insert_id(char *id); - void insert_id(IpAddr *ipaddr); + void insert_id(char const *id); + void insert_id(IpAddr const &ipaddr); protected: Machine(char const *hostname, sockaddr const *addr); - static self *_instance; ///< Singleton for the class. - std::unordered_set machine_id_strings; - std::unordered_map machine_id_ipaddrs; + static self_type *_instance; ///< Singleton for the class. + std::unordered_set, ts::caseless_compare> machine_id_strings; + std::unordered_set machine_id_ipaddrs; }; diff --git a/iocore/utils/Machine.cc b/iocore/utils/Machine.cc index ddb7942f0cb..2867cc52910 100644 --- a/iocore/utils/Machine.cc +++ b/iocore/utils/Machine.cc @@ -31,21 +31,6 @@ #include #endif -static void -make_to_lower_case(const char *name, int name_len, char *lower_case_name, int buf_len) -{ - int i; - - if (name_len > (buf_len - 1)) { - name_len = buf_len - 1; - } - - for (i = 0; i < name_len; i++) { - lower_case_name[i] = ParseRules::ink_tolower(name[i]); - } - lower_case_name[i] = '\0'; -} - // Singleton Machine *Machine::_instance = nullptr; @@ -65,29 +50,23 @@ Machine::init(char const *name, sockaddr const *ip) } Machine::Machine(char const *the_hostname, sockaddr const *addr) - : hostname(nullptr), hostname_len(0), ip_string_len(0), ip_hex_string_len(0) { - char localhost[1024]; - char ip_strbuf[INET6_ADDRSTRLEN]; int status; // return for system calls. - - ip_string[0] = 0; - ip_hex_string[0] = 0; - ink_zero(ip); - ink_zero(ip4); - ink_zero(ip6); + ip_text_buffer ip_strbuf; + char localhost[1024]; uuid.initialize(TS_UUID_V4); ink_release_assert(nullptr != uuid.getString()); // The Process UUID must be available on startup - localhost[sizeof(localhost) - 1] = 0; // ensure termination. - if (!ats_is_ip(addr)) { if (!the_hostname) { - ink_release_assert(!gethostname(localhost, sizeof(localhost) - 1)); - the_hostname = localhost; + // @c gethostname has a broken interface - there's no way to determine the actual size of + // the host name explicitly - the error case doesn't return the size. The standards based + // limit is 63, or 255 for a FQDN. + auto result = gethostname(localhost, sizeof(localhost)); + ink_release_assert(result == 0); + host_name.assign(localhost, result); } - hostname = ats_strdup(the_hostname); #if HAVE_IFADDRS_H ifaddrs *ifa_addrs = nullptr; @@ -98,7 +77,7 @@ Machine::Machine(char const *the_hostname, sockaddr const *addr) // you would expect. On a normal system with just two interfaces and // one address / interface the return count is 120. Stack space is // cheap so it's best to go big. - static const int N_REQ = 1024; + static constexpr int N_REQ = 1024; ifconf conf; ifreq req[N_REQ]; if (0 <= s) { @@ -111,7 +90,8 @@ Machine::Machine(char const *the_hostname, sockaddr const *addr) #endif if (0 != status) { - Warning("Unable to determine local host '%s' address information - %s", hostname, strerror(errno)); + Warning("Unable to determine local host '%.*s' address information - %s", int(host_name.size()), host_name.data(), + strerror(errno)); } else { // Loop through the interface addresses and prefer by type. enum { @@ -172,8 +152,7 @@ Machine::Machine(char const *the_hostname, sockaddr const *addr) if (spot_type != LL && getnameinfo(ifip, ats_ip_size(ifip), localhost, sizeof(localhost) - 1, nullptr, 0, 0) == 0) { insert_id(localhost); } - IpAddr *ipaddr = new IpAddr(ifip); - insert_id(ipaddr); + insert_id(IpAddr(ifip)); if (ats_is_ip4(ifip)) { if (spot_type > ip4_type) { ats_ip_copy(&ip4, ifip); @@ -216,86 +195,53 @@ Machine::Machine(char const *the_hostname, sockaddr const *addr) ip_text_buffer ipbuff; Warning("Failed to find hostname for address '%s' - %s", ats_ip_ntop(addr, ipbuff, sizeof(ipbuff)), gai_strerror(status)); } else { - hostname = ats_strdup(localhost); + host_name.assign(localhost); } } - hostname_len = hostname ? strlen(hostname) : 0; - - ats_ip_ntop(&ip.sa, ip_string, sizeof(ip_string)); - ip_string_len = strlen(ip_string); - ip_hex_string_len = ats_ip_to_hex(&ip.sa, ip_hex_string, sizeof(ip_hex_string)); + char hex_buff[TS_IP6_SIZE * 2 + 1]; + ats_ip_to_hex(&ip.sa, hex_buff, sizeof(hex_buff)); + ip_hex_string.assign(hex_buff); } -Machine::~Machine() -{ - ats_free(hostname); - for (auto &machine_id_ipaddr : machine_id_ipaddrs) { - delete machine_id_ipaddr.second; - } -} +Machine::~Machine() {} bool -Machine::is_self(const char *name) +Machine::is_self(std::string const &name) { - return is_self(name, strlen(name)); + return machine_id_strings.find(name) != machine_id_strings.end(); } bool -Machine::is_self(const char *name, int name_len) +Machine::is_self(std::string_view name) { - char lower_case_name[TS_MAX_HOST_NAME_LEN + 1] = {0}; - - if (name_len == 0) { - return false; - } - - make_to_lower_case(name, name_len, lower_case_name, sizeof(lower_case_name)); - - return machine_id_strings.find(lower_case_name) != machine_id_strings.end(); + return this->is_self(std::string(name)); } bool -Machine::is_self(const IpAddr *ipaddr) +Machine::is_self(IpAddr const &ipaddr) { - char string_value[INET6_ADDRSTRLEN + 1] = {0}; - - if (ipaddr == nullptr) { - return false; - } - ipaddr->toString(string_value, sizeof(string_value)); - return machine_id_ipaddrs.find(string_value) != machine_id_ipaddrs.end(); + return machine_id_ipaddrs.end() != machine_id_ipaddrs.find(ipaddr); } bool Machine::is_self(struct sockaddr const *addr) { - char string_value[INET6_ADDRSTRLEN + 1] = {0}; - - if (addr == nullptr) { - return false; - } - ats_ip_ntop(addr, string_value, sizeof(string_value)); - return machine_id_ipaddrs.find(string_value) != machine_id_ipaddrs.end(); + return machine_id_ipaddrs.find(IpAddr(addr)) != machine_id_ipaddrs.end(); } void -Machine::insert_id(char *id) +Machine::insert_id(char const *id) { - char lower_case_name[TS_MAX_HOST_NAME_LEN + 1] = {0}; - - make_to_lower_case(id, strlen(id), lower_case_name, sizeof(lower_case_name)); - machine_id_strings.emplace(lower_case_name); + machine_id_strings.emplace(id); } void -Machine::insert_id(IpAddr *ipaddr) +Machine::insert_id(IpAddr const &ipaddr) { - char string_value[INET6_ADDRSTRLEN + 1] = {0}; + ip_text_buffer buff; - if (ipaddr != nullptr) { - ipaddr->toString(string_value, sizeof(string_value)); - machine_id_strings.emplace(string_value); - machine_id_ipaddrs.emplace(string_value, ipaddr); - } + ipaddr.toString(buff, sizeof(buff)); + machine_id_strings.emplace(buff); + machine_id_ipaddrs.emplace(ipaddr); } diff --git a/proxy/ParentSelection.cc b/proxy/ParentSelection.cc index f39686aa815..8746a8c8c86 100644 --- a/proxy/ParentSelection.cc +++ b/proxy/ParentSelection.cc @@ -394,7 +394,7 @@ ParentRecord::PreProcessParents(const char *val, const int line_num, char *buf, ink_assert(length < sizeof(fqdn)); memset(fqdn, 0, sizeof(fqdn)); strncpy(fqdn, token, length); - if (self_detect && machine->is_self(fqdn)) { + if (self_detect && machine->is_self(std::string_view(fqdn))) { if (self_detect == 1) { Debug("parent_select", "token: %s, matches this machine. Removing self from parent list at line %d", fqdn, line_num); token = strtok_r(nullptr, PARENT_DELIMITERS, &savePtr); @@ -405,7 +405,7 @@ ParentRecord::PreProcessParents(const char *val, const int line_num, char *buf, } } } else { - if (self_detect && machine->is_self(token)) { + if (self_detect && machine->is_self(std::string_view(token))) { if (self_detect == 1) { Debug("parent_select", "token: %s, matches this machine. Removing self from parent list at line %d", token, line_num); token = strtok_r(nullptr, PARENT_DELIMITERS, &savePtr); diff --git a/proxy/http/HttpTransactHeaders.cc b/proxy/http/HttpTransactHeaders.cc index f649aab78f7..6f59c19f70c 100644 --- a/proxy/http/HttpTransactHeaders.cc +++ b/proxy/http/HttpTransactHeaders.cc @@ -1008,7 +1008,7 @@ HttpTransactHeaders::add_forwarded_field_to_request(HttpTransact::State *s, HTTP hdr << "by=_" << m.uuid.getString(); } - if (optSet[HttpForwarded::BY_IP] and (m.ip_string_len > 0)) { + if (optSet[HttpForwarded::BY_IP] and m.ip.isValid()) { if (hdr.size()) { hdr << ';'; } diff --git a/proxy/http/remap/Makefile.am b/proxy/http/remap/Makefile.am index dc51f477479..e08ad0faeae 100644 --- a/proxy/http/remap/Makefile.am +++ b/proxy/http/remap/Makefile.am @@ -133,7 +133,6 @@ test_NextHopStrategyFactory_CPPFLAGS = \ @YAMLCPP_INCLUDES@ test_NextHopStrategyFactory_LDADD = \ - $(top_builddir)/src/tscpp/util/libtscpputil.la \ $(top_builddir)/src/tscore/libtscore.la \ $(top_builddir)/proxy/hdrs/libhdrs.a \ $(top_builddir)/iocore/eventsystem/libinkevent.a \ @@ -141,6 +140,7 @@ test_NextHopStrategyFactory_LDADD = \ $(top_builddir)/proxy/logging/liblogging.a \ $(top_builddir)/mgmt/libmgmt_p.la \ $(top_builddir)/iocore/utils/libinkutils.a \ + $(top_builddir)/src/tscpp/util/libtscpputil.la \ @YAMLCPP_LIBS@ \ @HWLOC_LIBS@ @@ -164,7 +164,6 @@ test_NextHopRoundRobin_CPPFLAGS = \ @YAMLCPP_INCLUDES@ test_NextHopRoundRobin_LDADD = \ - $(top_builddir)/src/tscpp/util/libtscpputil.la \ $(top_builddir)/src/tscore/libtscore.la \ $(top_builddir)/proxy/hdrs/libhdrs.a \ $(top_builddir)/iocore/eventsystem/libinkevent.a \ @@ -172,19 +171,20 @@ test_NextHopRoundRobin_LDADD = \ $(top_builddir)/proxy/logging/liblogging.a \ $(top_builddir)/mgmt/libmgmt_p.la \ $(top_builddir)/iocore/utils/libinkutils.a \ + $(top_builddir)/src/tscpp/util/libtscpputil.la \ @YAMLCPP_LIBS@ \ @HWLOC_LIBS@ -test_NextHopRoundRobin_LDFLAGS = $(AM_LDFLAGS) -L$(top_builddir)/src/tscore/.libs -ltscore +test_NextHopRoundRobin_LDFLAGS = $(AM_LDFLAGS) -L$(top_builddir)/src/tscore/.libs -ltscore -L$(top_builddir)/src/tscpp/util/.libs -ltscpputil test_NextHopRoundRobin_SOURCES = \ + unit-tests/test_NextHopRoundRobin.cc \ + unit-tests/nexthop_test_stubs.cc \ NextHopSelectionStrategy.cc \ NextHopStrategyFactory.cc \ NextHopRoundRobin.cc \ NextHopConsistentHash.cc \ - NextHopHealthStatus.cc \ - unit-tests/test_NextHopRoundRobin.cc \ - unit-tests/nexthop_test_stubs.cc + NextHopHealthStatus.cc test_NextHopConsistentHash_CPPFLAGS = \ $(AM_CPPFLAGS) \ @@ -195,7 +195,6 @@ test_NextHopConsistentHash_CPPFLAGS = \ @YAMLCPP_INCLUDES@ test_NextHopConsistentHash_LDADD = \ - $(top_builddir)/src/tscpp/util/libtscpputil.la \ $(top_builddir)/src/tscore/libtscore.la \ $(top_builddir)/proxy/hdrs/libhdrs.a \ $(top_builddir)/iocore/eventsystem/libinkevent.a \ @@ -203,6 +202,7 @@ test_NextHopConsistentHash_LDADD = \ $(top_builddir)/proxy/logging/liblogging.a \ $(top_builddir)/mgmt/libmgmt_p.la \ $(top_builddir)/iocore/utils/libinkutils.a \ + $(top_builddir)/src/tscpp/util/libtscpputil.la \ @YAMLCPP_LIBS@ \ @HWLOC_LIBS@ diff --git a/proxy/http/remap/NextHopConsistentHash.cc b/proxy/http/remap/NextHopConsistentHash.cc index a8943620c84..55026e638e0 100644 --- a/proxy/http/remap/NextHopConsistentHash.cc +++ b/proxy/http/remap/NextHopConsistentHash.cc @@ -292,7 +292,7 @@ NextHopConsistentHash::findNextHop(TSHttpTxn txnp, void *ih, time_t now) result->first_choice_status = (hst) ? hst->status : TSHostStatus::TS_HOST_STATUS_UP; // if peering and the selected host is myself, change rings and search for an upstream // parent. - if (ring_mode == NH_PEERING_RING && machine->is_self(pRec->hostname.c_str())) { + if (ring_mode == NH_PEERING_RING && machine->is_self(pRec->hostname)) { // switch to the upstream ring. cur_ring = 1; continue; diff --git a/proxy/http/remap/NextHopSelectionStrategy.cc b/proxy/http/remap/NextHopSelectionStrategy.cc index c57555ee1cc..e6c758976c2 100644 --- a/proxy/http/remap/NextHopSelectionStrategy.cc +++ b/proxy/http/remap/NextHopSelectionStrategy.cc @@ -214,7 +214,7 @@ NextHopSelectionStrategy::Init(ts::Yaml::Map &n) std::shared_ptr host_rec = std::make_shared(hosts_list[hst].as()); host_rec->group_index = grp; host_rec->host_index = hst; - if (mach->is_self(host_rec->hostname.c_str())) { + if (mach->is_self(host_rec->hostname)) { h_stat.setHostStatus(host_rec->hostname.c_str(), TSHostStatus::TS_HOST_STATUS_DOWN, 0, Reason::SELF_DETECT); } hosts_inner.push_back(std::move(host_rec)); diff --git a/proxy/http/remap/unit-tests/nexthop_test_stubs.cc b/proxy/http/remap/unit-tests/nexthop_test_stubs.cc index 3c8a2e2eee2..5213eb9367f 100644 --- a/proxy/http/remap/unit-tests/nexthop_test_stubs.cc +++ b/proxy/http/remap/unit-tests/nexthop_test_stubs.cc @@ -27,6 +27,8 @@ */ +#include + #include "HttpSM.h" #include "nexthop_test_stubs.h" @@ -176,26 +178,10 @@ ConfigUpdateCbTable::invoke(char const *p) #include "I_Machine.h" -static Machine *my_machine = nullptr; - -Machine::Machine(char const *hostname, sockaddr const *addr) {} -Machine::~Machine() -{ - delete my_machine; -} -Machine * -Machine::instance() -{ - if (my_machine == nullptr) { - my_machine = new Machine(nullptr, nullptr); - } - return my_machine; -} -bool -Machine::is_self(const char *name) -{ - return false; -} +static bool StubMachineInit = []() -> bool { + Machine::init("localhost", nullptr); + return true; +}(); #include "HostStatus.h" diff --git a/proxy/logging/LogAccess.cc b/proxy/logging/LogAccess.cc index 32489d02a12..da82c04ced3 100644 --- a/proxy/logging/LogAccess.cc +++ b/proxy/logging/LogAccess.cc @@ -110,16 +110,15 @@ LogAccess::init() int LogAccess::marshal_proxy_host_name(char *buf) { - char *str = nullptr; - int len = 0; - Machine *machine = Machine::instance(); + int len = 0; + char const *str = nullptr; - if (machine) { - str = machine->hostname; + if (Machine *machine = Machine::instance(); machine) { + str = machine->host_name.c_str(); + len = machine->host_name.length(); } - len = LogAccess::strlen(str); - + len = INK_ALIGN_DEFAULT(len + 1); if (buf) { marshal_str(buf, str, len); } diff --git a/proxy/logging/LogConfig.cc b/proxy/logging/LogConfig.cc index 54359a4a117..7b8a171cc01 100644 --- a/proxy/logging/LogConfig.cc +++ b/proxy/logging/LogConfig.cc @@ -69,7 +69,7 @@ void LogConfig::setup_default_values() { - hostname = ats_strdup(Machine::instance()->hostname); + hostname = ats_strdup(Machine::instance()->host_name.c_str()); log_buffer_size = static_cast(10 * LOG_KILOBYTE); max_secs_per_buffer = 5; max_space_mb_for_logs = 100; diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index 427c6f11305..e9079e171dc 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -10090,7 +10090,7 @@ TSRemapToUrlGet(TSHttpTxn txnp, TSMLoc *urlLocp) TSReturnCode TSHostnameIsSelf(const char *hostname, size_t hostname_len) { - return Machine::instance()->is_self(hostname, hostname_len) ? TS_SUCCESS : TS_ERROR; + return Machine::instance()->is_self(std::string_view{hostname, hostname_len}) ? TS_SUCCESS : TS_ERROR; } TSReturnCode