From: Andrey Ignatov Date: Thu, 2 Nov 2017 17:22:09 +0000 (-0700) Subject: Introduce non-throwing try* methods for IPAddress{,V4,V6}. X-Git-Tag: v2017.11.06.00~12 X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=30c1e1dcee8143c6804231a009ae939e5793dc56 Introduce non-throwing try* methods for IPAddress{,V4,V6}. Summary: Now there is no interface to create `IPAddress{,V4,V6}` from a string or `ByteRange` that doesn't throw. All available static methods throw `IPAddressFormatException`. It has a few disadvantages: == 1. It's unsafe == Caller is not forced to catch exception, it's very easy to write `IPAddress(potentiallyInvalidString)` and discover that it can throw when it's already in prod. == 2. It's inconvenient == if caller is aware about exception, (s)he's forced to write `try {} catch` that is inconvenient and leads to code like this: folly::IPAddress taskIp; try { taskIp = folly::IPAddress(kv.second.taskIp); } catch (const folly::IPAddressFormatException&) { // Error handling .. } // Use IP .. == 3. It's expensive == Amended benchmark shows that `IPAddress` constructor is ~10 times slower when a string with invalid IP is passed to it. --- The diff introduces two non-throwing interfaces for all tree flavors of `IPAddress`: `tryFromString()` tries to create IP address from string and returns either corresponding IP address or `enum class IPAddressFormatError` using `folly::Expected`. `tryFromBinary()` does same thing but for `ByteRange` input. The code can be as short as: if (auto maybeIp = IPAddress::tryFromString(ipStr)) { // Use maybeIp.value() .. } The `try` prefix is chosen to be consistent with other interfaces in folly, e.g. `to` and `tryTo`. Reviewed By: yfeldblum Differential Revision: D6211182 fbshipit-source-id: f27cf90997c100a5fd42138e66ff9bb172204c20 --- diff --git a/folly/IPAddress.cpp b/folly/IPAddress.cpp index bb79617b..2ef69d64 100644 --- a/folly/IPAddress.cpp +++ b/folly/IPAddress.cpp @@ -46,7 +46,7 @@ void toAppend(IPAddress addr, fbstring* result) { result->append(addr.str()); } -bool IPAddress::validate(StringPiece ip) { +bool IPAddress::validate(StringPiece ip) noexcept { return IPAddressV4::validate(ip) || IPAddressV6::validate(ip); } @@ -125,6 +125,18 @@ IPAddress IPAddress::fromBinary(ByteRange bytes) { } } +Expected IPAddress::tryFromBinary( + ByteRange bytes) noexcept { + // Check IPv6 first since it's our main protocol. + if (bytes.size() == 16) { + return IPAddressV6::tryFromBinary(bytes); + } else if (bytes.size() == 4) { + return IPAddressV4::tryFromBinary(bytes); + } else { + return makeUnexpected(IPAddressFormatError::UNSUPPORTED_ADDR_FAMILY); + } +} + // public static IPAddress IPAddress::fromLong(uint32_t src) { return IPAddress(IPAddressV4::fromLong(src)); @@ -137,45 +149,25 @@ IPAddress IPAddress::fromLongHBO(uint32_t src) { IPAddress::IPAddress() : addr_(), family_(AF_UNSPEC) {} // public string constructor -IPAddress::IPAddress(StringPiece addr) : addr_(), family_(AF_UNSPEC) { - string ip = addr.str(); // inet_pton() needs NUL-terminated string - auto throwFormatException = [&](const string& msg) { - throw IPAddressFormatException(sformat("Invalid IP '{}': {}", ip, msg)); - }; - - if (ip.size() < 2) { - throwFormatException("address too short"); - } - if (ip.front() == '[' && ip.back() == ']') { - ip = ip.substr(1, ip.size() - 2); +IPAddress::IPAddress(StringPiece str) : addr_(), family_(AF_UNSPEC) { + auto maybeIp = tryFromString(str); + if (maybeIp.hasError()) { + throw IPAddressFormatException( + to("Invalid IP address '", str, "'")); } + *this = std::move(maybeIp.value()); +} +Expected IPAddress::tryFromString( + StringPiece str) noexcept { // need to check for V4 address second, since IPv4-mapped IPv6 addresses may // contain a period - if (ip.find(':') != string::npos) { - struct addrinfo* result; - struct addrinfo hints; - memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_INET6; - hints.ai_socktype = SOCK_STREAM; - hints.ai_flags = AI_NUMERICHOST; - if (!getaddrinfo(ip.c_str(), nullptr, &hints, &result)) { - struct sockaddr_in6* ipAddr = (struct sockaddr_in6*)result->ai_addr; - addr_ = IPAddressV46(IPAddressV6(*ipAddr)); - family_ = AF_INET6; - freeaddrinfo(result); - } else { - throwFormatException("getsockaddr failed for V6 address"); - } - } else if (ip.find('.') != string::npos) { - in_addr ipAddr; - if (inet_pton(AF_INET, ip.c_str(), &ipAddr) != 1) { - throwFormatException("inet_pton failed for V4 address"); - } - addr_ = IPAddressV46(IPAddressV4(ipAddr)); - family_ = AF_INET; + if (str.find(':') != string::npos) { + return IPAddressV6::tryFromString(str); + } else if (str.find('.') != string::npos) { + return IPAddressV4::tryFromString(str); } else { - throwFormatException("invalid address format"); + return makeUnexpected(IPAddressFormatError::UNSUPPORTED_ADDR_FAMILY); } } @@ -202,30 +194,30 @@ IPAddress::IPAddress(const sockaddr* addr) : addr_(), family_(AF_UNSPEC) { } // public ipv4 constructor -IPAddress::IPAddress(const IPAddressV4 ipV4Addr) +IPAddress::IPAddress(const IPAddressV4 ipV4Addr) noexcept : addr_(ipV4Addr), family_(AF_INET) {} // public ipv4 constructor -IPAddress::IPAddress(const in_addr ipV4Addr) +IPAddress::IPAddress(const in_addr ipV4Addr) noexcept : addr_(IPAddressV4(ipV4Addr)), family_(AF_INET) {} // public ipv6 constructor -IPAddress::IPAddress(const IPAddressV6& ipV6Addr) +IPAddress::IPAddress(const IPAddressV6& ipV6Addr) noexcept : addr_(ipV6Addr), family_(AF_INET6) {} // public ipv6 constructor -IPAddress::IPAddress(const in6_addr& ipV6Addr) +IPAddress::IPAddress(const in6_addr& ipV6Addr) noexcept : addr_(IPAddressV6(ipV6Addr)), family_(AF_INET6) {} // Assign from V4 address -IPAddress& IPAddress::operator=(const IPAddressV4& ipv4_addr) { +IPAddress& IPAddress::operator=(const IPAddressV4& ipv4_addr) noexcept { addr_ = IPAddressV46(ipv4_addr); family_ = AF_INET; return *this; } // Assign from V6 address -IPAddress& IPAddress::operator=(const IPAddressV6& ipv6_addr) { +IPAddress& IPAddress::operator=(const IPAddressV6& ipv6_addr) noexcept { addr_ = IPAddressV46(ipv6_addr); family_ = AF_INET6; return *this; diff --git a/folly/IPAddress.h b/folly/IPAddress.h index 1b9d205e..3f3840bf 100644 --- a/folly/IPAddress.h +++ b/folly/IPAddress.h @@ -73,7 +73,7 @@ class IPAddress { public: // returns true iff the input string can be parsed as an ip-address - static bool validate(StringPiece ip); + static bool validate(StringPiece ip) noexcept; // return the V4 representation of the address, converting it from V6 to V4 if // needed. Note that this will throw an IPAddressFormatException if the V6 @@ -114,6 +114,20 @@ class IPAddress { */ static IPAddress fromBinary(ByteRange bytes); + /** + * Non-throwing version of fromBinary(). + * On failure returns IPAddressFormatError. + */ + static Expected tryFromBinary( + ByteRange bytes) noexcept; + + /** + * Tries to create a new IPAddress instance from provided string and + * returns it on success. Returns IPAddressFormatError on failure. + */ + static Expected tryFromString( + StringPiece str) noexcept; + /** * Create an IPAddress from a 32bit long (network byte order). * @throws IPAddressFormatException @@ -152,25 +166,25 @@ class IPAddress { explicit IPAddress(const sockaddr* addr); // Create an IPAddress from a V4 address - /* implicit */ IPAddress(const IPAddressV4 ipV4Addr); - /* implicit */ IPAddress(const in_addr addr); + /* implicit */ IPAddress(const IPAddressV4 ipV4Addr) noexcept; + /* implicit */ IPAddress(const in_addr addr) noexcept; // Create an IPAddress from a V6 address - /* implicit */ IPAddress(const IPAddressV6& ipV6Addr); - /* implicit */ IPAddress(const in6_addr& addr); + /* implicit */ IPAddress(const IPAddressV6& ipV6Addr) noexcept; + /* implicit */ IPAddress(const in6_addr& addr) noexcept; // Assign from V4 address - IPAddress& operator=(const IPAddressV4& ipV4Addr); + IPAddress& operator=(const IPAddressV4& ipV4Addr) noexcept; // Assign from V6 address - IPAddress& operator=(const IPAddressV6& ipV6Addr); + IPAddress& operator=(const IPAddressV6& ipV6Addr) noexcept; /** * Converts an IPAddress to an IPAddressV4 instance. * @note This is not some handy convenience wrapper to convert an IPv4 address * to a mapped IPv6 address. If you want that use * IPAddress::createIPv6(addr) - * @throws IPAddressFormatException is not a V4 instance + * @throws InvalidAddressFamilyException is not a V4 instance */ const IPAddressV4& asV4() const { if (UNLIKELY(!isV4())) { @@ -418,11 +432,11 @@ class IPAddress { IPAddressV4 ipV4Addr; IPAddressV6 ipV6Addr; // default constructor - IPAddressV46() { + IPAddressV46() noexcept { std::memset(this, 0, sizeof(IPAddressV46)); } - explicit IPAddressV46(const IPAddressV4& addr) : ipV4Addr(addr) {} - explicit IPAddressV46(const IPAddressV6& addr) : ipV6Addr(addr) {} + explicit IPAddressV46(const IPAddressV4& addr) noexcept : ipV4Addr(addr) {} + explicit IPAddressV46(const IPAddressV6& addr) noexcept : ipV6Addr(addr) {} } IPAddressV46; IPAddressV46 addr_; sa_family_t family_; diff --git a/folly/IPAddressException.h b/folly/IPAddressException.h index 37a163ef..d1830ba6 100644 --- a/folly/IPAddressException.h +++ b/folly/IPAddressException.h @@ -24,6 +24,11 @@ namespace folly { +/** + * Error codes for non-throwing interface of IPAddress family of functions. + */ +enum class IPAddressFormatError { INVALID_IP, UNSUPPORTED_ADDR_FAMILY }; + /** * Exception for invalid IP addresses. */ diff --git a/folly/IPAddressV4.cpp b/folly/IPAddressV4.cpp index c373e092..af55def4 100644 --- a/folly/IPAddressV4.cpp +++ b/folly/IPAddressV4.cpp @@ -44,14 +44,8 @@ void toAppend(IPAddressV4 addr, fbstring* result) { result->append(addr.str()); } -bool IPAddressV4::validate(StringPiece ip) { - constexpr size_t kStrMaxLen = INET_ADDRSTRLEN; - std::array ip_cstr; - const size_t len = std::min(ip.size(), kStrMaxLen); - std::memcpy(ip_cstr.data(), ip.data(), len); - ip_cstr[len] = 0; - struct in_addr addr; - return 1 == inet_pton(AF_INET, ip_cstr.data(), &addr); +bool IPAddressV4::validate(StringPiece ip) noexcept { + return tryFromString(ip).hasValue(); } // public static @@ -87,27 +81,57 @@ uint32_t IPAddressV4::toLongHBO(StringPiece ip) { IPAddressV4::IPAddressV4() {} // ByteArray4 constructor -IPAddressV4::IPAddressV4(const ByteArray4& src) : addr_(src) {} +IPAddressV4::IPAddressV4(const ByteArray4& src) noexcept : addr_(src) {} // public string constructor IPAddressV4::IPAddressV4(StringPiece addr) : addr_() { - auto ip = addr.str(); - if (inet_pton(AF_INET, ip.c_str(), &addr_.inAddr_) != 1) { - throw IPAddressFormatException(sformat("Invalid IPv4 address '{}'", addr)); + auto maybeIp = tryFromString(addr); + if (maybeIp.hasError()) { + throw IPAddressFormatException( + to("Invalid IPv4 address '", addr, "'")); + } + *this = std::move(maybeIp.value()); +} + +Expected IPAddressV4::tryFromString( + StringPiece str) noexcept { + struct in_addr inAddr; + if (inet_pton(AF_INET, str.str().c_str(), &inAddr) != 1) { + return makeUnexpected(IPAddressFormatError::INVALID_IP); } + return IPAddressV4(inAddr); } // in_addr constructor -IPAddressV4::IPAddressV4(const in_addr src) : addr_(src) {} +IPAddressV4::IPAddressV4(const in_addr src) noexcept : addr_(src) {} -// public -void IPAddressV4::setFromBinary(ByteRange bytes) { - if (bytes.size() != 4) { - throw IPAddressFormatException(sformat( - "Invalid IPv4 binary data: length must be 4 bytes, got {}", +IPAddressV4 IPAddressV4::fromBinary(ByteRange bytes) { + auto maybeIp = tryFromBinary(bytes); + if (maybeIp.hasError()) { + throw IPAddressFormatException(to( + "Invalid IPv4 binary data: length must be 4 bytes, got ", bytes.size())); } + return maybeIp.value(); +} + +Expected IPAddressV4::tryFromBinary( + ByteRange bytes) noexcept { + IPAddressV4 addr; + auto setResult = addr.trySetFromBinary(bytes); + if (setResult.hasError()) { + return makeUnexpected(std::move(setResult.error())); + } + return addr; +} + +Expected IPAddressV4::trySetFromBinary( + ByteRange bytes) noexcept { + if (bytes.size() != 4) { + return makeUnexpected(IPAddressFormatError::INVALID_IP); + } memcpy(&addr_.inAddr_.s_addr, bytes.data(), sizeof(in_addr)); + return folly::unit; } // static @@ -126,8 +150,6 @@ IPAddressV4 IPAddressV4::fromInverseArpaName(const std::string& arpaname) { // reverse 1.0.168.192 -> 192.168.0.1 return IPAddressV4(join(".", pieces.rbegin(), pieces.rend())); } - -// public IPAddressV6 IPAddressV4::createIPv6() const { ByteArray16 ba{}; ba[10] = 0xff; diff --git a/folly/IPAddressV4.h b/folly/IPAddressV4.h index 76534add..6f6f11ed 100644 --- a/folly/IPAddressV4.h +++ b/folly/IPAddressV4.h @@ -22,7 +22,9 @@ #include #include +#include #include +#include #include #include #include @@ -60,7 +62,7 @@ class IPAddressV4 { 4 /*words*/ * 3 /*max chars per word*/ + 3 /*separators*/; // returns true iff the input string can be parsed as an ipv4-address - static bool validate(StringPiece ip); + static bool validate(StringPiece ip) noexcept; // create an IPAddressV4 instance from a uint32_t (network byte order) static IPAddressV4 fromLong(uint32_t src); @@ -71,11 +73,21 @@ class IPAddressV4 { * Create a new IPAddress instance from the provided binary data. * @throws IPAddressFormatException if the input length is not 4 bytes. */ - static IPAddressV4 fromBinary(ByteRange bytes) { - IPAddressV4 addr; - addr.setFromBinary(bytes); - return addr; - } + static IPAddressV4 fromBinary(ByteRange bytes); + + /** + * Non-throwing version of fromBinary(). + * On failure returns IPAddressFormatError. + */ + static Expected tryFromBinary( + ByteRange bytes) noexcept; + + /** + * Tries to create a new IPAddressV4 instance from provided string and + * returns it on success. Returns IPAddressFormatError on failure. + */ + static Expected tryFromString( + StringPiece str) noexcept; /** * Returns the address as a Range. @@ -113,10 +125,10 @@ class IPAddressV4 { explicit IPAddressV4(StringPiece ip); // ByteArray4 constructor - explicit IPAddressV4(const ByteArray4& src); + explicit IPAddressV4(const ByteArray4& src) noexcept; // in_addr constructor - explicit IPAddressV4(const in_addr src); + explicit IPAddressV4(const in_addr src) noexcept; // Return the V6 mapped representation of the address. IPAddressV6 createIPv6() const; @@ -285,9 +297,10 @@ class IPAddressV4 { /** * Set the current IPAddressV4 object to have the address specified by bytes. - * @throws IPAddressFormatException if bytes.size() is not 4. + * Returns IPAddressFormatError if bytes.size() is not 4. */ - void setFromBinary(ByteRange bytes); + Expected trySetFromBinary( + ByteRange bytes) noexcept; }; // boost::hash uses hash_value() so this allows boost::hash to work diff --git a/folly/IPAddressV6.cpp b/folly/IPAddressV6.cpp index be909315..cdd82acc 100644 --- a/folly/IPAddressV6.cpp +++ b/folly/IPAddressV6.cpp @@ -62,18 +62,8 @@ void toAppend(IPAddressV6 addr, fbstring* result) { result->append(addr.str()); } -bool IPAddressV6::validate(StringPiece ip) { - if (ip.size() > 0 && ip.front() == '[' && ip.back() == ']') { - ip = ip.subpiece(1, ip.size() - 2); - } - - constexpr size_t kStrMaxLen = INET6_ADDRSTRLEN; - std::array ip_cstr; - const size_t len = std::min(ip.size(), kStrMaxLen); - std::memcpy(ip_cstr.data(), ip.data(), len); - ip_cstr[len] = 0; - struct in6_addr addr; - return 1 == inet_pton(AF_INET6, ip_cstr.data(), &addr); +bool IPAddressV6::validate(StringPiece ip) noexcept { + return tryFromString(ip).hasValue(); } // public default constructor @@ -81,12 +71,21 @@ IPAddressV6::IPAddressV6() {} // public string constructor IPAddressV6::IPAddressV6(StringPiece addr) { - auto ip = addr.str(); + auto maybeIp = tryFromString(addr); + if (maybeIp.hasError()) { + throw IPAddressFormatException( + to("Invalid IPv6 address '", addr, "'")); + } + *this = std::move(maybeIp.value()); +} + +Expected IPAddressV6::tryFromString( + StringPiece str) noexcept { + auto ip = str.str(); // Allow addresses surrounded in brackets if (ip.size() < 2) { - throw IPAddressFormatException( - sformat("Invalid IPv6 address '{}': address too short", ip)); + return makeUnexpected(IPAddressFormatError::INVALID_IP); } if (ip.front() == '[' && ip.back() == ']') { ip = ip.substr(1, ip.size() - 2); @@ -98,25 +97,26 @@ IPAddressV6::IPAddressV6(StringPiece addr) { hints.ai_family = AF_INET6; hints.ai_socktype = SOCK_STREAM; hints.ai_flags = AI_NUMERICHOST; - if (!getaddrinfo(ip.c_str(), nullptr, &hints, &result)) { - struct sockaddr_in6* ipAddr = (struct sockaddr_in6*)result->ai_addr; - addr_.in6Addr_ = ipAddr->sin6_addr; - scope_ = uint16_t(ipAddr->sin6_scope_id); - freeaddrinfo(result); - } else { - throw IPAddressFormatException(sformat("Invalid IPv6 address '{}'", ip)); + if (::getaddrinfo(ip.c_str(), nullptr, &hints, &result) == 0) { + SCOPE_EXIT { + ::freeaddrinfo(result); + }; + const struct sockaddr_in6* sa = + reinterpret_cast(result->ai_addr); + return IPAddressV6(*sa); } + return makeUnexpected(IPAddressFormatError::INVALID_IP); } // in6_addr constructor -IPAddressV6::IPAddressV6(const in6_addr& src) : addr_(src) {} +IPAddressV6::IPAddressV6(const in6_addr& src) noexcept : addr_(src) {} // sockaddr_in6 constructor -IPAddressV6::IPAddressV6(const sockaddr_in6& src) +IPAddressV6::IPAddressV6(const sockaddr_in6& src) noexcept : addr_(src.sin6_addr), scope_(uint16_t(src.sin6_scope_id)) {} // ByteArray16 constructor -IPAddressV6::IPAddressV6(const ByteArray16& src) : addr_(src) {} +IPAddressV6::IPAddressV6(const ByteArray16& src) noexcept : addr_(src) {} // link-local constructor IPAddressV6::IPAddressV6(LinkLocalTag, MacAddress mac) : addr_(mac) {} @@ -165,14 +165,34 @@ Optional IPAddressV6::getMacAddressFromEUI64() const { return Optional(MacAddress::fromBinary(range(bytes))); } -void IPAddressV6::setFromBinary(ByteRange bytes) { - if (bytes.size() != 16) { - throw IPAddressFormatException(sformat( - "Invalid IPv6 binary data: length must be 16 bytes, got {}", +IPAddressV6 IPAddressV6::fromBinary(ByteRange bytes) { + auto maybeIp = tryFromBinary(bytes); + if (maybeIp.hasError()) { + throw IPAddressFormatException(to( + "Invalid IPv6 binary data: length must be 16 bytes, got ", bytes.size())); } + return maybeIp.value(); +} + +Expected IPAddressV6::tryFromBinary( + ByteRange bytes) noexcept { + IPAddressV6 addr; + auto setResult = addr.trySetFromBinary(bytes); + if (setResult.hasError()) { + return makeUnexpected(std::move(setResult.error())); + } + return addr; +} + +Expected IPAddressV6::trySetFromBinary( + ByteRange bytes) noexcept { + if (bytes.size() != 16) { + return makeUnexpected(IPAddressFormatError::INVALID_IP); + } memcpy(&addr_.in6Addr_.s6_addr, bytes.data(), sizeof(in6_addr)); scope_ = 0; + return unit; } // static diff --git a/folly/IPAddressV6.h b/folly/IPAddressV6.h index 78ec4c76..161a2404 100644 --- a/folly/IPAddressV6.h +++ b/folly/IPAddressV6.h @@ -24,7 +24,9 @@ #include #include +#include #include +#include #include #include #include @@ -92,17 +94,27 @@ class IPAddressV6 { 8 /*words*/ * 4 /*hex chars per word*/ + 7 /*separators*/; // returns true iff the input string can be parsed as an ipv6-address - static bool validate(StringPiece ip); + static bool validate(StringPiece ip) noexcept; /** * Create a new IPAddress instance from the provided binary data. * @throws IPAddressFormatException if the input length is not 16 bytes. */ - static IPAddressV6 fromBinary(ByteRange bytes) { - IPAddressV6 addr; - addr.setFromBinary(bytes); - return addr; - } + static IPAddressV6 fromBinary(ByteRange bytes); + + /** + * Non-throwing version of fromBinary(). + * On failure returns IPAddressFormatError. + */ + static Expected tryFromBinary( + ByteRange bytes) noexcept; + + /** + * Tries to create a new IPAddressV6 instance from provided string and + * returns it on success. Returns IPAddressFormatError on failure. + */ + static Expected tryFromString( + StringPiece str) noexcept; /** * Create a new IPAddress instance from the ip6.arpa representation. @@ -131,13 +143,13 @@ class IPAddressV6 { explicit IPAddressV6(StringPiece ip); // ByteArray16 constructor - explicit IPAddressV6(const ByteArray16& src); + explicit IPAddressV6(const ByteArray16& src) noexcept; // in6_addr constructor - explicit IPAddressV6(const in6_addr& src); + explicit IPAddressV6(const in6_addr& src) noexcept; // sockaddr_in6 constructor - explicit IPAddressV6(const sockaddr_in6& src); + explicit IPAddressV6(const sockaddr_in6& src) noexcept; /** * Create a link-local IPAddressV6 from the specified ethernet MAC address. @@ -406,9 +418,10 @@ class IPAddressV6 { /** * Set the current IPAddressV6 object to have the address specified by bytes. - * @throws IPAddressFormatException if bytes.size() is not 16. + * Returns IPAddressFormatError if bytes.size() is not 16. */ - void setFromBinary(ByteRange bytes); + Expected trySetFromBinary( + ByteRange bytes) noexcept; }; // boost::hash uses hash_value() so this allows boost::hash to work diff --git a/folly/test/IPAddressBenchmark.cpp b/folly/test/IPAddressBenchmark.cpp index fc9ca727..06a76658 100644 --- a/folly/test/IPAddressBenchmark.cpp +++ b/folly/test/IPAddressBenchmark.cpp @@ -116,6 +116,52 @@ BENCHMARK_RELATIVE(ipv6_append_to_fully_qualified_port, iters) { } } +BENCHMARK_DRAW_LINE() + +BENCHMARK(ipv6_ctor_valid, iters) { + while (iters--) { + try { + IPAddressV6 ip("2803:6082:18e0:2c49:1a23:9ee0:5c87:9800"); + doNotOptimizeAway(ip); + } catch (const IPAddressFormatException& ex) { + doNotOptimizeAway(ex); + } + } +} + +BENCHMARK_RELATIVE(ipv6_ctor_invalid, iters) { + while (iters--) { + try { + IPAddressV6 ip("2803:6082:18e0:2c49:1a23:9ee0:5c87:980r"); + doNotOptimizeAway(ip); + } catch (const IPAddressFormatException& ex) { + doNotOptimizeAway(ex); + } + } +} + +BENCHMARK_DRAW_LINE() + +BENCHMARK(ipv6_try_from_string_valid, iters) { + while (iters--) { + auto maybeIp = + IPAddressV6::tryFromString("2803:6082:18e0:2c49:1a23:9ee0:5c87:9800"); + CHECK(maybeIp.hasValue()); + doNotOptimizeAway(maybeIp); + doNotOptimizeAway(maybeIp.value()); + } +} + +BENCHMARK_RELATIVE(ipv6_try_from_string_invalid, iters) { + while (iters--) { + auto maybeIp = + IPAddressV6::tryFromString("2803:6082:18e0:2c49:1a23:9ee0:5c87:980r"); + CHECK(maybeIp.hasError()); + doNotOptimizeAway(maybeIp); + doNotOptimizeAway(maybeIp.error()); + } +} + // Benchmark results on Intel Xeon CPU E5-2660 @ 2.20GHz // ============================================================================ // folly/test/IPAddressBenchmark.cpp relative time/iter iters/s @@ -131,6 +177,12 @@ BENCHMARK_RELATIVE(ipv6_append_to_fully_qualified_port, iters) { // ---------------------------------------------------------------------------- // ipv6_to_fully_qualified_port 150.76ns 6.63M // ipv6_append_to_fully_qualified_port 178.73% 84.35ns 11.86M +// ---------------------------------------------------------------------------- +// ipv6_ctor_valid 379.97ns 2.63M +// ipv6_ctor_invalid 10.38% 3.66us 273.22K +// ---------------------------------------------------------------------------- +// ipv6_try_from_string_valid 375.34ns 2.66M +// ipv6_try_from_string_invalid 111.93% 335.34ns 2.98M // ============================================================================ int main(int argc, char* argv[]) { diff --git a/folly/test/IPAddressTest.cpp b/folly/test/IPAddressTest.cpp index 5f7c1f62..02f54676 100644 --- a/folly/test/IPAddressTest.cpp +++ b/folly/test/IPAddressTest.cpp @@ -154,6 +154,38 @@ struct IPAddressSerializeTest : TestWithParam {}; struct IPAddressByteAccessorTest : TestWithParam {}; struct IPAddressBitAccessorTest : TestWithParam {}; +struct StringTestParam { + std::string in; + folly::Optional out; + folly::Optional out4; + folly::Optional out6; +}; + +struct TryFromStringTest : TestWithParam { + static std::vector ipInOutProvider() { + const std::string lo6{"::1"}; + const std::string lo6brackets{"[::1]"}; + const std::string ip6{"1234::abcd"}; + const std::string invalid6{"[::aaaR]"}; + + const std::string lo4{"127.0.0.1"}; + const std::string ip4{"192.168.0.1"}; + const std::string invalid4{"127.0.0.256"}; + + const static std::vector ret = { + {lo6, lo6, none, lo6}, + {lo6brackets, lo6, none, lo6}, + {ip6, ip6, none, ip6}, + {invalid6, none, none, none}, + {lo4, lo4, lo4, none}, + {ip4, ip4, ip4, none}, + {invalid4, none, none, none}, + }; + + return ret; + } +}; + // tests code example TEST(IPAddress, CodeExample) { EXPECT_EQ(4, sizeof(IPAddressV4)); @@ -531,6 +563,44 @@ TEST(IPAddress, ToSockaddrStorage) { } } +TEST_P(TryFromStringTest, IPAddress) { + auto param = GetParam(); + auto maybeIp = IPAddress::tryFromString(param.in); + if (param.out) { + EXPECT_TRUE(maybeIp.hasValue()); + EXPECT_EQ(param.out, maybeIp.value().str()); + } else { + EXPECT_TRUE(maybeIp.hasError()); + EXPECT_TRUE( + IPAddressFormatError::INVALID_IP == maybeIp.error() || + IPAddressFormatError::UNSUPPORTED_ADDR_FAMILY == maybeIp.error()); + } +} + +TEST_P(TryFromStringTest, IPAddressV4) { + auto param = GetParam(); + auto maybeIp = IPAddressV4::tryFromString(param.in); + if (param.out4) { + EXPECT_TRUE(maybeIp.hasValue()); + EXPECT_EQ(param.out4, maybeIp.value().str()); + } else { + EXPECT_TRUE(maybeIp.hasError()); + EXPECT_EQ(IPAddressFormatError::INVALID_IP, maybeIp.error()); + } +} + +TEST_P(TryFromStringTest, IPAddressV6) { + auto param = GetParam(); + auto maybeIp = IPAddressV6::tryFromString(param.in); + if (param.out6) { + EXPECT_TRUE(maybeIp.hasValue()); + EXPECT_EQ(param.out6, maybeIp.value().str()); + } else { + EXPECT_TRUE(maybeIp.hasError()); + EXPECT_EQ(IPAddressFormatError::INVALID_IP, maybeIp.error()); + } +} + TEST(IPAddress, ToString) { // Test with IPAddressV4 IPAddressV4 addr_10_0_0_1("10.0.0.1"); @@ -587,12 +657,18 @@ TEST_P(IPAddressCtorTest, InvalidCreation) { << "should have thrown an IPAddressFormatException"; } -// Test that invalid binary values throw an exception +// Test that invalid binary values throw or return an exception TEST_P(IPAddressCtorBinaryTest, InvalidBinary) { auto bin = GetParam(); - EXPECT_THROW( - IPAddress::fromBinary(ByteRange(&bin[0], bin.size())), - IPAddressFormatException); + auto byteRange = ByteRange(&bin[0], bin.size()); + // Throwing versions. + EXPECT_THROW(IPAddress::fromBinary(byteRange), IPAddressFormatException); + EXPECT_THROW(IPAddressV4::fromBinary(byteRange), IPAddressFormatException); + EXPECT_THROW(IPAddressV6::fromBinary(byteRange), IPAddressFormatException); + // Non-throwing versions. + EXPECT_TRUE(IPAddress::tryFromBinary(byteRange).hasError()); + EXPECT_TRUE(IPAddressV4::tryFromBinary(byteRange).hasError()); + EXPECT_TRUE(IPAddressV6::tryFromBinary(byteRange).hasError()); } TEST(IPAddressSource, ToHex) { @@ -837,6 +913,10 @@ TEST(IPAddress, fromBinaryV4) { addr2 = IPAddressV4::fromBinary(bytes); EXPECT_EQ(fromStr, addr2); + auto maybeAddr3 = IPAddressV4::tryFromBinary(bytes); + EXPECT_TRUE(maybeAddr3.hasValue()); + EXPECT_EQ(fromStr, maybeAddr3.value()); + IPAddress genericAddr = IPAddress::fromBinary(bytes); ASSERT_TRUE(genericAddr.isV4()); EXPECT_EQ(fromStr, genericAddr.asV4()); @@ -921,6 +1001,10 @@ TEST(IPAddress, fromBinaryV6) { addr2 = IPAddressV6::fromBinary(bytes); EXPECT_EQ(fromStr, addr2); + auto maybeAddr3 = IPAddressV6::tryFromBinary(bytes); + EXPECT_TRUE(maybeAddr3.hasValue()); + EXPECT_EQ(fromStr, maybeAddr3.value()); + IPAddress genericAddr = IPAddress::fromBinary(bytes); ASSERT_TRUE(genericAddr.isV6()); EXPECT_EQ(fromStr, genericAddr.asV6()); @@ -1458,6 +1542,10 @@ INSTANTIATE_TEST_CASE_P( IPAddress, IPAddressBitAccessorTest, ::testing::ValuesIn(validAddressProvider)); +INSTANTIATE_TEST_CASE_P( + IPAddress, + TryFromStringTest, + ::testing::ValuesIn(TryFromStringTest::ipInOutProvider())); TEST(IPAddressV4, fetchMask) { struct X : private IPAddressV4 {