From a36961659dae280af4514a9bcb6dcb99c6ff7de6 Mon Sep 17 00:00:00 2001 From: Henna Huang Date: Mon, 15 May 2017 15:54:07 -0400 Subject: [PATCH 1/4] Throw EnvoyException on address parse error --- source/common/network/address_impl.cc | 60 ------------- source/common/network/address_impl.h | 17 ---- source/common/network/cidr_range.cc | 4 +- source/common/network/utility.cc | 64 +++++++++++++- source/common/network/utility.h | 18 ++++ test/common/network/BUILD | 1 + test/common/network/address_impl_test.cc | 73 +++------------- test/common/network/cidr_range_test.cc | 86 +++++++++---------- test/common/network/utility_test.cc | 86 +++++++++++++++++-- test/common/tracing/zipkin/BUILD | 1 + test/common/tracing/zipkin/tracer_test.cc | 5 +- .../tracing/zipkin/zipkin_core_types_test.cc | 29 ++++--- test/integration/fake_upstream.cc | 2 +- test/test_common/network_utility.cc | 2 +- 14 files changed, 236 insertions(+), 212 deletions(-) diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index f8067d857dd9e..46938136419f5 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -220,65 +220,5 @@ int PipeInstance::socket(SocketType type) const { return ::socket(AF_UNIX, flagsFromSocketType(type), 0); } -InstanceConstSharedPtr parseInternetAddress(const std::string& ip_addr) { - sockaddr_in sa4; - if (inet_pton(AF_INET, ip_addr.c_str(), &sa4.sin_addr) == 1) { - sa4.sin_family = AF_INET; - sa4.sin_port = 0; - return std::make_shared(&sa4); - } - sockaddr_in6 sa6; - if (inet_pton(AF_INET6, ip_addr.c_str(), &sa6.sin6_addr) == 1) { - sa6.sin6_family = AF_INET6; - sa6.sin6_port = 0; - return std::make_shared(sa6); - } - return nullptr; -} - -InstanceConstSharedPtr parseInternetAddressAndPort(const std::string& addr) { - if (addr.empty()) { - return nullptr; - } - if (addr[0] == '[') { - // Appears to be an IPv6 address. Find the "]:" that separates the address from the port. - auto pos = addr.rfind("]:"); - if (pos == std::string::npos) { - return nullptr; - } - const auto ip_str = addr.substr(1, pos - 1); - const auto port_str = addr.substr(pos + 2); - uint64_t port64; - if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { - return nullptr; - } - sockaddr_in6 sa6; - if (ip_str.empty() || inet_pton(AF_INET6, ip_str.c_str(), &sa6.sin6_addr) != 1) { - return nullptr; - } - sa6.sin6_family = AF_INET6; - sa6.sin6_port = htons(port64); - return std::make_shared(sa6); - } - // Treat it as an IPv4 address followed by a port. - auto pos = addr.rfind(":"); - if (pos == std::string::npos) { - return nullptr; - } - const auto ip_str = addr.substr(0, pos); - const auto port_str = addr.substr(pos + 1); - uint64_t port64; - if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { - return nullptr; - } - sockaddr_in sa4; - if (ip_str.empty() || inet_pton(AF_INET, ip_str.c_str(), &sa4.sin_addr) != 1) { - return nullptr; - } - sa4.sin_family = AF_INET; - sa4.sin_port = htons(port64); - return std::make_shared(&sa4); -} - } // Address } // Network diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h index 2a91176738408..ee58ac5748616 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -172,23 +172,6 @@ class Ipv6Instance : public InstanceBase { IpHelper ip_; }; -/* - * Parse an internet host address (IPv4 or IPv6) and create an Instance from it. - * The address must not include a port number. - * @param ip_addr string to be parsed as an internet address. - * @return pointer to the Instance, or nullptr if unable to parse the address. - */ -InstanceConstSharedPtr parseInternetAddress(const std::string& ip_addr); - -/* - * Parse an internet host address (IPv4 or IPv6) AND port, and create an Instance from it. - * @param ip_addr string to be parsed as an internet address and port. Examples: - * - "1.2.3.4:80" - * - "[1234:5678::9]:443" - * @return pointer to the Instance, or nullptr if unable to parse the address. - */ -InstanceConstSharedPtr parseInternetAddressAndPort(const std::string& ip_addr); - /** * Implementation of a pipe address (unix domain socket on unix). */ diff --git a/source/common/network/cidr_range.cc b/source/common/network/cidr_range.cc index fbe295be31e8b..dd307c87fd047 100644 --- a/source/common/network/cidr_range.cc +++ b/source/common/network/cidr_range.cc @@ -106,14 +106,14 @@ CidrRange CidrRange::create(InstanceConstSharedPtr address, int length) { // static CidrRange CidrRange::create(const std::string& address, int length) { - return create(parseInternetAddress(address), length); + return create(Utility::parseInternetAddress(address), length); } // static CidrRange CidrRange::create(const std::string& range) { std::vector parts = StringUtil::split(range, '/'); if (parts.size() == 2) { - InstanceConstSharedPtr ptr = parseInternetAddress(parts[0]); + InstanceConstSharedPtr ptr = Utility::parseInternetAddress(parts[0]); if (ptr != nullptr && ptr->type() == Type::Ip) { uint64_t length64; if (StringUtil::atoul(parts[1].c_str(), length64, 10)) { diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 4aba77f5d04d4..0b7a6c5ac2a4d 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -88,7 +88,7 @@ const std::string Utility::UNIX_SCHEME = "unix://"; Address::InstanceConstSharedPtr Utility::resolveUrl(const std::string& url) { if (url.find(TCP_SCHEME) == 0) { - return Address::parseInternetAddressAndPort(url.substr(TCP_SCHEME.size())); + return parseInternetAddressAndPort(url.substr(TCP_SCHEME.size())); } else if (url.find(UNIX_SCHEME) == 0) { return Address::InstanceConstSharedPtr{ new Address::PipeInstance(url.substr(UNIX_SCHEME.size()))}; @@ -129,6 +129,68 @@ uint32_t Utility::portFromTcpUrl(const std::string& url) { } } +Address::InstanceConstSharedPtr Utility::parseInternetAddress(const std::string& ip_address) { + sockaddr_in sa4; + if (inet_pton(AF_INET, ip_address.c_str(), &sa4.sin_addr) == 1) { + sa4.sin_family = AF_INET; + sa4.sin_port = 0; + return std::make_shared(&sa4); + } + sockaddr_in6 sa6; + if (inet_pton(AF_INET6, ip_address.c_str(), &sa6.sin6_addr) == 1) { + sa6.sin6_family = AF_INET6; + sa6.sin6_port = 0; + return std::make_shared(sa6); + } + throw EnvoyException(fmt::format("malformed IP address: {}", ip_address)); +} + +Address::InstanceConstSharedPtr +Utility::parseInternetAddressAndPort(const std::string& ip_address) { + std::string error_msg(fmt::format("malformed IP address: {}", ip_address)); + if (ip_address.empty()) { + throw EnvoyException(error_msg); + } + if (ip_address[0] == '[') { + // Appears to be an IPv6 address. Find the "]:" that separates the address from the port. + auto pos = ip_address.rfind("]:"); + if (pos == std::string::npos) { + throw EnvoyException(error_msg); + } + const auto ip_str = ip_address.substr(1, pos - 1); + const auto port_str = ip_address.substr(pos + 2); + uint64_t port64; + if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { + throw EnvoyException(error_msg); + } + sockaddr_in6 sa6; + if (ip_str.empty() || inet_pton(AF_INET6, ip_str.c_str(), &sa6.sin6_addr) != 1) { + throw EnvoyException(error_msg); + } + sa6.sin6_family = AF_INET6; + sa6.sin6_port = htons(port64); + return std::make_shared(sa6); + } + // Treat it as an IPv4 address followed by a port. + auto pos = ip_address.rfind(":"); + if (pos == std::string::npos) { + throw EnvoyException(error_msg); + } + const auto ip_str = ip_address.substr(0, pos); + const auto port_str = ip_address.substr(pos + 1); + uint64_t port64; + if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { + throw EnvoyException(error_msg); + } + sockaddr_in sa4; + if (ip_str.empty() || inet_pton(AF_INET, ip_str.c_str(), &sa4.sin_addr) != 1) { + throw EnvoyException(error_msg); + } + sa4.sin_family = AF_INET; + sa4.sin_port = htons(port64); + return std::make_shared(&sa4); +} + // TODO(hennna): Currently getLocalAddress does not support choosing between // multiple interfaces and addresses not returned by getifaddrs. In additon, // the default is to return a loopback address of type version. This function may diff --git a/source/common/network/utility.h b/source/common/network/utility.h index 6a5dbb04ac83a..fd4e445515680 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -78,6 +78,24 @@ class Utility { */ static uint32_t portFromTcpUrl(const std::string& url); + /* + * Parse an internet host address (IPv4 or IPv6) and create an Instance from it. Throw + * an exception if unable to parse the address. The address must not include a port number. + * @param ip_address string to be parsed as an internet address. + * @return pointer to the Instance, or nullptr if unable to parse the address. + */ + static Address::InstanceConstSharedPtr parseInternetAddress(const std::string& ip_address); + + /* + * Parse an internet host address (IPv4 or IPv6) AND port, and create an Instance from it. + * Throw an exception if unable to parse the address. + * @param ip_addr string to be parsed as an internet address and port. Examples: + * - "1.2.3.4:80" + * - "[1234:5678::9]:443" + * @return pointer to the Instance. + */ + static Address::InstanceConstSharedPtr parseInternetAddressAndPort(const std::string& ip_address); + /** * Get the local address of the first interface address that is of type * version and is not a loopback address. If no matches are found, return the diff --git a/test/common/network/BUILD b/test/common/network/BUILD index 612f6f040d0c9..96a7fdc6895da 100644 --- a/test/common/network/BUILD +++ b/test/common/network/BUILD @@ -13,6 +13,7 @@ envoy_cc_test( srcs = ["address_impl_test.cc"], deps = [ "//source/common/network:address_lib", + "//source/common/network:utility_lib", "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", "//test/test_common:utility_lib", diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index 1636c6ae140d8..c5d5763cb7722 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -11,6 +11,7 @@ #include "common/common/utility.h" #include "common/network/address_impl.h" +#include "common/network/utility.h" #include "test/test_common/environment.h" #include "test/test_common/network_utility.h" @@ -36,7 +37,7 @@ void makeFdBlocking(int fd) { } void testSocketBindAndConnect(const std::string& addr_port_str) { - auto addr_port = parseInternetAddressAndPort(addr_port_str); + auto addr_port = Network::Utility::parseInternetAddressAndPort(addr_port_str); ASSERT_NE(addr_port, nullptr); if (addr_port->ip()->port() == 0) { addr_port = Network::Test::findOrCheckFreePort(addr_port, SocketType::Stream); @@ -106,7 +107,7 @@ TEST(Ipv4InstanceTest, SocketAddress) { EXPECT_EQ("1.2.3.4", address.ip()->addressAsString()); EXPECT_EQ(6502U, address.ip()->port()); EXPECT_EQ(IpVersion::v4, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("1.2.3.4"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("1.2.3.4"), address)); } TEST(Ipv4InstanceTest, AddressOnly) { @@ -116,7 +117,7 @@ TEST(Ipv4InstanceTest, AddressOnly) { EXPECT_EQ("3.4.5.6", address.ip()->addressAsString()); EXPECT_EQ(0U, address.ip()->port()); EXPECT_EQ(IpVersion::v4, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("3.4.5.6"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("3.4.5.6"), address)); } TEST(Ipv4InstanceTest, AddressAndPort) { @@ -127,7 +128,7 @@ TEST(Ipv4InstanceTest, AddressAndPort) { EXPECT_FALSE(address.ip()->isAnyAddress()); EXPECT_EQ(80U, address.ip()->port()); EXPECT_EQ(IpVersion::v4, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("127.0.0.1"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("127.0.0.1"), address)); } TEST(Ipv4InstanceTest, PortOnly) { @@ -138,37 +139,12 @@ TEST(Ipv4InstanceTest, PortOnly) { EXPECT_TRUE(address.ip()->isAnyAddress()); EXPECT_EQ(443U, address.ip()->port()); EXPECT_EQ(IpVersion::v4, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("0.0.0.0"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("0.0.0.0"), address)); } TEST(Ipv4InstanceTest, BadAddress) { EXPECT_THROW(Ipv4Instance("foo"), EnvoyException); EXPECT_THROW(Ipv4Instance("bar", 1), EnvoyException); - EXPECT_EQ(parseInternetAddress(""), nullptr); - EXPECT_EQ(parseInternetAddress("1.2.3"), nullptr); - EXPECT_EQ(parseInternetAddress("1.2.3.4.5"), nullptr); - EXPECT_EQ(parseInternetAddress("1.2.3.256"), nullptr); - EXPECT_EQ(parseInternetAddress("foo"), nullptr); -} - -TEST(Ipv4InstanceTest, ParseInternetAddressAndPort) { - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3.4")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3.4:")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3.4::1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3.4:-1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort(":1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort(" :1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3:1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3.4]:2")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("1.2.3.4:65536")); - - auto ptr = parseInternetAddressAndPort("0.0.0.0:0"); - ASSERT_NE(ptr, nullptr); - EXPECT_EQ(ptr->asString(), "0.0.0.0:0"); - - ptr = parseInternetAddressAndPort("255.255.255.255:65535"); - ASSERT_NE(ptr, nullptr); - EXPECT_EQ(ptr->asString(), "255.255.255.255:65535"); } TEST(Ipv6InstanceTest, SocketAddress) { @@ -184,7 +160,7 @@ TEST(Ipv6InstanceTest, SocketAddress) { EXPECT_FALSE(address.ip()->isAnyAddress()); EXPECT_EQ(32000U, address.ip()->port()); EXPECT_EQ(IpVersion::v6, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("1:0023::0Ef"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("1:0023::0Ef"), address)); } TEST(Ipv6InstanceTest, AddressOnly) { @@ -194,7 +170,8 @@ TEST(Ipv6InstanceTest, AddressOnly) { EXPECT_EQ("2001:db8:85a3::8a2e:370:7334", address.ip()->addressAsString()); EXPECT_EQ(0U, address.ip()->port()); EXPECT_EQ(IpVersion::v6, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("2001:db8:85a3::8a2e:0370:7334"), address)); + EXPECT_TRUE(addressesEqual( + Network::Utility::parseInternetAddress("2001:db8:85a3::8a2e:0370:7334"), address)); } TEST(Ipv6InstanceTest, AddressAndPort) { @@ -204,7 +181,7 @@ TEST(Ipv6InstanceTest, AddressAndPort) { EXPECT_EQ("::1", address.ip()->addressAsString()); EXPECT_EQ(80U, address.ip()->port()); EXPECT_EQ(IpVersion::v6, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("0:0:0:0:0:0:0:1"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("0:0:0:0:0:0:0:1"), address)); } TEST(Ipv6InstanceTest, PortOnly) { @@ -215,40 +192,12 @@ TEST(Ipv6InstanceTest, PortOnly) { EXPECT_TRUE(address.ip()->isAnyAddress()); EXPECT_EQ(443U, address.ip()->port()); EXPECT_EQ(IpVersion::v6, address.ip()->version()); - EXPECT_TRUE(addressesEqual(parseInternetAddress("::0000"), address)); + EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("::0000"), address)); } TEST(Ipv6InstanceTest, BadAddress) { EXPECT_THROW(Ipv6Instance("foo"), EnvoyException); EXPECT_THROW(Ipv6Instance("bar", 1), EnvoyException); - EXPECT_EQ(parseInternetAddress("0:0:0:0"), nullptr); - EXPECT_EQ(parseInternetAddress("fffff::"), nullptr); - EXPECT_EQ(parseInternetAddress("/foo"), nullptr); -} - -TEST(Ipv6InstanceTest, ParseInternetAddressAndPort) { - EXPECT_EQ(nullptr, parseInternetAddressAndPort("")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("::1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("::")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[[::]]:1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[::]:1]:2")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("]:[::1]:2")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[1.2.3.4:0")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[1.2.3.4]:0")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[::]:")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[::]:-1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[::]:bogus")); - - auto ptr = parseInternetAddressAndPort("[::]:0"); - ASSERT_NE(ptr, nullptr); - EXPECT_EQ(ptr->asString(), "[::]:0"); - - ptr = parseInternetAddressAndPort("[1::1]:65535"); - ASSERT_NE(ptr, nullptr); - EXPECT_EQ(ptr->asString(), "[1::1]:65535"); - - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[::]:-1")); - EXPECT_EQ(nullptr, parseInternetAddressAndPort("[1::1]:65536")); } TEST(PipeInstanceTest, Basic) { diff --git a/test/common/network/cidr_range_test.cc b/test/common/network/cidr_range_test.cc index b0efc7edf2c11..d060cf9a57382 100644 --- a/test/common/network/cidr_range_test.cc +++ b/test/common/network/cidr_range_test.cc @@ -7,6 +7,7 @@ #include "common/network/address_impl.h" #include "common/network/cidr_range.h" +#include "common/network/utility.h" #include "gtest/gtest.h" #include "spdlog/spdlog.h" @@ -66,7 +67,7 @@ TEST(TruncateIpAddressAndLength, Various) { }; test_cases.size(); for (const auto& kv : test_cases) { - InstanceConstSharedPtr inPtr = parseInternetAddress(kv.first.first); + InstanceConstSharedPtr inPtr = Utility::parseInternetAddress(kv.first.first); EXPECT_NE(inPtr, nullptr) << kv.first.first; int length_io = kv.first.second; InstanceConstSharedPtr outPtr = CidrRange::truncateIpAddressAndLength(inPtr, &length_io); @@ -82,16 +83,16 @@ TEST(TruncateIpAddressAndLength, Various) { } TEST(Ipv4CidrRangeTest, InstanceConstSharedPtrAndLengthCtor) { - InstanceConstSharedPtr ptr = parseInternetAddress("1.2.3.5"); + InstanceConstSharedPtr ptr = Utility::parseInternetAddress("1.2.3.5"); CidrRange rng(CidrRange::create(ptr, 31)); // Copy ctor. EXPECT_TRUE(rng.isValid()); EXPECT_EQ(rng.length(), 31); EXPECT_EQ(rng.version(), IpVersion::v4); EXPECT_EQ(rng.asString(), "1.2.3.4/31"); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("1.2.3.3"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("1.2.3.4"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("1.2.3.5"))); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("1.2.3.6"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("1.2.3.3"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("1.2.3.4"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("1.2.3.5"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("1.2.3.6"))); CidrRange rng2(CidrRange::create(ptr, -1)); // Invalid length. EXPECT_FALSE(rng2.isValid()); @@ -108,16 +109,15 @@ TEST(Ipv4CidrRangeTest, StringAndLengthCtor) { EXPECT_EQ(rng.asString(), "1.2.3.4/31"); EXPECT_EQ(rng.length(), 31); EXPECT_EQ(rng.version(), IpVersion::v4); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("1.2.3.3"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("1.2.3.4"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("1.2.3.5"))); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("1.2.3.6"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("1.2.3.3"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("1.2.3.4"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("1.2.3.5"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("1.2.3.6"))); rng = CidrRange::create("1.2.3.4", -10); // Invalid length. EXPECT_FALSE(rng.isValid()); - rng = CidrRange::create("bogus", 31); // Invalid address. - EXPECT_FALSE(rng.isValid()); + EXPECT_THROW(CidrRange::create("bogus", 31), EnvoyException); // Invalid address. } TEST(Ipv4CidrRangeTest, StringCtor) { @@ -126,16 +126,15 @@ TEST(Ipv4CidrRangeTest, StringCtor) { EXPECT_EQ(rng.asString(), "1.2.3.4/31"); EXPECT_EQ(rng.length(), 31); EXPECT_EQ(rng.version(), IpVersion::v4); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("1.2.3.3"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("1.2.3.4"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("1.2.3.5"))); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("1.2.3.6"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("1.2.3.3"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("1.2.3.4"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("1.2.3.5"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("1.2.3.6"))); CidrRange rng2 = CidrRange::create("1.2.3.4/-10"); // Invalid length. EXPECT_FALSE(rng2.isValid()); - CidrRange rng3 = CidrRange::create("bogus/31"); // Invalid address. - EXPECT_FALSE(rng3.isValid()); + EXPECT_THROW(CidrRange::create("bogus/31"), EnvoyException); // Invalid address. CidrRange rng4 = CidrRange::create("/31"); // Missing address. EXPECT_FALSE(rng4.isValid()); @@ -150,28 +149,28 @@ TEST(Ipv4CidrRangeTest, BigRange) { EXPECT_EQ(rng.asString(), "10.0.0.0/8"); EXPECT_EQ(rng.length(), 8); EXPECT_EQ(rng.version(), IpVersion::v4); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("9.255.255.255"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("9.255.255.255"))); std::string addr; for (int i = 0; i < 256; ++i) { addr = fmt::format("10.{}.0.1", i); - EXPECT_TRUE(rng.isInRange(parseInternetAddress(addr))) << addr; + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress(addr))) << addr; addr = fmt::format("10.{}.255.255", i); - EXPECT_TRUE(rng.isInRange(parseInternetAddress(addr))) << addr; + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress(addr))) << addr; } - EXPECT_FALSE(rng.isInRange(parseInternetAddress("11.0.0.0"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("11.0.0.0"))); } TEST(Ipv6CidrRange, InstanceConstSharedPtrAndLengthCtor) { - InstanceConstSharedPtr ptr = parseInternetAddress("abcd::0345"); + InstanceConstSharedPtr ptr = Utility::parseInternetAddress("abcd::0345"); CidrRange rng(CidrRange::create(ptr, 127)); // Copy ctor. EXPECT_TRUE(rng.isValid()); EXPECT_EQ(rng.length(), 127); EXPECT_EQ(rng.version(), IpVersion::v6); EXPECT_EQ(rng.asString(), "abcd::344/127"); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("abcd::343"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("abcd::344"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("abcd::345"))); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("abcd::346"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("abcd::343"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("abcd::344"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("abcd::345"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("abcd::346"))); CidrRange rng2(CidrRange::create(ptr, -1)); // Invalid length. EXPECT_FALSE(rng2.isValid()); @@ -188,16 +187,15 @@ TEST(Ipv6CidrRange, StringAndLengthCtor) { EXPECT_EQ(rng.asString(), "::ffc0/122"); EXPECT_EQ(rng.length(), 122); EXPECT_EQ(rng.version(), IpVersion::v6); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("::ffbf"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("::ffc0"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("::ffff"))); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("::1:0"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("::ffbf"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("::ffc0"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("::ffff"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("::1:0"))); rng = CidrRange::create("::ffff", -2); // Invalid length. EXPECT_FALSE(rng.isValid()); - rng = CidrRange::create("bogus", 122); // Invalid address. - EXPECT_FALSE(rng.isValid()); + EXPECT_THROW(CidrRange::create("bogus", 122), EnvoyException); // Invalid address. } TEST(Ipv6CidrRange, StringCtor) { @@ -206,16 +204,15 @@ TEST(Ipv6CidrRange, StringCtor) { EXPECT_EQ(rng.asString(), "::fc00/118"); EXPECT_EQ(rng.length(), 118); EXPECT_EQ(rng.version(), IpVersion::v6); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("::fbff"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("::fc00"))); - EXPECT_TRUE(rng.isInRange(parseInternetAddress("::ffff"))); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("::1:00"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("::fbff"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("::fc00"))); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress("::ffff"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("::1:00"))); CidrRange rng2 = CidrRange::create("::fc1f/-10"); // Invalid length. EXPECT_FALSE(rng2.isValid()); - CidrRange rng3 = CidrRange::create("::fc1f00/118"); // Invalid address. - EXPECT_FALSE(rng3.isValid()); + EXPECT_THROW(CidrRange::create("::fc1f00/118"), EnvoyException); // Invalid address. CidrRange rng4 = CidrRange::create("/118"); // Missing address. EXPECT_FALSE(rng4.isValid()); @@ -231,15 +228,18 @@ TEST(Ipv6CidrRange, BigRange) { EXPECT_EQ(rng.asString(), "2001:db8:85a3::/64"); EXPECT_EQ(rng.length(), 64); EXPECT_EQ(rng.version(), IpVersion::v6); - EXPECT_FALSE(rng.isInRange(parseInternetAddress("2001:0db8:85a2:ffff:ffff:ffff:ffff:ffff"))); + EXPECT_FALSE( + rng.isInRange(Utility::parseInternetAddress("2001:0db8:85a2:ffff:ffff:ffff:ffff:ffff"))); std::string addr; for (char c : std::string("0123456789abcdef")) { addr = fmt::format("{}:000{}::", prefix, std::string(1, c)); - EXPECT_TRUE(rng.isInRange(parseInternetAddress(addr))) << addr << " not in " << rng.asString(); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress(addr))) << addr << " not in " + << rng.asString(); addr = fmt::format("{}:fff{}:ffff:ffff:ffff", prefix, std::string(1, c)); - EXPECT_TRUE(rng.isInRange(parseInternetAddress(addr))) << addr << " not in " << rng.asString(); + EXPECT_TRUE(rng.isInRange(Utility::parseInternetAddress(addr))) << addr << " not in " + << rng.asString(); } - EXPECT_FALSE(rng.isInRange(parseInternetAddress("2001:0db8:85a4::"))); + EXPECT_FALSE(rng.isInRange(Utility::parseInternetAddress("2001:0db8:85a4::"))); } } // Address diff --git a/test/common/network/utility_test.cc b/test/common/network/utility_test.cc index 91d6694e77e15..b52a489847c3d 100644 --- a/test/common/network/utility_test.cc +++ b/test/common/network/utility_test.cc @@ -131,36 +131,104 @@ TEST(NetworkUtility, Url) { TEST(NetworkUtility, resolveUrl) { EXPECT_THROW(Utility::resolveUrl("foo"), EnvoyException); EXPECT_THROW(Utility::resolveUrl("abc://foo"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://1.2.3.4:1234/"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://127.0.0.1:8001/"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://127.0.0.1:0/foo"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://127.0.0.1:"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://192.168.3.3"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://192.168.3.3.3:0"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://192.168.3:0"), EnvoyException); + + EXPECT_THROW(Utility::resolveUrl("tcp://[::1]"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://[:::1]:1"), EnvoyException); + EXPECT_THROW(Utility::resolveUrl("tcp://foo:0"), EnvoyException); + EXPECT_EQ("", Utility::resolveUrl("unix://")->asString()); EXPECT_EQ("foo", Utility::resolveUrl("unix://foo")->asString()); EXPECT_EQ("tmp", Utility::resolveUrl("unix://tmp")->asString()); EXPECT_EQ("tmp/server", Utility::resolveUrl("unix://tmp/server")->asString()); - EXPECT_EQ(nullptr, Utility::resolveUrl("tcp://192.168.3.3")); - EXPECT_EQ(nullptr, Utility::resolveUrl("tcp://192.168.3.3.3:0")); - EXPECT_EQ(nullptr, Utility::resolveUrl("tcp://192.168.3:0")); - EXPECT_EQ(nullptr, Utility::resolveUrl("tcp://[::1]")); - EXPECT_EQ(nullptr, Utility::resolveUrl("tcp://[:::1]:1")); - EXPECT_EQ(nullptr, Utility::resolveUrl("tcp://foo:0")); + EXPECT_EQ("1.2.3.4:1234", Utility::resolveUrl("tcp://1.2.3.4:1234")->asString()); EXPECT_EQ("0.0.0.0:0", Utility::resolveUrl("tcp://0.0.0.0:0")->asString()); + EXPECT_EQ("127.0.0.1:0", Utility::resolveUrl("tcp://127.0.0.1:0")->asString()); + EXPECT_EQ("[::1]:1", Utility::resolveUrl("tcp://[::1]:1")->asString()); + EXPECT_EQ("[::]:0", Utility::resolveUrl("tcp://[::]:0")->asString()); EXPECT_EQ("[1::2:3]:4", Utility::resolveUrl("tcp://[1::2:3]:4")->asString()); EXPECT_EQ("[a::1]:0", Utility::resolveUrl("tcp://[a::1]:0")->asString()); EXPECT_EQ("[a:b:c:d::]:0", Utility::resolveUrl("tcp://[a:b:c:d::]:0")->asString()); } +TEST(NetworkUtility, ParseInternetAddress) { + EXPECT_THROW(Utility::parseInternetAddress(""), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("1.2.3"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("1.2.3.4.5"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("1.2.3.256"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("foo"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("0:0:0:0"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("fffff::"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("/foo"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("[::]"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddress("[::1]:1"), EnvoyException); + + EXPECT_EQ("1.2.3.4:0", Utility::parseInternetAddress("1.2.3.4")->asString()); + EXPECT_EQ("0.0.0.0:0", Utility::parseInternetAddress("0.0.0.0")->asString()); + EXPECT_EQ("127.0.0.1:0", Utility::parseInternetAddress("127.0.0.1")->asString()); + + EXPECT_EQ("[::1]:0", Utility::parseInternetAddress("::1")->asString()); + EXPECT_EQ("[::]:0", Utility::parseInternetAddress("::")->asString()); + EXPECT_EQ("[1::2:3]:0", Utility::parseInternetAddress("1::2:3")->asString()); + EXPECT_EQ("[a::1]:0", Utility::parseInternetAddress("a::1")->asString()); + EXPECT_EQ("[a:b:c:d::]:0", Utility::parseInternetAddress("a:b:c:d::")->asString()); +} + +TEST(NetworkUtility, ParseInternetAddressAndPort) { + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4:"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4::1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4:-1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort(":1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort(" :1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3:1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4]:2"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4:65536"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("1.2.3.4:8008/"), EnvoyException); + + EXPECT_EQ("0.0.0.0:0", Utility::parseInternetAddressAndPort("0.0.0.0:0")->asString()); + EXPECT_EQ("255.255.255.255:65535", + Utility::parseInternetAddressAndPort("255.255.255.255:65535")->asString()); + EXPECT_EQ("127.0.0.1:0", Utility::parseInternetAddressAndPort("127.0.0.1:0")->asString()); + + EXPECT_THROW(Utility::parseInternetAddressAndPort(""), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("::1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("::"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[[::]]:1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[::]:1]:2"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("]:[::1]:2"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[1.2.3.4:0"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[1.2.3.4]:0"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[::]:"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[::]:-1"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[::]:bogus"), EnvoyException); + EXPECT_THROW(Utility::parseInternetAddressAndPort("[1::1]:65536"), EnvoyException); + + EXPECT_EQ("[::]:0", Utility::parseInternetAddressAndPort("[::]:0")->asString()); + EXPECT_EQ("[1::1]:65535", Utility::parseInternetAddressAndPort("[1::1]:65535")->asString()); + EXPECT_EQ("[::1]:0", Utility::parseInternetAddressAndPort("[::1]:0")->asString()); +} + class NetworkUtilityGetLocalAddress : public testing::TestWithParam {}; INSTANTIATE_TEST_CASE_P(IpVersions, NetworkUtilityGetLocalAddress, testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); -TEST_P(NetworkUtilityGetLocalAddress, getLocalAddress) { +TEST_P(NetworkUtilityGetLocalAddress, GetLocalAddress) { EXPECT_NE(nullptr, Utility::getLocalAddress(GetParam())); } -TEST(NetworkUtility, getOriginalDst) { EXPECT_EQ(nullptr, Utility::getOriginalDst(-1)); } +TEST(NetworkUtility, GetOriginalDst) { EXPECT_EQ(nullptr, Utility::getOriginalDst(-1)); } -TEST(NetworkUtility, loopbackAddress) { +TEST(NetworkUtility, LoopbackAddress) { { Address::Ipv4Instance address("127.0.0.1"); EXPECT_TRUE(Utility::isLoopbackAddress(address)); diff --git a/test/common/tracing/zipkin/BUILD b/test/common/tracing/zipkin/BUILD index 617ab44cdd6e6..0cb331f4b9e7f 100644 --- a/test/common/tracing/zipkin/BUILD +++ b/test/common/tracing/zipkin/BUILD @@ -26,6 +26,7 @@ envoy_cc_test( "//source/common/common:utility_lib", "//source/common/http:conn_manager_lib", "//source/common/network:address_lib", + "//source/common/network:utility_lib", "//source/common/runtime:runtime_lib", "//source/common/tracing/zipkin:zipkin_lib", "//test/mocks:common_lib", diff --git a/test/common/tracing/zipkin/tracer_test.cc b/test/common/tracing/zipkin/tracer_test.cc index 9d919c58c6045..754614dd16142 100644 --- a/test/common/tracing/zipkin/tracer_test.cc +++ b/test/common/tracing/zipkin/tracer_test.cc @@ -1,5 +1,6 @@ #include "common/common/utility.h" #include "common/network/address_impl.h" +#include "common/network/utility.h" #include "common/runtime/runtime_impl.h" #include "common/tracing/zipkin/tracer.h" #include "common/tracing/zipkin/util.h" @@ -30,7 +31,7 @@ class TestReporterImpl : public Reporter { TEST(ZipkinTracerTest, spanCreation) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:9000"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:9000"); NiceMock random_generator; Tracer tracer("my_service_name", addr, random_generator); NiceMock mock_start_time; @@ -208,7 +209,7 @@ TEST(ZipkinTracerTest, spanCreation) { TEST(ZipkinTracerTest, finishSpan) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:9000"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:9000"); NiceMock random_generator; Tracer tracer("my_service_name", addr, random_generator); NiceMock mock_start_time; diff --git a/test/common/tracing/zipkin/zipkin_core_types_test.cc b/test/common/tracing/zipkin/zipkin_core_types_test.cc index 239bdb1c9a170..dbe25f47c75d8 100644 --- a/test/common/tracing/zipkin/zipkin_core_types_test.cc +++ b/test/common/tracing/zipkin/zipkin_core_types_test.cc @@ -1,5 +1,6 @@ #include "common/common/utility.h" #include "common/network/address_impl.h" +#include "common/network/utility.h" #include "common/tracing/zipkin/zipkin_core_constants.h" #include "common/tracing/zipkin/zipkin_core_types.h" @@ -14,11 +15,11 @@ TEST(ZipkinCoreTypesEndpointTest, defaultConstructor) { EXPECT_EQ(R"({"ipv4":"","port":0,"serviceName":""})", ep.toJson()); Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddress("127.0.0.1"); + Network::Utility::parseInternetAddress("127.0.0.1"); ep.setAddress(addr); EXPECT_EQ(R"({"ipv4":"127.0.0.1","port":0,"serviceName":""})", ep.toJson()); - addr = Network::Address::parseInternetAddressAndPort( + addr = Network::Utility::parseInternetAddressAndPort( "[2001:0db8:85a3:0000:0000:8a2e:0370:4444]:7334"); ep.setAddress(addr); EXPECT_EQ(R"({"ipv6":"2001:db8:85a3::8a2e:370:4444","port":7334,"serviceName":""})", ep.toJson()); @@ -33,13 +34,13 @@ TEST(ZipkinCoreTypesEndpointTest, defaultConstructor) { TEST(ZipkinCoreTypesEndpointTest, customConstructor) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep(std::string("my_service"), addr); EXPECT_EQ("my_service", ep.serviceName()); EXPECT_EQ(R"({"ipv4":"127.0.0.1","port":3306,"serviceName":"my_service"})", ep.toJson()); - addr = Network::Address::parseInternetAddressAndPort( + addr = Network::Utility::parseInternetAddressAndPort( "[2001:0db8:85a3:0000:0000:8a2e:0370:4444]:7334"); ep.setAddress(addr); @@ -50,7 +51,7 @@ TEST(ZipkinCoreTypesEndpointTest, customConstructor) { TEST(ZipkinCoreTypesEndpointTest, copyOperator) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep1(std::string("my_service"), addr); Endpoint ep2(ep1); @@ -63,7 +64,7 @@ TEST(ZipkinCoreTypesEndpointTest, copyOperator) { TEST(ZipkinCoreTypesEndpointTest, assignmentOperator) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep1(std::string("my_service"), addr); Endpoint ep2 = ep1; @@ -96,7 +97,7 @@ TEST(ZipkinCoreTypesAnnotationTest, defaultConstructor) { // Test the copy-semantics flavor of setEndpoint Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep(std::string("my_service"), addr); ann.setEndpoint(ep); EXPECT_TRUE(ann.isSetEndpoint()); @@ -111,7 +112,7 @@ TEST(ZipkinCoreTypesAnnotationTest, defaultConstructor) { EXPECT_EQ(expected_json, ann.toJson()); // Test the move-semantics flavor of setEndpoint - addr = Network::Address::parseInternetAddressAndPort("192.168.1.1:5555"); + addr = Network::Utility::parseInternetAddressAndPort("192.168.1.1:5555"); Endpoint ep2(std::string("my_service_2"), addr); ann.setEndpoint(std::move(ep2)); EXPECT_TRUE(ann.isSetEndpoint()); @@ -137,7 +138,7 @@ TEST(ZipkinCoreTypesAnnotationTest, defaultConstructor) { TEST(ZipkinCoreTypesAnnotationTest, customConstructor) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep(std::string("my_service"), addr); uint64_t timestamp = std::chrono::duration_cast( @@ -161,7 +162,7 @@ TEST(ZipkinCoreTypesAnnotationTest, customConstructor) { TEST(ZipkinCoreTypesAnnotationTest, copyConstructor) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep(std::string("my_service"), addr); uint64_t timestamp = std::chrono::duration_cast( @@ -178,7 +179,7 @@ TEST(ZipkinCoreTypesAnnotationTest, copyConstructor) { TEST(ZipkinCoreTypesAnnotationTest, assignmentOperator) { Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep(std::string("my_service"), addr); uint64_t timestamp = std::chrono::duration_cast( @@ -213,7 +214,7 @@ TEST(ZipkinCoreTypesBinaryAnnotationTest, defaultConstructor) { // Test the copy-semantics flavor of setEndpoint Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("127.0.0.1:3306"); + Network::Utility::parseInternetAddressAndPort("127.0.0.1:3306"); Endpoint ep(std::string("my_service"), addr); ann.setEndpoint(ep); EXPECT_TRUE(ann.isSetEndpoint()); @@ -229,7 +230,7 @@ TEST(ZipkinCoreTypesBinaryAnnotationTest, defaultConstructor) { EXPECT_EQ(expected_json, ann.toJson()); // Test the move-semantics flavor of setEndpoint - addr = Network::Address::parseInternetAddressAndPort("192.168.1.1:5555"); + addr = Network::Utility::parseInternetAddressAndPort("192.168.1.1:5555"); Endpoint ep2(std::string("my_service_2"), addr); ann.setEndpoint(std::move(ep2)); EXPECT_TRUE(ann.isSetEndpoint()); @@ -353,7 +354,7 @@ TEST(ZipkinCoreTypesSpanTest, defaultConstructor) { endpoint.setServiceName("my_service_name"); Network::Address::InstanceConstSharedPtr addr = - Network::Address::parseInternetAddressAndPort("192.168.1.2:3306"); + Network::Utility::parseInternetAddressAndPort("192.168.1.2:3306"); endpoint.setAddress(addr); ann.setValue(Zipkin::ZipkinCoreConstants::get().CLIENT_SEND); diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 2c3ff34fba4e4..3c05dfe5b03b5 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -207,7 +207,7 @@ static Network::ListenSocketPtr makeTcpListenSocket(uint32_t port) { static Network::ListenSocketPtr makeTcpListenSocket(const Network::Address::IpVersion version, uint32_t port) { return Network::ListenSocketPtr{new Network::TcpListenSocket( - Network::Address::parseInternetAddressAndPort( + Network::Utility::parseInternetAddressAndPort( fmt::format("{}:{}", Network::Test::getAnyAddressUrlString(version), port)), true)}; } diff --git a/test/test_common/network_utility.cc b/test/test_common/network_utility.cc index 6219bd484f6f9..da8830dfc9784 100644 --- a/test/test_common/network_utility.cc +++ b/test/test_common/network_utility.cc @@ -71,7 +71,7 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared Address::InstanceConstSharedPtr findOrCheckFreePort(const std::string& addr_port, Address::SocketType type) { - auto instance = Address::parseInternetAddressAndPort(addr_port); + auto instance = Utility::parseInternetAddressAndPort(addr_port); if (instance != nullptr) { instance = findOrCheckFreePort(instance, type); } else { From 6933b7599708da17a677b346e6f84e0cd9c2be6b Mon Sep 17 00:00:00 2001 From: Henna Huang Date: Mon, 15 May 2017 20:30:30 -0400 Subject: [PATCH 2/4] Response to comments --- source/common/network/utility.cc | 23 +++++++++++++---------- source/common/network/utility.h | 3 +++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 0b7a6c5ac2a4d..86d734da6f07f 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -147,25 +147,24 @@ Address::InstanceConstSharedPtr Utility::parseInternetAddress(const std::string& Address::InstanceConstSharedPtr Utility::parseInternetAddressAndPort(const std::string& ip_address) { - std::string error_msg(fmt::format("malformed IP address: {}", ip_address)); if (ip_address.empty()) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } if (ip_address[0] == '[') { // Appears to be an IPv6 address. Find the "]:" that separates the address from the port. auto pos = ip_address.rfind("]:"); if (pos == std::string::npos) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } const auto ip_str = ip_address.substr(1, pos - 1); const auto port_str = ip_address.substr(pos + 2); - uint64_t port64; + uint64_t port64 = 0; if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } sockaddr_in6 sa6; if (ip_str.empty() || inet_pton(AF_INET6, ip_str.c_str(), &sa6.sin6_addr) != 1) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } sa6.sin6_family = AF_INET6; sa6.sin6_port = htons(port64); @@ -174,23 +173,27 @@ Utility::parseInternetAddressAndPort(const std::string& ip_address) { // Treat it as an IPv4 address followed by a port. auto pos = ip_address.rfind(":"); if (pos == std::string::npos) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } const auto ip_str = ip_address.substr(0, pos); const auto port_str = ip_address.substr(pos + 1); - uint64_t port64; + uint64_t port64 = 0; if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } sockaddr_in sa4; if (ip_str.empty() || inet_pton(AF_INET, ip_str.c_str(), &sa4.sin_addr) != 1) { - throw EnvoyException(error_msg); + Utility::throwWithMalformedIp(ip_address); } sa4.sin_family = AF_INET; sa4.sin_port = htons(port64); return std::make_shared(&sa4); } +void Utility::throwWithMalformedIp(const std::string& ip_address) { + throw EnvoyException(fmt::format("malformed IP address: {}", ip_address)); +} + // TODO(hennna): Currently getLocalAddress does not support choosing between // multiple interfaces and addresses not returned by getifaddrs. In additon, // the default is to return a loopback address of type version. This function may diff --git a/source/common/network/utility.h b/source/common/network/utility.h index fd4e445515680..f3936410737e3 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -171,6 +171,9 @@ class Utility { * @return whether the port appears in at least one of the ranges in the list */ static bool portInRangeList(const Address::Instance& address, const std::list& list); + +private: + static void throwWithMalformedIp(const std::string& ip_address); }; } // Network From 92ed644ab76549fc8dcfa238b4ee693f7cd619df Mon Sep 17 00:00:00 2001 From: Henna Huang Date: Tue, 16 May 2017 10:41:18 -0400 Subject: [PATCH 3/4] Response to comments --- source/common/network/cidr_range.cc | 2 +- source/common/network/utility.cc | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/source/common/network/cidr_range.cc b/source/common/network/cidr_range.cc index dd307c87fd047..0e9c629498db3 100644 --- a/source/common/network/cidr_range.cc +++ b/source/common/network/cidr_range.cc @@ -114,7 +114,7 @@ CidrRange CidrRange::create(const std::string& range) { std::vector parts = StringUtil::split(range, '/'); if (parts.size() == 2) { InstanceConstSharedPtr ptr = Utility::parseInternetAddress(parts[0]); - if (ptr != nullptr && ptr->type() == Type::Ip) { + if (ptr->type() == Type::Ip) { uint64_t length64; if (StringUtil::atoul(parts[1].c_str(), length64, 10)) { if ((ptr->ip()->version() == IpVersion::v6 && length64 <= 128) || diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 86d734da6f07f..67183fb24622a 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -148,23 +148,23 @@ Address::InstanceConstSharedPtr Utility::parseInternetAddress(const std::string& Address::InstanceConstSharedPtr Utility::parseInternetAddressAndPort(const std::string& ip_address) { if (ip_address.empty()) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } if (ip_address[0] == '[') { // Appears to be an IPv6 address. Find the "]:" that separates the address from the port. auto pos = ip_address.rfind("]:"); if (pos == std::string::npos) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } const auto ip_str = ip_address.substr(1, pos - 1); const auto port_str = ip_address.substr(pos + 2); uint64_t port64 = 0; if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } sockaddr_in6 sa6; if (ip_str.empty() || inet_pton(AF_INET6, ip_str.c_str(), &sa6.sin6_addr) != 1) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } sa6.sin6_family = AF_INET6; sa6.sin6_port = htons(port64); @@ -173,17 +173,17 @@ Utility::parseInternetAddressAndPort(const std::string& ip_address) { // Treat it as an IPv4 address followed by a port. auto pos = ip_address.rfind(":"); if (pos == std::string::npos) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } const auto ip_str = ip_address.substr(0, pos); const auto port_str = ip_address.substr(pos + 1); uint64_t port64 = 0; if (port_str.empty() || !StringUtil::atoul(port_str.c_str(), port64, 10) || port64 > 65535) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } sockaddr_in sa4; if (ip_str.empty() || inet_pton(AF_INET, ip_str.c_str(), &sa4.sin_addr) != 1) { - Utility::throwWithMalformedIp(ip_address); + throwWithMalformedIp(ip_address); } sa4.sin_family = AF_INET; sa4.sin_port = htons(port64); From b9393a1feb578f736d8b99b2b527f31e72f7a08e Mon Sep 17 00:00:00 2001 From: Henna Huang Date: Tue, 16 May 2017 11:29:23 -0400 Subject: [PATCH 4/4] Response to comments --- source/common/network/utility.cc | 3 ++- source/common/network/utility.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 67183fb24622a..1a41f2aed80f8 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -142,7 +142,8 @@ Address::InstanceConstSharedPtr Utility::parseInternetAddress(const std::string& sa6.sin6_port = 0; return std::make_shared(sa6); } - throw EnvoyException(fmt::format("malformed IP address: {}", ip_address)); + throwWithMalformedIp(ip_address); + NOT_REACHED; } Address::InstanceConstSharedPtr diff --git a/source/common/network/utility.h b/source/common/network/utility.h index f3936410737e3..cbb05df1df076 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -88,7 +88,7 @@ class Utility { /* * Parse an internet host address (IPv4 or IPv6) AND port, and create an Instance from it. - * Throw an exception if unable to parse the address. + * @throws an exception if unable to parse the address. * @param ip_addr string to be parsed as an internet address and port. Examples: * - "1.2.3.4:80" * - "[1234:5678::9]:443"