From: Petr Lapukhov Date: Wed, 17 Jan 2018 03:01:23 +0000 (-0800) Subject: add tryCreateNetwork() X-Git-Tag: v2018.01.22.00~24 X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=9bf260657e140cace3cab8850d6a40fac3a5cd93;ds=sidebyside add tryCreateNetwork() Summary: Add non-throwing version of createNetwork(), and rework existing code to throw exceptions based on error codes returned by non-throwing version. Reviewed By: yfeldblum Differential Revision: D6705425 fbshipit-source-id: 268ff64c36e7cceeea3463248d18b7b2cb81390e --- diff --git a/folly/IPAddress.cpp b/folly/IPAddress.cpp index 97c00d73..5f698f28 100644 --- a/folly/IPAddress.cpp +++ b/folly/IPAddress.cpp @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #include #include @@ -68,43 +67,108 @@ IPAddressV6 IPAddress::createIPv6(const IPAddress& addr) { } } +namespace { +vector splitIpSlashCidr(StringPiece ipSlashCidr) { + vector vec; + split("/", ipSlashCidr, vec); + return vec; +} +} // namespace + // public static CIDRNetwork IPAddress::createNetwork( StringPiece ipSlashCidr, int defaultCidr, /* = -1 */ bool applyMask /* = true */) { - if (defaultCidr > std::numeric_limits::max()) { + auto const ret = + IPAddress::tryCreateNetwork(ipSlashCidr, defaultCidr, applyMask); + + if (ret.hasValue()) { + return ret.value(); + } + + if (ret.error() == CIDRNetworkError::INVALID_DEFAULT_CIDR) { throw std::range_error("defaultCidr must be <= UINT8_MAX"); } - vector vec; - split("/", ipSlashCidr, vec); - vector::size_type elemCount = vec.size(); - if (elemCount == 0 || // weird invalid string - elemCount > 2) { // invalid string (IP/CIDR/extras) + if (ret.error() == CIDRNetworkError::INVALID_IP_SLASH_CIDR) { throw IPAddressFormatException(sformat( "Invalid ipSlashCidr specified. Expected IP/CIDR format, got '{}'", ipSlashCidr)); } - IPAddress subnet(vec.at(0)); - auto cidr = - uint8_t((defaultCidr > -1) ? defaultCidr : (subnet.isV4() ? 32 : 128)); - if (elemCount == 2) { - try { - cidr = to(vec.at(1)); - } catch (...) { + // Handler the remaining error cases. We re-parse the ip/mask pair + // to make error messages more meaningful + auto const vec = splitIpSlashCidr(ipSlashCidr); + + switch (ret.error()) { + case CIDRNetworkError::INVALID_IP: + CHECK_GE(vec.size(), 1); + throw IPAddressFormatException( + sformat("Invalid IP address {}", vec.at(0))); + case CIDRNetworkError::INVALID_CIDR: + CHECK_GE(vec.size(), 2); throw IPAddressFormatException( sformat("Mask value '{}' not a valid mask", vec.at(1))); + case CIDRNetworkError::CIDR_MISMATCH: { + auto const subnet = IPAddress::tryFromString(vec.at(0)).value(); + auto cidr = static_cast( + (defaultCidr > -1) ? defaultCidr : (subnet.isV4() ? 32 : 128)); + + throw IPAddressFormatException(sformat( + "CIDR value '{}' is > network bit count '{}'", + vec.size() == 2 ? vec.at(1) : to(cidr), + subnet.bitCount())); } + default: + // unreachable + break; } - if (cidr > subnet.bitCount()) { - throw IPAddressFormatException(sformat( - "CIDR value '{}' is > network bit count '{}'", - cidr, - subnet.bitCount())); + + CHECK(0); + + return CIDRNetwork{}; +} + +// public static +Expected IPAddress::tryCreateNetwork( + StringPiece ipSlashCidr, + int defaultCidr, + bool applyMask) { + if (defaultCidr > std::numeric_limits::max()) { + return makeUnexpected(CIDRNetworkError::INVALID_DEFAULT_CIDR); + } + + auto const vec = splitIpSlashCidr(ipSlashCidr); + auto const elemCount = vec.size(); + + if (elemCount == 0 || // weird invalid string + elemCount > 2) { // invalid string (IP/CIDR/extras) + return makeUnexpected(CIDRNetworkError::INVALID_IP_SLASH_CIDR); } - return std::make_pair(applyMask ? subnet.mask(cidr) : subnet, cidr); + + auto const subnet = IPAddress::tryFromString(vec.at(0)); + if (subnet.hasError()) { + return makeUnexpected(CIDRNetworkError::INVALID_IP); + } + + auto cidr = static_cast( + (defaultCidr > -1) ? defaultCidr : (subnet.value().isV4() ? 32 : 128)); + + if (elemCount == 2) { + auto const maybeCidr = tryTo(vec.at(1)); + if (maybeCidr.hasError()) { + return makeUnexpected(CIDRNetworkError::INVALID_CIDR); + } + cidr = maybeCidr.value(); + } + + if (cidr > subnet.value().bitCount()) { + return makeUnexpected(CIDRNetworkError::CIDR_MISMATCH); + } + + return std::make_pair( + applyMask ? subnet.value().mask(cidr) : subnet.value(), cidr); } // public static diff --git a/folly/IPAddress.h b/folly/IPAddress.h index 193f67fe..943c5cec 100644 --- a/folly/IPAddress.h +++ b/folly/IPAddress.h @@ -91,6 +91,19 @@ class IPAddress { * is -1, will use /32 for IPv4 and /128 for IPv6) * @param [in] mask apply mask on the address or not, * e.g. 192.168.13.46/24 => 192.168.13.0/24 + * @return either pair with IPAddress network and uint8_t mask or + * CIDRNetworkError + */ + static Expected tryCreateNetwork( + StringPiece ipSlashCidr, + int defaultCidr = -1, + bool mask = true); + + /** + * Create a network and mask from a CIDR formatted address string. + * Same as tryCreateNetwork() but throws IPAddressFormatException on error. + * The implementation calls tryCreateNetwork(...) underneath + * * @throws IPAddressFormatException if invalid address * @return pair with IPAddress network and uint8_t mask */ diff --git a/folly/IPAddressException.h b/folly/IPAddressException.h index 1db11177..feb0284c 100644 --- a/folly/IPAddressException.h +++ b/folly/IPAddressException.h @@ -29,6 +29,17 @@ namespace folly { */ enum class IPAddressFormatError { INVALID_IP, UNSUPPORTED_ADDR_FAMILY }; +/** + * Wraps error from parsing IP/MASK string + */ +enum class CIDRNetworkError { + INVALID_DEFAULT_CIDR, + INVALID_IP_SLASH_CIDR, + INVALID_IP, + INVALID_CIDR, + CIDR_MISMATCH, +}; + /** * Exception for invalid IP addresses. */ diff --git a/folly/test/IPAddressTest.cpp b/folly/test/IPAddressTest.cpp index 422e615a..e52f581e 100644 --- a/folly/test/IPAddressTest.cpp +++ b/folly/test/IPAddressTest.cpp @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - #include #include @@ -281,10 +280,10 @@ TEST(IPAddress, InvalidAddressFamilyExceptions) { } } -TEST(IPAddress, CreateNetwork) { +TEST(IPAddress, TryCreateNetwork) { // test valid IPv4 network { - auto net = IPAddress::createNetwork("192.168.0.1/24"); + auto net = IPAddress::tryCreateNetwork("192.168.0.1/24").value(); ASSERT_TRUE(net.first.isV4()); EXPECT_EQ("192.168.0.0", net.first.str()); EXPECT_EQ(24, net.second); @@ -292,7 +291,7 @@ TEST(IPAddress, CreateNetwork) { } // test valid IPv4 network without applying mask { - auto net = IPAddress::createNetwork("192.168.0.1/24", -1, false); + auto net = IPAddress::tryCreateNetwork("192.168.0.1/24", -1, false).value(); ASSERT_TRUE(net.first.isV4()); EXPECT_EQ("192.168.0.1", net.first.str()); EXPECT_EQ(24, net.second); @@ -300,7 +299,7 @@ TEST(IPAddress, CreateNetwork) { } // test valid IPv6 network { - auto net = IPAddress::createNetwork("1999::1/24"); + auto net = IPAddress::tryCreateNetwork("1999::1/24").value(); ASSERT_TRUE(net.first.isV6()); EXPECT_EQ("1999::", net.first.str()); EXPECT_EQ(24, net.second); @@ -308,20 +307,30 @@ TEST(IPAddress, CreateNetwork) { } // test valid IPv6 network without applying mask { - auto net = IPAddress::createNetwork("1999::1/24", -1, false); + auto net = IPAddress::tryCreateNetwork("1999::1/24", -1, false).value(); ASSERT_TRUE(net.first.isV6()); EXPECT_EQ("1999::1", net.first.str()); EXPECT_EQ(24, net.second); EXPECT_EQ("1999::1/24", IPAddress::networkToString(net)); } + + // test invalid default CIDR + EXPECT_EQ( + CIDRNetworkError::INVALID_DEFAULT_CIDR, + IPAddress::tryCreateNetwork("192.168.1.1", 300).error()); + // test empty string - EXPECT_THROW(IPAddress::createNetwork(""), IPAddressFormatException); + EXPECT_EQ( + CIDRNetworkError::INVALID_IP, IPAddress::tryCreateNetwork("").error()); + // test multi slash string - EXPECT_THROW( - IPAddress::createNetwork("192.168.0.1/24/36"), IPAddressFormatException); + EXPECT_EQ( + CIDRNetworkError::INVALID_IP_SLASH_CIDR, + IPAddress::tryCreateNetwork("192.168.0.1/24/36").error()); + // test no slash string with default IPv4 { - auto net = IPAddress::createNetwork("192.168.0.1"); + auto net = IPAddress::tryCreateNetwork("192.168.0.1").value(); ASSERT_TRUE(net.first.isV4()); EXPECT_EQ("192.168.0.1", net.first.str()); EXPECT_EQ(32, net.second); // auto-detected @@ -332,12 +341,27 @@ TEST(IPAddress, CreateNetwork) { } // test no slash string with default IPv6 { - auto net = IPAddress::createNetwork("1999::1"); + auto net = IPAddress::tryCreateNetwork("1999::1").value(); ASSERT_TRUE(net.first.isV6()); EXPECT_EQ("1999::1", net.first.str()); EXPECT_EQ(128, net.second); } // test no slash string with invalid default + EXPECT_EQ( + CIDRNetworkError::CIDR_MISMATCH, + IPAddress::tryCreateNetwork("192.168.0.1", 33).error()); +} + +// test that throwing version actually throws +TEST(IPAddress, CreateNetworkExceptions) { + // test invalid default CIDR + EXPECT_THROW(IPAddress::createNetwork("192.168.0.1", 300), std::range_error); + // test empty string + EXPECT_THROW(IPAddress::createNetwork(""), IPAddressFormatException); + // test multi slash string + EXPECT_THROW( + IPAddress::createNetwork("192.168.0.1/24/36"), IPAddressFormatException); + // test no slash string with invalid default EXPECT_THROW( IPAddress::createNetwork("192.168.0.1", 33), IPAddressFormatException); }