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

folly/IPAddress.cpp

@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #include <folly/IPAddress.h>
 
 #include <limits>
@@ -68,43 +67,108 @@ IPAddressV6 IPAddress::createIPv6(const IPAddress& addr) {
   }
 }
 
+namespace {
+vector<string> splitIpSlashCidr(StringPiece ipSlashCidr) {
+  vector<string> 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<uint8_t>::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<string> vec;
-  split("/", ipSlashCidr, vec);
-  vector<string>::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<uint8_t>(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<uint8_t>(
+          (defaultCidr > -1) ? defaultCidr : (subnet.isV4() ? 32 : 128));
+
+      throw IPAddressFormatException(sformat(
+          "CIDR value '{}' is > network bit count '{}'",
+          vec.size() == 2 ? vec.at(1) : to<string>(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<CIDRNetwork, CIDRNetworkError> IPAddress::tryCreateNetwork(
+    StringPiece ipSlashCidr,
+    int defaultCidr,
+    bool applyMask) {
+  if (defaultCidr > std::numeric_limits<uint8_t>::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<uint8_t>(
+      (defaultCidr > -1) ? defaultCidr : (subnet.value().isV4() ? 32 : 128));
+
+  if (elemCount == 2) {
+    auto const maybeCidr = tryTo<uint8_t>(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

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<CIDRNetwork, CIDRNetworkError> 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
    */

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.
  */

folly/test/IPAddressTest.cpp

@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #include <sys/types.h>
 
 #include <string>
@@ -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);
 }