Problem: UDP address parser uses ad hoc code to detect multicast address

Solution: factor the code into ip_resolver, add IPv6 support and unit tests.

src/ip_resolver.cpp

@@ -20,6 +20,27 @@
 
 #include "ip_resolver.hpp"
 
+int zmq::ip_addr_t::family () const
+{
+    return generic.sa_family;
+}
+
+bool zmq::ip_addr_t::is_multicast () const
+{
+    if (family () == AF_INET) {
+        uint32_t addr = ntohl (ipv4.sin_addr.s_addr);
+
+        //  IPv4 Multicast: address MSBs are 1110
+        //  Range: 224.0.0.0 - 239.255.255.255
+        return (addr & 0xf0000000) == 0xe0000000;
+    } else {
+        const uint8_t *addr = (const uint8_t *) &ipv6.sin6_addr;
+
+        //  IPv6 Multicast: ff00::/8
+        return addr[0] == 0xff;
+    }
+}
+
 zmq::ip_resolver_options_t::ip_resolver_options_t () :
     bindable_wanted (false),
     nic_name_allowed (false),
@@ -221,7 +242,7 @@ int zmq::ip_resolver_t::resolve (ip_addr_t *ip_addr_, const char *name_)
     //  for us but since we don't resolve service names it's a bit overkill and
     //  we'd still have to do it manually when the address is resolved by
     //  'resolve_nic_name'
-    if (ip_addr_->generic.sa_family == AF_INET6) {
+    if (ip_addr_->family () == AF_INET6) {
         ip_addr_->ipv6.sin6_port = htons (port);
         ip_addr_->ipv6.sin6_scope_id = zone_id;
     } else {

src/ip_resolver.hpp

@@ -42,6 +42,9 @@ union ip_addr_t
     sockaddr generic;
     sockaddr_in ipv4;
     sockaddr_in6 ipv6;
+
+    int family () const;
+    bool is_multicast () const;
 };
 
 class ip_resolver_options_t

src/tcp_address.cpp

@@ -116,8 +116,7 @@ int zmq::tcp_address_t::resolve (const char *name_, bool local_, bool ipv6_)
 
 int zmq::tcp_address_t::to_string (std::string &addr_)
 {
-    if (address.generic.sa_family != AF_INET
-        && address.generic.sa_family != AF_INET6) {
+    if (address.family () != AF_INET && address.family () != AF_INET6) {
         addr_.clear ();
         return -1;
     }
@@ -132,7 +131,7 @@ int zmq::tcp_address_t::to_string (std::string &addr_)
         return rc;
     }
 
-    if (address.generic.sa_family == AF_INET6) {
+    if (address.family () == AF_INET6) {
         std::stringstream s;
         s << "tcp://[" << hbuf << "]:" << ntohs (address.ipv6.sin6_port);
         addr_ = s.str ();
@@ -164,7 +163,7 @@ const sockaddr *zmq::tcp_address_t::src_addr () const
 
 socklen_t zmq::tcp_address_t::src_addrlen () const
 {
-    if (address.generic.sa_family == AF_INET6)
+    if (address.family () == AF_INET6)
         return (socklen_t) sizeof (source_address.ipv6);
     else
         return (socklen_t) sizeof (source_address.ipv4);
@@ -181,7 +180,7 @@ unsigned short zmq::tcp_address_t::family () const
 sa_family_t zmq::tcp_address_t::family () const
 #endif
 {
-    return address.generic.sa_family;
+    return address.family ();
 }
 
 zmq::tcp_address_mask_t::tcp_address_mask_t () :
@@ -228,7 +227,7 @@ int zmq::tcp_address_mask_t::resolve (const char *name_, bool ipv6_)
 
     // Parse the cidr mask number.
     if (mask_str.empty ()) {
-        if (address.generic.sa_family == AF_INET6)
+        if (address.family () == AF_INET6)
             address_mask = 128;
         else
             address_mask = 32;
@@ -236,8 +235,8 @@ int zmq::tcp_address_mask_t::resolve (const char *name_, bool ipv6_)
         address_mask = 0;
     else {
         const int mask = atoi (mask_str.c_str ());
-        if ((mask < 1) || (address.generic.sa_family == AF_INET6 && mask > 128)
-            || (address.generic.sa_family != AF_INET6 && mask > 32)) {
+        if ((mask < 1) || (address.family () == AF_INET6 && mask > 128)
+            || (address.family () != AF_INET6 && mask > 32)) {
             errno = EINVAL;
             return -1;
         }
@@ -249,8 +248,7 @@ int zmq::tcp_address_mask_t::resolve (const char *name_, bool ipv6_)
 
 int zmq::tcp_address_mask_t::to_string (std::string &addr_)
 {
-    if (address.generic.sa_family != AF_INET
-        && address.generic.sa_family != AF_INET6) {
+    if (address.family () != AF_INET && address.family () != AF_INET6) {
         addr_.clear ();
         return -1;
     }
@@ -267,7 +265,7 @@ int zmq::tcp_address_mask_t::to_string (std::string &addr_)
         return rc;
     }
 
-    if (address.generic.sa_family == AF_INET6) {
+    if (address.family () == AF_INET6) {
         std::stringstream s;
         s << "[" << hbuf << "]/" << address_mask;
         addr_ = s.str ();

src/udp_address.cpp

@@ -79,17 +79,12 @@ int zmq::udp_address_t::resolve (const char *name_, bool bind_)
         return -1;
     }
 
+    is_multicast = addr.is_multicast ();
     dest_address = addr.ipv4;
 
-    // we will check only first byte of IP
-    // and if it from 224 to 239, then it can
-    // represent multicast IP.
-    int i = dest_address.sin_addr.s_addr & 0xFF;
-    if (i >= 224 && i <= 239) {
+    if (is_multicast) {
         multicast = dest_address.sin_addr;
-        is_multicast = true;
-    } else
-        is_multicast = false;
+    }
 
     iface.s_addr = htonl (INADDR_ANY);
     if (iface.s_addr == INADDR_NONE) {

unittests/unittest_ip_resolver.cpp

@@ -157,7 +157,7 @@ static void test_resolve (zmq::ip_resolver_options_t opts_,
 
 #if defined ZMQ_HAVE_WINDOWS
     if (family == AF_INET6 && expected_addr_v4_failover_ != NULL &&
-        addr.generic.sa_family == AF_INET) {
+        addr.family () == AF_INET) {
         //  We've requested an IPv6 but the system gave us an IPv4, use the
         //  failover address
         family = AF_INET;
@@ -167,7 +167,7 @@ static void test_resolve (zmq::ip_resolver_options_t opts_,
     (void)expected_addr_v4_failover_;
 #endif
 
-    TEST_ASSERT_EQUAL (family, addr.generic.sa_family);
+    TEST_ASSERT_EQUAL (family, addr.family ());
 
     if (family == AF_INET6) {
         struct in6_addr expected_addr;
@@ -187,7 +187,6 @@ static void test_resolve (zmq::ip_resolver_options_t opts_,
 
         assert (test_inet_pton (AF_INET, expected_addr_, &expected_addr) == 1);
 
-        TEST_ASSERT_EQUAL (AF_INET, addr.generic.sa_family);
         TEST_ASSERT_EQUAL (expected_addr.s_addr, ip4_addr->sin_addr.s_addr);
         TEST_ASSERT_EQUAL (htons (expected_port_), ip4_addr->sin_port);
     }
@@ -823,6 +822,82 @@ void test_dns_ipv6_scope_port_brackets ()
                   "fdf5:d058:d656::1", 4444, 1);
 }
 
+static void test_addr (int family_, const char *addr_, bool multicast_)
+{
+    if (family_ == AF_INET6 && !is_ipv6_available ()) {
+        TEST_IGNORE_MESSAGE ("ipv6 is not available");
+    }
+
+    zmq::ip_resolver_options_t resolver_opts;
+
+    resolver_opts.ipv6 (family_ == AF_INET6);
+
+    test_ip_resolver_t resolver (resolver_opts);
+    zmq::ip_addr_t addr;
+
+    int rc = resolver.resolve (&addr, addr_);
+
+    assert (rc == 0);
+
+    TEST_ASSERT_EQUAL (family_, addr.family ());
+    TEST_ASSERT_EQUAL (multicast_, addr.is_multicast ());
+}
+
+static void test_addr_unicast_ipv4 ()
+{
+    test_addr (AF_INET, "1.2.3.4", false);
+}
+
+static void test_addr_unicast_ipv6 ()
+{
+    test_addr (AF_INET6, "abcd::1", false);
+}
+
+static void test_addr_multicast_ipv4 ()
+{
+    test_addr (AF_INET, "230.1.2.3", true);
+}
+
+static void test_addr_multicast_ipv6 ()
+{
+    test_addr (AF_INET6, "ffab::1234", true);
+}
+
+static void test_addr_multicast_ipv4_min ()
+{
+    test_addr (AF_INET, "224.0.0.0", true);
+}
+
+static void test_addr_multicast_ipv6_min ()
+{
+    test_addr (AF_INET6, "ff00::", true);
+}
+
+static void test_addr_multicast_ipv4_max ()
+{
+    test_addr (AF_INET, "239.255.255.255", true);
+}
+
+static void test_addr_multicast_ipv6_max ()
+{
+    test_addr (AF_INET6, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", true);
+}
+
+static void test_addr_multicast_ipv4_sub ()
+{
+    test_addr (AF_INET, "223.255.255.255", false);
+}
+
+static void test_addr_multicast_ipv6_sub ()
+{
+    test_addr (AF_INET6, "feff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false);
+}
+
+static void test_addr_multicast_ipv4_over ()
+{
+    test_addr (AF_INET, "240.0.0.0", false);
+}
+
 int main (void)
 {
     zmq::initialize_network ();
@@ -901,6 +976,17 @@ int main (void)
     RUN_TEST (test_dns_ipv6_scope);
     RUN_TEST (test_dns_ipv6_scope_port);
     RUN_TEST (test_dns_ipv6_scope_port_brackets);
+    RUN_TEST (test_addr_unicast_ipv4);
+    RUN_TEST (test_addr_unicast_ipv6);
+    RUN_TEST (test_addr_multicast_ipv4);
+    RUN_TEST (test_addr_multicast_ipv6);
+    RUN_TEST (test_addr_multicast_ipv4_min);
+    RUN_TEST (test_addr_multicast_ipv6_min);
+    RUN_TEST (test_addr_multicast_ipv4_max);
+    RUN_TEST (test_addr_multicast_ipv6_max);
+    RUN_TEST (test_addr_multicast_ipv4_sub);
+    RUN_TEST (test_addr_multicast_ipv6_sub);
+    RUN_TEST (test_addr_multicast_ipv4_over);
 
     zmq::shutdown_network ();