From 3fa0eea4d861e1b3c347d0bed6773e8371bc4da4 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 18 May 2025 22:25:53 +1000 Subject: [PATCH 1/7] Remove the udp_key function and make sock_t able to be a map key directly --- src/extension/network/NUClearNetwork.cpp | 44 +--- src/extension/network/NUClearNetwork.hpp | 4 +- src/util/network/sock_t.cpp | 63 ++++++ src/util/network/sock_t.hpp | 116 ++++++++--- tests/tests/util/network/sock_t.cpp | 249 +++++++++++++++++++++++ 5 files changed, 411 insertions(+), 65 deletions(-) create mode 100644 src/util/network/sock_t.cpp create mode 100644 tests/tests/util/network/sock_t.cpp diff --git a/src/extension/network/NUClearNetwork.cpp b/src/extension/network/NUClearNetwork.cpp index ae0118b7..ded56372 100644 --- a/src/extension/network/NUClearNetwork.cpp +++ b/src/extension/network/NUClearNetwork.cpp @@ -112,37 +112,11 @@ namespace extension { next_event_callback = std::move(f); } - std::array NUClearNetwork::udp_key(const sock_t& address) { - - // Get our keys for our maps, it will be the ip and then port - std::array key = {0}; - - switch (address.sock.sa_family) { - case AF_INET: - // The first chars are 0 (ipv6) and after that is our address and then port - std::memcpy(&key[6], &address.ipv4.sin_addr, sizeof(address.ipv4.sin_addr)); - key[8] = address.ipv4.sin_port; - break; - - case AF_INET6: - // IPv6 address then port - std::memcpy(key.data(), &address.ipv6.sin6_addr, sizeof(address.ipv6.sin6_addr)); - key[8] = address.ipv6.sin6_port; - break; - - default: throw std::invalid_argument("Unknown address family"); - } - - return key; - } - - void NUClearNetwork::remove_target(const std::shared_ptr& target) { // Erase udp - auto key = udp_key(target->target); - if (udp_target.find(key) != udp_target.end()) { - udp_target.erase(udp_target.find(key)); + if (udp_target.find(target->target) != udp_target.end()) { + udp_target.erase(udp_target.find(target->target)); } // Erase name @@ -161,7 +135,6 @@ namespace extension { } } - void NUClearNetwork::open_data(const sock_t& bind_address) { // Create the "join any" address for this address family @@ -383,7 +356,7 @@ namespace extension { auto all_target = std::make_shared("", announce_target); targets.push_front(all_target); name_target.insert(std::make_pair("", all_target)); - udp_target.insert(std::make_pair(udp_key(announce_target), all_target)); + udp_target.insert(std::make_pair(announce_target, all_target)); // Work out our MTU for udp packets packet_data_mtu = network_mtu; // Start with the total mtu @@ -570,14 +543,11 @@ namespace extension { // This is a real packet! get our header information const PacketHeader& header = *reinterpret_cast(payload.data()); - // Get the map key for this device - auto key = udp_key(address); - // From here on, we are doing things with our target lists that if changed would make us sad std::shared_ptr remote; /* Mutex scope */ { const std::lock_guard lock(target_mutex); - auto r = udp_target.find(key); + auto r = udp_target.find(address); remote = r == udp_target.end() ? nullptr : r->second; } @@ -601,10 +571,10 @@ namespace extension { const std::lock_guard lock(target_mutex); // Double check they are new - if (udp_target.count(key) == 0) { + if (udp_target.count(address) == 0) { new_connection = true; targets.push_back(ptr); - udp_target.insert(std::make_pair(key, ptr)); + udp_target.insert(std::make_pair(address, ptr)); name_target.insert(std::make_pair(name, ptr)); // Say hi back! @@ -639,7 +609,7 @@ namespace extension { const std::lock_guard lock(target_mutex); // Double check they are gone after locking before removal - if (udp_target.count(key) > 0) { + if (udp_target.count(address) > 0) { left = true; remove_target(remote); } diff --git a/src/extension/network/NUClearNetwork.hpp b/src/extension/network/NUClearNetwork.hpp index d5f17e6d..c0779f2d 100644 --- a/src/extension/network/NUClearNetwork.hpp +++ b/src/extension/network/NUClearNetwork.hpp @@ -310,8 +310,8 @@ namespace extension { /// A map of string names to targets with that name std::multimap, std::less<>> name_target; - /// A map of ip/port pairs to the network target they belong to - std::map, std::shared_ptr> udp_target; + /// A map of socket addresses to the network target they belong to + std::map> udp_target; }; } // namespace network diff --git a/src/util/network/sock_t.cpp b/src/util/network/sock_t.cpp new file mode 100644 index 00000000..be806915 --- /dev/null +++ b/src/util/network/sock_t.cpp @@ -0,0 +1,63 @@ +/* + * MIT License + * + * Copyright (c) 2025 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#include "sock_t.hpp" + +#include + +#include + +namespace NUClear { +namespace util { + namespace network { + + socklen_t sock_t::size() const { + switch (sock.sa_family) { + case AF_INET: return sizeof(sockaddr_in); + case AF_INET6: return sizeof(sockaddr_in6); + default: throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); + } + } + + std::pair sock_t::address(bool numeric) const { + std::array host{}; + std::array service{}; + + if (::getnameinfo(&sock, + size(), + host.data(), + static_cast(host.size()), + service.data(), + static_cast(service.size()), + NI_NUMERICSERV | (numeric ? NI_NUMERICHOST : 0)) + != 0) { + throw std::system_error( + network_errno, + std::system_category(), + "Cannot get address for socket address family " + std::to_string(sock.sa_family)); + } + + return {host.data(), static_cast(std::stoi(service.data()))}; + } + + } // namespace network +} // namespace util +} // namespace NUClear diff --git a/src/util/network/sock_t.hpp b/src/util/network/sock_t.hpp index 4c52b2c0..8b005a22 100644 --- a/src/util/network/sock_t.hpp +++ b/src/util/network/sock_t.hpp @@ -24,9 +24,12 @@ #define NUCLEAR_UTIL_NETWORK_SOCK_T_HPP #include +#include #include #include +#include #include +#include #include "../platform.hpp" @@ -34,42 +37,103 @@ namespace NUClear { namespace util { namespace network { + /** + * A struct representing a socket address, supporting both IPv4 and IPv6. + * This struct provides a unified interface for handling socket addresses across different address families. + */ struct sock_t { + /** + * A union of socket address structures, allowing access to the address in different formats. + * This union includes sockaddr_storage, sockaddr, sockaddr_in, and sockaddr_in6. + */ union { - sockaddr_storage storage; - sockaddr sock; - sockaddr_in ipv4; - sockaddr_in6 ipv6; + sockaddr_storage storage; //< The storage for the socket address + sockaddr sock; //< The socket address + sockaddr_in ipv4; //< The IPv4 address + sockaddr_in6 ipv6; //< The IPv6 address }; - socklen_t size() const { - switch (sock.sa_family) { - case AF_INET: return sizeof(sockaddr_in); - case AF_INET6: return sizeof(sockaddr_in6); + /** + * Equality comparison operator for sock_t. + * + * @param a The first sock_t object + * @param b The second sock_t object + * + * @return true if the addresses are equal, false otherwise. + */ + friend bool operator==(const sock_t& a, const sock_t& b) { + if (a.sock.sa_family != b.sock.sa_family) { + return false; + } + + switch (a.sock.sa_family) { + case AF_INET: + return a.ipv4.sin_port == b.ipv4.sin_port && a.ipv4.sin_addr.s_addr == b.ipv4.sin_addr.s_addr; + case AF_INET6: + return a.ipv6.sin6_port == b.ipv6.sin6_port + && std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0; default: - throw std::runtime_error("Cannot get size for socket address family " - + std::to_string(sock.sa_family)); + throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); } } - std::pair address(bool numeric_host = false) const { - std::array host{}; - std::array service{}; - const int result = ::getnameinfo(reinterpret_cast(&storage), - size(), - host.data(), - static_cast(host.size()), - service.data(), - static_cast(service.size()), - NI_NUMERICSERV | (numeric_host ? NI_NUMERICHOST : 0)); - if (result != 0) { - throw std::system_error( - network_errno, - std::system_category(), - "Cannot get address for socket address family " + std::to_string(sock.sa_family)); + /** + * Inequality comparison operator for sock_t. + * + * @param a The first sock_t object + * @param b The second sock_t object + * + * @return true if the addresses are not equal, false otherwise + */ + friend bool operator!=(const sock_t& a, const sock_t& b) { + return !(a == b); + } + + /** + * Less-than comparison operator for sock_t. + * + * @param a The first sock_t object + * @param b The second sock_t object + * + * @return true if a is less than b, false otherwise + */ + friend bool operator<(const sock_t& a, const sock_t& b) { + if (a.sock.sa_family != b.sock.sa_family) { + return a.sock.sa_family < b.sock.sa_family; + } + + switch (a.sock.sa_family) { + case AF_INET: + return std::tie(a.ipv4.sin_addr.s_addr, a.ipv4.sin_port) + < std::tie(b.ipv4.sin_addr.s_addr, b.ipv4.sin_port); + case AF_INET6: { + int cmp = std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)); + return cmp < 0 || (cmp == 0 && a.ipv6.sin6_port < b.ipv6.sin6_port); + } + default: + throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); } - return std::make_pair(std::string(host.data()), static_cast(std::stoi(service.data()))); } + + /** + * Returns the size of the socket address structure. + * + * @return The size of the socket address structure + * + * @throws std::system_error if the address family is unsupported + */ + socklen_t size() const; + + /** + * Resolves the socket address to a hostname and port. + * + * @param numeric If true, returns the numeric IP address instead of the hostname + * + * @return A pair containing the hostname (or numeric IP) and the port + * + * @throws std::system_error if the address cannot be resolved + */ + std::pair address(bool numeric = false) const; }; } // namespace network diff --git a/tests/tests/util/network/sock_t.cpp b/tests/tests/util/network/sock_t.cpp new file mode 100644 index 00000000..8da5b2c5 --- /dev/null +++ b/tests/tests/util/network/sock_t.cpp @@ -0,0 +1,249 @@ +/* + * MIT License + * + * Copyright (c) 2025 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#include "util/network/sock_t.hpp" + +#include +#include + +#include +#include + +using namespace NUClear::util::network; + +SCENARIO("Address families are handled correctly", "[sock_t]") { + GIVEN("An IPv4 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET; + addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr.ipv4.sin_port = htons(12345); + + THEN("its size should match sockaddr_in") { + REQUIRE(addr.size() == sizeof(sockaddr_in)); + } + + THEN("it should resolve to the correct IP and port") { + auto address_result = addr.address(true); // Use numeric host + auto& host = address_result.first; + auto& port = address_result.second; + REQUIRE(host == "192.168.1.1"); + REQUIRE(port == 12345); + } + } + + GIVEN("An IPv6 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET6; + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + addr.ipv6.sin6_port = htons(54321); + + THEN("its size should match sockaddr_in6") { + REQUIRE(addr.size() == sizeof(sockaddr_in6)); + } + + THEN("it should resolve to the correct IP and port") { + auto address_result = addr.address(true); // Use numeric host + auto& host = address_result.first; + auto& port = address_result.second; + REQUIRE(host == "2001:db8::1"); + REQUIRE(port == 54321); + } + } + + GIVEN("An unknown address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unknown family + + THEN("attempting to get its size should throw") { + REQUIRE_THROWS_AS(addr.size(), std::system_error); + } + + THEN("attempting to resolve it should throw") { + REQUIRE_THROWS_AS(addr.address(), std::system_error); + } + } +} + +SCENARIO("Addresses can be compared", "[sock_t]") { + GIVEN("Two identical IPv4 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + THEN("they should be equal") { + REQUIRE(addr1 == addr2); + REQUIRE_FALSE(addr1 != addr2); + } + } + + GIVEN("Two different IPv4 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two identical IPv6 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should be equal") { + REQUIRE(addr1 == addr2); + REQUIRE_FALSE(addr1 != addr2); + } + } + + GIVEN("Two different IPv6 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr1[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + // 2001:db8::2 + uint8_t ipv6_addr2[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("An IPv4 and an IPv6 address") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET6; + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } +} + +SCENARIO("Addresses can be ordered", "[sock_t]") { + GIVEN("Two IPv4 addresses with different IPs") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + // For IPv4 address ordering + bool ipv4_less = (addr1.sock.sa_family < addr2.sock.sa_family) + || (addr1.sock.sa_family == addr2.sock.sa_family + && (addr1.ipv4.sin_addr.s_addr < addr2.ipv4.sin_addr.s_addr + || (addr1.ipv4.sin_addr.s_addr == addr2.ipv4.sin_addr.s_addr + && addr1.ipv4.sin_port < addr2.ipv4.sin_port))); + REQUIRE(ipv4_less); + } + + GIVEN("Two IPv6 addresses with different IPs") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr1[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + // 2001:db8::2 + uint8_t ipv6_addr2[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + // For IPv6 address ordering + int cmp = std::memcmp(&addr1.ipv6.sin6_addr, &addr2.ipv6.sin6_addr, sizeof(in6_addr)); + bool ipv6_less = (addr1.sock.sa_family < addr2.sock.sa_family) + || (addr1.sock.sa_family == addr2.sock.sa_family + && (cmp < 0 || (cmp == 0 && addr1.ipv6.sin6_port < addr2.ipv6.sin6_port))); + REQUIRE(ipv6_less); + } + + GIVEN("An IPv4 and an IPv6 address") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET6; + + THEN("IPv4 should be less than IPv6") { + REQUIRE(addr1.sock.sa_family < addr2.sock.sa_family); + } + } +} From e81fc58cc408dea4d8464d2007fcc613d951a626 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 18 May 2025 22:49:13 +1000 Subject: [PATCH 2/7] More coverage and add an << operator to make the test output better --- src/util/network/sock_t.hpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/util/network/sock_t.hpp b/src/util/network/sock_t.hpp index 8b005a22..6156e227 100644 --- a/src/util/network/sock_t.hpp +++ b/src/util/network/sock_t.hpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -104,11 +105,11 @@ namespace util { switch (a.sock.sa_family) { case AF_INET: - return std::tie(a.ipv4.sin_addr.s_addr, a.ipv4.sin_port) - < std::tie(b.ipv4.sin_addr.s_addr, b.ipv4.sin_port); + return std::forward_as_tuple(a.ipv4.sin_addr.s_addr, ntohs(a.ipv4.sin_port)) + < std::forward_as_tuple(b.ipv4.sin_addr.s_addr, ntohs(b.ipv4.sin_port)); case AF_INET6: { int cmp = std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)); - return cmp < 0 || (cmp == 0 && a.ipv6.sin6_port < b.ipv6.sin6_port); + return cmp < 0 || (cmp == 0 && ntohs(a.ipv6.sin6_port) < ntohs(b.ipv6.sin6_port)); } default: throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); @@ -134,6 +135,19 @@ namespace util { * @throws std::system_error if the address cannot be resolved */ std::pair address(bool numeric = false) const; + + /** + * Output stream operator for sock_t. + * Outputs the address in the format "{host}:{port}" + * + * @param os The output stream to write to + * @param addr The socket address to output + * @return The output stream + */ + friend std::ostream& operator<<(std::ostream& os, const sock_t& addr) { + auto addr_pair = addr.address(true); + return os << addr_pair.first << ":" << addr_pair.second; + } }; } // namespace network From 39a96ae91186f74bc2fb5c27cac8a1261ecf913f Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 18 May 2025 23:01:53 +1000 Subject: [PATCH 3/7] more test --- tests/tests/util/network/sock_t.cpp | 200 ++++++++++++++++++++++++++-- 1 file changed, 186 insertions(+), 14 deletions(-) diff --git a/tests/tests/util/network/sock_t.cpp b/tests/tests/util/network/sock_t.cpp index 8da5b2c5..e4d5ff6a 100644 --- a/tests/tests/util/network/sock_t.cpp +++ b/tests/tests/util/network/sock_t.cpp @@ -25,6 +25,7 @@ #include #include +#include #include using namespace NUClear::util::network; @@ -199,13 +200,27 @@ SCENARIO("Addresses can be ordered", "[sock_t]") { addr1.ipv4.sin_port = htons(12345); addr2.ipv4.sin_port = htons(12345); - // For IPv4 address ordering - bool ipv4_less = (addr1.sock.sa_family < addr2.sock.sa_family) - || (addr1.sock.sa_family == addr2.sock.sa_family - && (addr1.ipv4.sin_addr.s_addr < addr2.ipv4.sin_addr.s_addr - || (addr1.ipv4.sin_addr.s_addr == addr2.ipv4.sin_addr.s_addr - && addr1.ipv4.sin_port < addr2.ipv4.sin_port))); - REQUIRE(ipv4_less); + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } + } + + GIVEN("Two IPv4 addresses with same IP but different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(54321); + + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } } GIVEN("Two IPv6 addresses with different IPs") { @@ -227,12 +242,30 @@ SCENARIO("Addresses can be ordered", "[sock_t]") { addr1.ipv6.sin6_port = htons(54321); addr2.ipv6.sin6_port = htons(54321); - // For IPv6 address ordering - int cmp = std::memcmp(&addr1.ipv6.sin6_addr, &addr2.ipv6.sin6_addr, sizeof(in6_addr)); - bool ipv6_less = (addr1.sock.sa_family < addr2.sock.sa_family) - || (addr1.sock.sa_family == addr2.sock.sa_family - && (cmp < 0 || (cmp == 0 && addr1.ipv6.sin6_port < addr2.ipv6.sin6_port))); - REQUIRE(ipv6_less); + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } + } + + GIVEN("Two IPv6 addresses with same IP but different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + + addr1.ipv6.sin6_port = htons(12345); + addr2.ipv6.sin6_port = htons(54321); + + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } } GIVEN("An IPv4 and an IPv6 address") { @@ -243,7 +276,146 @@ SCENARIO("Addresses can be ordered", "[sock_t]") { addr2.sock.sa_family = AF_INET6; THEN("IPv4 should be less than IPv6") { - REQUIRE(addr1.sock.sa_family < addr2.sock.sa_family); + REQUIRE(addr1 < addr2); + } + } +} + +SCENARIO("Address equality operators handle all conditions", "[sock_t]") { + GIVEN("Two IPv4 addresses with different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two IPv4 addresses with different IPs but same port") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two IPv6 addresses with different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + + addr1.ipv6.sin6_port = htons(12345); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two IPv6 addresses with different IPs but same port") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr1[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + // 2001:db8::2 + uint8_t ipv6_addr2[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } +} + +SCENARIO("Address resolution handles errors", "[sock_t]") { + GIVEN("An unsupported address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unsupported address family + + THEN("attempting to resolve it should throw") { + REQUIRE_THROWS_AS(addr.address(), std::system_error); + } + } +} + +SCENARIO("Socket addresses can be streamed", "[sock_t]") { + GIVEN("An IPv4 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET; + addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr.ipv4.sin_port = htons(12345); + + THEN("it should stream in the correct format") { + std::stringstream ss; + ss << addr; + REQUIRE(ss.str() == "192.168.1.1:12345"); + } + } + + GIVEN("An IPv6 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET6; + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + addr.ipv6.sin6_port = htons(54321); + + THEN("it should stream in the correct format") { + std::stringstream ss; + ss << addr; + REQUIRE(ss.str() == "2001:db8::1:54321"); + } + } + + GIVEN("An unsupported address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unsupported address family + + THEN("streaming should throw") { + std::stringstream ss; + REQUIRE_THROWS_AS(ss << addr, std::system_error); } } } From d223cdf71191b362e4d4199c111e99ba26538268 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 18 May 2025 23:19:22 +1000 Subject: [PATCH 4/7] cover error cases --- src/util/network/sock_t.hpp | 42 ++++++++++++++--------------- tests/tests/util/network/sock_t.cpp | 32 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/util/network/sock_t.hpp b/src/util/network/sock_t.hpp index 6156e227..1836f745 100644 --- a/src/util/network/sock_t.hpp +++ b/src/util/network/sock_t.hpp @@ -63,21 +63,21 @@ namespace util { * @return true if the addresses are equal, false otherwise. */ friend bool operator==(const sock_t& a, const sock_t& b) { - if (a.sock.sa_family != b.sock.sa_family) { - return false; + if ((a.sock.sa_family != AF_INET && a.sock.sa_family != AF_INET6) + || (b.sock.sa_family != AF_INET && b.sock.sa_family != AF_INET6)) { + throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); } - switch (a.sock.sa_family) { - case AF_INET: - return a.ipv4.sin_port == b.ipv4.sin_port && a.ipv4.sin_addr.s_addr == b.ipv4.sin_addr.s_addr; - case AF_INET6: - return a.ipv6.sin6_port == b.ipv6.sin6_port - && std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0; - default: - throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); + if (a.sock.sa_family != b.sock.sa_family) { + return false; } + return a.sock.sa_family == AF_INET + ? a.ipv4.sin_port == b.ipv4.sin_port && a.ipv4.sin_addr.s_addr == b.ipv4.sin_addr.s_addr + : a.ipv6.sin6_port == b.ipv6.sin6_port + && std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0; } + /** * Inequality comparison operator for sock_t. * @@ -99,21 +99,21 @@ namespace util { * @return true if a is less than b, false otherwise */ friend bool operator<(const sock_t& a, const sock_t& b) { + if ((a.sock.sa_family != AF_INET && a.sock.sa_family != AF_INET6) + || (b.sock.sa_family != AF_INET && b.sock.sa_family != AF_INET6)) { + throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); + } + if (a.sock.sa_family != b.sock.sa_family) { return a.sock.sa_family < b.sock.sa_family; } - switch (a.sock.sa_family) { - case AF_INET: - return std::forward_as_tuple(a.ipv4.sin_addr.s_addr, ntohs(a.ipv4.sin_port)) - < std::forward_as_tuple(b.ipv4.sin_addr.s_addr, ntohs(b.ipv4.sin_port)); - case AF_INET6: { - int cmp = std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)); - return cmp < 0 || (cmp == 0 && ntohs(a.ipv6.sin6_port) < ntohs(b.ipv6.sin6_port)); - } - default: - throw std::system_error(EAFNOSUPPORT, std::system_category(), "Unsupported address family"); - } + return a.sock.sa_family == AF_INET + ? std::forward_as_tuple(a.ipv4.sin_addr.s_addr, ntohs(a.ipv4.sin_port)) + < std::forward_as_tuple(b.ipv4.sin_addr.s_addr, ntohs(b.ipv4.sin_port)) + : std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) < 0 + || (std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0 + && ntohs(a.ipv6.sin6_port) < ntohs(b.ipv6.sin6_port)); } /** diff --git a/tests/tests/util/network/sock_t.cpp b/tests/tests/util/network/sock_t.cpp index e4d5ff6a..90840da2 100644 --- a/tests/tests/util/network/sock_t.cpp +++ b/tests/tests/util/network/sock_t.cpp @@ -376,6 +376,38 @@ SCENARIO("Address resolution handles errors", "[sock_t]") { THEN("attempting to resolve it should throw") { REQUIRE_THROWS_AS(addr.address(), std::system_error); } + + THEN("equality comparison should throw") { + sock_t other{}; + other.sock.sa_family = AF_UNSPEC; + REQUIRE_THROWS_AS(addr == other, std::system_error); + REQUIRE_THROWS_AS(addr != other, std::system_error); + } + + THEN("less than comparison should throw") { + sock_t other{}; + other.sock.sa_family = AF_UNSPEC; + REQUIRE_THROWS_AS(addr < other, std::system_error); + } + } + + GIVEN("A socket with an unsupported address family compared with a valid socket") { + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + sock_t valid_addr{}; + valid_addr.sock.sa_family = AF_INET; + valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + valid_addr.ipv4.sin_port = htons(12345); + + THEN("equality comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); + REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); + } + + THEN("less than comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr < valid_addr, std::system_error); + } } } From c9d4b6243a0c4dc027931927b0d6221d11a06932 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 19 May 2025 08:10:36 +1000 Subject: [PATCH 5/7] Fix namespace in test --- src/util/network/sock_t.hpp | 1 - tests/tests/util/network/sock_t.cpp | 784 ++++++++++++++-------------- 2 files changed, 395 insertions(+), 390 deletions(-) diff --git a/src/util/network/sock_t.hpp b/src/util/network/sock_t.hpp index 1836f745..8b3a7a38 100644 --- a/src/util/network/sock_t.hpp +++ b/src/util/network/sock_t.hpp @@ -77,7 +77,6 @@ namespace util { && std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0; } - /** * Inequality comparison operator for sock_t. * diff --git a/tests/tests/util/network/sock_t.cpp b/tests/tests/util/network/sock_t.cpp index 90840da2..f3cef1cb 100644 --- a/tests/tests/util/network/sock_t.cpp +++ b/tests/tests/util/network/sock_t.cpp @@ -28,426 +28,432 @@ #include #include -using namespace NUClear::util::network; - -SCENARIO("Address families are handled correctly", "[sock_t]") { - GIVEN("An IPv4 address") { - sock_t addr{}; - addr.sock.sa_family = AF_INET; - addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr.ipv4.sin_port = htons(12345); - - THEN("its size should match sockaddr_in") { - REQUIRE(addr.size() == sizeof(sockaddr_in)); - } - - THEN("it should resolve to the correct IP and port") { - auto address_result = addr.address(true); // Use numeric host - auto& host = address_result.first; - auto& port = address_result.second; - REQUIRE(host == "192.168.1.1"); - REQUIRE(port == 12345); - } - } - - GIVEN("An IPv6 address") { - sock_t addr{}; - addr.sock.sa_family = AF_INET6; - // 2001:db8::1 - uint8_t ipv6_addr[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - addr.ipv6.sin6_port = htons(54321); - - THEN("its size should match sockaddr_in6") { - REQUIRE(addr.size() == sizeof(sockaddr_in6)); +namespace NUClear { +namespace util { + namespace network { + + SCENARIO("Address families are handled correctly", "[sock_t]") { + GIVEN("An IPv4 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET; + addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr.ipv4.sin_port = htons(12345); + + THEN("its size should match sockaddr_in") { + REQUIRE(addr.size() == sizeof(sockaddr_in)); + } + + THEN("it should resolve to the correct IP and port") { + auto address_result = addr.address(true); // Use numeric host + auto& host = address_result.first; + auto& port = address_result.second; + REQUIRE(host == "192.168.1.1"); + REQUIRE(port == 12345); + } + } + + GIVEN("An IPv6 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET6; + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + addr.ipv6.sin6_port = htons(54321); + + THEN("its size should match sockaddr_in6") { + REQUIRE(addr.size() == sizeof(sockaddr_in6)); + } + + THEN("it should resolve to the correct IP and port") { + auto address_result = addr.address(true); // Use numeric host + auto& host = address_result.first; + auto& port = address_result.second; + REQUIRE(host == "2001:db8::1"); + REQUIRE(port == 54321); + } + } + + GIVEN("An unknown address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unknown family + + THEN("attempting to get its size should throw") { + REQUIRE_THROWS_AS(addr.size(), std::system_error); + } + + THEN("attempting to resolve it should throw") { + REQUIRE_THROWS_AS(addr.address(), std::system_error); + } + } } - THEN("it should resolve to the correct IP and port") { - auto address_result = addr.address(true); // Use numeric host - auto& host = address_result.first; - auto& port = address_result.second; - REQUIRE(host == "2001:db8::1"); - REQUIRE(port == 54321); + SCENARIO("Addresses can be compared", "[sock_t]") { + GIVEN("Two identical IPv4 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + THEN("they should be equal") { + REQUIRE(addr1 == addr2); + REQUIRE_FALSE(addr1 != addr2); + } + } + + GIVEN("Two different IPv4 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two identical IPv6 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should be equal") { + REQUIRE(addr1 == addr2); + REQUIRE_FALSE(addr1 != addr2); + } + } + + GIVEN("Two different IPv6 addresses") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr1[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + // 2001:db8::2 + uint8_t ipv6_addr2[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("An IPv4 and an IPv6 address") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET6; + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } } - } - GIVEN("An unknown address family") { - sock_t addr{}; - addr.sock.sa_family = AF_UNSPEC; // Unknown family - - THEN("attempting to get its size should throw") { - REQUIRE_THROWS_AS(addr.size(), std::system_error); - } - - THEN("attempting to resolve it should throw") { - REQUIRE_THROWS_AS(addr.address(), std::system_error); - } - } -} + SCENARIO("Addresses can be ordered", "[sock_t]") { + GIVEN("Two IPv4 addresses with different IPs") { + sock_t addr1{}; + sock_t addr2{}; -SCENARIO("Addresses can be compared", "[sock_t]") { - GIVEN("Two identical IPv4 addresses") { - sock_t addr1{}; - sock_t addr2{}; + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(12345); - - THEN("they should be equal") { - REQUIRE(addr1 == addr2); - REQUIRE_FALSE(addr1 != addr2); - } - } - - GIVEN("Two different IPv4 addresses") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; - - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 - - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(12345); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } + } - GIVEN("Two identical IPv6 addresses") { - sock_t addr1{}; - sock_t addr2{}; + GIVEN("Two IPv4 addresses with same IP but different ports") { + sock_t addr1{}; + sock_t addr2{}; - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; - // 2001:db8::1 - uint8_t ipv6_addr[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr1.ipv6.sin6_port = htons(54321); - addr2.ipv6.sin6_port = htons(54321); + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(54321); - THEN("they should be equal") { - REQUIRE(addr1 == addr2); - REQUIRE_FALSE(addr1 != addr2); - } - } - - GIVEN("Two different IPv6 addresses") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr1[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - // 2001:db8::2 - uint8_t ipv6_addr2[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); - - addr1.ipv6.sin6_port = htons(54321); - addr2.ipv6.sin6_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } + } - GIVEN("An IPv4 and an IPv6 address") { - sock_t addr1{}; - sock_t addr2{}; + GIVEN("Two IPv6 addresses with different IPs") { + sock_t addr1{}; + sock_t addr2{}; - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET6; + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } -} + // 2001:db8::1 + uint8_t ipv6_addr1[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + // 2001:db8::2 + uint8_t ipv6_addr2[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); -SCENARIO("Addresses can be ordered", "[sock_t]") { - GIVEN("Two IPv4 addresses with different IPs") { - sock_t addr1{}; - sock_t addr2{}; + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } + } - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 + GIVEN("Two IPv6 addresses with same IP but different ports") { + sock_t addr1{}; + sock_t addr2{}; - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(12345); + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; - THEN("addr1 should be less than addr2") { - REQUIRE(addr1 < addr2); - } - } + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - GIVEN("Two IPv4 addresses with same IP but different ports") { - sock_t addr1{}; - sock_t addr2{}; + addr1.ipv6.sin6_port = htons(12345); + addr2.ipv6.sin6_port = htons(54321); - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; + THEN("addr1 should be less than addr2") { + REQUIRE(addr1 < addr2); + } + } - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + GIVEN("An IPv4 and an IPv6 address") { + sock_t addr1{}; + sock_t addr2{}; - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(54321); + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET6; - THEN("addr1 should be less than addr2") { - REQUIRE(addr1 < addr2); + THEN("IPv4 should be less than IPv6") { + REQUIRE(addr1 < addr2); + } + } } - } - - GIVEN("Two IPv6 addresses with different IPs") { - sock_t addr1{}; - sock_t addr2{}; - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr1[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - // 2001:db8::2 - uint8_t ipv6_addr2[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); - - addr1.ipv6.sin6_port = htons(54321); - addr2.ipv6.sin6_port = htons(54321); - - THEN("addr1 should be less than addr2") { - REQUIRE(addr1 < addr2); + SCENARIO("Address equality operators handle all conditions", "[sock_t]") { + GIVEN("Two IPv4 addresses with different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two IPv4 addresses with different IPs but same port") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(12345); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two IPv6 addresses with different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + + addr1.ipv6.sin6_port = htons(12345); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + + GIVEN("Two IPv6 addresses with different IPs but same port") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr1[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + // 2001:db8::2 + uint8_t ipv6_addr2[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); + + addr1.ipv6.sin6_port = htons(54321); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } } - } - - GIVEN("Two IPv6 addresses with same IP but different ports") { - sock_t addr1{}; - sock_t addr2{}; - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - - addr1.ipv6.sin6_port = htons(12345); - addr2.ipv6.sin6_port = htons(54321); - - THEN("addr1 should be less than addr2") { - REQUIRE(addr1 < addr2); + SCENARIO("Address resolution handles errors", "[sock_t]") { + GIVEN("An unsupported address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unsupported address family + + THEN("attempting to resolve it should throw") { + REQUIRE_THROWS_AS(addr.address(), std::system_error); + } + + THEN("equality comparison should throw") { + sock_t other{}; + other.sock.sa_family = AF_UNSPEC; + REQUIRE_THROWS_AS(addr == other, std::system_error); + REQUIRE_THROWS_AS(addr != other, std::system_error); + } + + THEN("less than comparison should throw") { + sock_t other{}; + other.sock.sa_family = AF_UNSPEC; + REQUIRE_THROWS_AS(addr < other, std::system_error); + } + } + + GIVEN("A socket with an unsupported address family compared with a valid socket") { + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + sock_t valid_addr{}; + valid_addr.sock.sa_family = AF_INET; + valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + valid_addr.ipv4.sin_port = htons(12345); + + THEN("equality comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); + REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); + } + + THEN("less than comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr < valid_addr, std::system_error); + } + } } - } - - GIVEN("An IPv4 and an IPv6 address") { - sock_t addr1{}; - sock_t addr2{}; - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET6; - - THEN("IPv4 should be less than IPv6") { - REQUIRE(addr1 < addr2); + SCENARIO("Socket addresses can be streamed", "[sock_t]") { + GIVEN("An IPv4 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET; + addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr.ipv4.sin_port = htons(12345); + + THEN("it should stream in the correct format") { + std::stringstream ss; + ss << addr; + REQUIRE(ss.str() == "192.168.1.1:12345"); + } + } + + GIVEN("An IPv6 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET6; + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + addr.ipv6.sin6_port = htons(54321); + + THEN("it should stream in the correct format") { + std::stringstream ss; + ss << addr; + REQUIRE(ss.str() == "2001:db8::1:54321"); + } + } + + GIVEN("An unsupported address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unsupported address family + + THEN("streaming should throw") { + std::stringstream ss; + REQUIRE_THROWS_AS(ss << addr, std::system_error); + } + } } - } -} - -SCENARIO("Address equality operators handle all conditions", "[sock_t]") { - GIVEN("Two IPv4 addresses with different ports") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; - - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - - GIVEN("Two IPv4 addresses with different IPs but same port") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; - - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 - - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(12345); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - - GIVEN("Two IPv6 addresses with different ports") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - - addr1.ipv6.sin6_port = htons(12345); - addr2.ipv6.sin6_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - - GIVEN("Two IPv6 addresses with different IPs but same port") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr1[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - // 2001:db8::2 - uint8_t ipv6_addr2[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); - - addr1.ipv6.sin6_port = htons(54321); - addr2.ipv6.sin6_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } -} - -SCENARIO("Address resolution handles errors", "[sock_t]") { - GIVEN("An unsupported address family") { - sock_t addr{}; - addr.sock.sa_family = AF_UNSPEC; // Unsupported address family - - THEN("attempting to resolve it should throw") { - REQUIRE_THROWS_AS(addr.address(), std::system_error); - } - - THEN("equality comparison should throw") { - sock_t other{}; - other.sock.sa_family = AF_UNSPEC; - REQUIRE_THROWS_AS(addr == other, std::system_error); - REQUIRE_THROWS_AS(addr != other, std::system_error); - } - - THEN("less than comparison should throw") { - sock_t other{}; - other.sock.sa_family = AF_UNSPEC; - REQUIRE_THROWS_AS(addr < other, std::system_error); - } - } - - GIVEN("A socket with an unsupported address family compared with a valid socket") { - sock_t invalid_addr{}; - invalid_addr.sock.sa_family = AF_UNSPEC; - - sock_t valid_addr{}; - valid_addr.sock.sa_family = AF_INET; - valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - valid_addr.ipv4.sin_port = htons(12345); - - THEN("equality comparison should throw") { - REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); - REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); - } - - THEN("less than comparison should throw") { - REQUIRE_THROWS_AS(invalid_addr < valid_addr, std::system_error); - } - } -} - -SCENARIO("Socket addresses can be streamed", "[sock_t]") { - GIVEN("An IPv4 address") { - sock_t addr{}; - addr.sock.sa_family = AF_INET; - addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr.ipv4.sin_port = htons(12345); - - THEN("it should stream in the correct format") { - std::stringstream ss; - ss << addr; - REQUIRE(ss.str() == "192.168.1.1:12345"); - } - } - - GIVEN("An IPv6 address") { - sock_t addr{}; - addr.sock.sa_family = AF_INET6; - // 2001:db8::1 - uint8_t ipv6_addr[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - addr.ipv6.sin6_port = htons(54321); - - THEN("it should stream in the correct format") { - std::stringstream ss; - ss << addr; - REQUIRE(ss.str() == "2001:db8::1:54321"); - } - } - - GIVEN("An unsupported address family") { - sock_t addr{}; - addr.sock.sa_family = AF_UNSPEC; // Unsupported address family - - THEN("streaming should throw") { - std::stringstream ss; - REQUIRE_THROWS_AS(ss << addr, std::system_error); - } - } -} + } // namespace network +} // namespace util +} // namespace NUClear From 713b5af2e0dc965d72e2b4bf893aa61706e27c1f Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 19 May 2025 08:19:00 +1000 Subject: [PATCH 6/7] Reorder/simplify --- tests/tests/util/network/sock_t.cpp | 227 +++++++++++----------------- 1 file changed, 92 insertions(+), 135 deletions(-) diff --git a/tests/tests/util/network/sock_t.cpp b/tests/tests/util/network/sock_t.cpp index f3cef1cb..75b252a2 100644 --- a/tests/tests/util/network/sock_t.cpp +++ b/tests/tests/util/network/sock_t.cpp @@ -32,16 +32,41 @@ namespace NUClear { namespace util { namespace network { - SCENARIO("Address families are handled correctly", "[sock_t]") { + SCENARIO("sock_t::size() returns the correct size for different address families", "[sock_t]") { GIVEN("An IPv4 address") { sock_t addr{}; - addr.sock.sa_family = AF_INET; - addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr.ipv4.sin_port = htons(12345); + addr.sock.sa_family = AF_INET; THEN("its size should match sockaddr_in") { REQUIRE(addr.size() == sizeof(sockaddr_in)); } + } + + GIVEN("An IPv6 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET6; + + THEN("its size should match sockaddr_in6") { + REQUIRE(addr.size() == sizeof(sockaddr_in6)); + } + } + + GIVEN("An unknown address family") { + sock_t addr{}; + addr.sock.sa_family = AF_UNSPEC; // Unknown family + + THEN("attempting to get its size should throw") { + REQUIRE_THROWS_AS(addr.size(), std::system_error); + } + } + } + + SCENARIO("sock_t::address() resolves addresses correctly", "[sock_t]") { + GIVEN("An IPv4 address") { + sock_t addr{}; + addr.sock.sa_family = AF_INET; + addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr.ipv4.sin_port = htons(12345); THEN("it should resolve to the correct IP and port") { auto address_result = addr.address(true); // Use numeric host @@ -61,10 +86,6 @@ namespace util { std::memcpy(&addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); addr.ipv6.sin6_port = htons(54321); - THEN("its size should match sockaddr_in6") { - REQUIRE(addr.size() == sizeof(sockaddr_in6)); - } - THEN("it should resolve to the correct IP and port") { auto address_result = addr.address(true); // Use numeric host auto& host = address_result.first; @@ -74,13 +95,9 @@ namespace util { } } - GIVEN("An unknown address family") { + GIVEN("An unsupported address family") { sock_t addr{}; - addr.sock.sa_family = AF_UNSPEC; // Unknown family - - THEN("attempting to get its size should throw") { - REQUIRE_THROWS_AS(addr.size(), std::system_error); - } + addr.sock.sa_family = AF_UNSPEC; // Unsupported address family THEN("attempting to resolve it should throw") { REQUIRE_THROWS_AS(addr.address(), std::system_error); @@ -88,7 +105,7 @@ namespace util { } } - SCENARIO("Addresses can be compared", "[sock_t]") { + SCENARIO("sock_t equality operators (== and !=) correctly compare addresses", "[sock_t]") { GIVEN("Two identical IPv4 addresses") { sock_t addr1{}; sock_t addr2{}; @@ -108,7 +125,7 @@ namespace util { } } - GIVEN("Two different IPv4 addresses") { + GIVEN("Two IPv4 addresses with different IPs") { sock_t addr1{}; sock_t addr2{}; @@ -127,6 +144,25 @@ namespace util { } } + GIVEN("Two IPv4 addresses with same IP but different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET; + addr2.sock.sa_family = AF_INET; + + addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + + addr1.ipv4.sin_port = htons(12345); + addr2.ipv4.sin_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + GIVEN("Two identical IPv6 addresses") { sock_t addr1{}; sock_t addr2{}; @@ -149,7 +185,7 @@ namespace util { } } - GIVEN("Two different IPv6 addresses") { + GIVEN("Two IPv6 addresses with different IPs") { sock_t addr1{}; sock_t addr2{}; @@ -174,6 +210,28 @@ namespace util { } } + GIVEN("Two IPv6 addresses with same IP but different ports") { + sock_t addr1{}; + sock_t addr2{}; + + addr1.sock.sa_family = AF_INET6; + addr2.sock.sa_family = AF_INET6; + + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + + addr1.ipv6.sin6_port = htons(12345); + addr2.ipv6.sin6_port = htons(54321); + + THEN("they should not be equal") { + REQUIRE_FALSE(addr1 == addr2); + REQUIRE(addr1 != addr2); + } + } + GIVEN("An IPv4 and an IPv6 address") { sock_t addr1{}; sock_t addr2{}; @@ -186,9 +244,24 @@ namespace util { REQUIRE(addr1 != addr2); } } + + GIVEN("A socket with an unsupported address family compared with a valid socket") { + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + sock_t valid_addr{}; + valid_addr.sock.sa_family = AF_INET; + valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 + valid_addr.ipv4.sin_port = htons(12345); + + THEN("equality comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); + REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); + } + } } - SCENARIO("Addresses can be ordered", "[sock_t]") { + SCENARIO("sock_t ordering operator (<) correctly orders addresses", "[sock_t]") { GIVEN("Two IPv4 addresses with different IPs") { sock_t addr1{}; sock_t addr2{}; @@ -281,117 +354,6 @@ namespace util { REQUIRE(addr1 < addr2); } } - } - - SCENARIO("Address equality operators handle all conditions", "[sock_t]") { - GIVEN("Two IPv4 addresses with different ports") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; - - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - - GIVEN("Two IPv4 addresses with different IPs but same port") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET; - addr2.sock.sa_family = AF_INET; - - addr1.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 - addr2.ipv4.sin_addr.s_addr = htonl(0xC0A80102); // 192.168.1.2 - - addr1.ipv4.sin_port = htons(12345); - addr2.ipv4.sin_port = htons(12345); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - - GIVEN("Two IPv6 addresses with different ports") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); - - addr1.ipv6.sin6_port = htons(12345); - addr2.ipv6.sin6_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - - GIVEN("Two IPv6 addresses with different IPs but same port") { - sock_t addr1{}; - sock_t addr2{}; - - addr1.sock.sa_family = AF_INET6; - addr2.sock.sa_family = AF_INET6; - - // 2001:db8::1 - uint8_t ipv6_addr1[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; - // 2001:db8::2 - uint8_t ipv6_addr2[16] = - {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}; - std::memcpy(&addr1.ipv6.sin6_addr, ipv6_addr1, sizeof(ipv6_addr1)); - std::memcpy(&addr2.ipv6.sin6_addr, ipv6_addr2, sizeof(ipv6_addr2)); - - addr1.ipv6.sin6_port = htons(54321); - addr2.ipv6.sin6_port = htons(54321); - - THEN("they should not be equal") { - REQUIRE_FALSE(addr1 == addr2); - REQUIRE(addr1 != addr2); - } - } - } - - SCENARIO("Address resolution handles errors", "[sock_t]") { - GIVEN("An unsupported address family") { - sock_t addr{}; - addr.sock.sa_family = AF_UNSPEC; // Unsupported address family - - THEN("attempting to resolve it should throw") { - REQUIRE_THROWS_AS(addr.address(), std::system_error); - } - - THEN("equality comparison should throw") { - sock_t other{}; - other.sock.sa_family = AF_UNSPEC; - REQUIRE_THROWS_AS(addr == other, std::system_error); - REQUIRE_THROWS_AS(addr != other, std::system_error); - } - - THEN("less than comparison should throw") { - sock_t other{}; - other.sock.sa_family = AF_UNSPEC; - REQUIRE_THROWS_AS(addr < other, std::system_error); - } - } GIVEN("A socket with an unsupported address family compared with a valid socket") { sock_t invalid_addr{}; @@ -402,18 +364,13 @@ namespace util { valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 valid_addr.ipv4.sin_port = htons(12345); - THEN("equality comparison should throw") { - REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); - REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); - } - THEN("less than comparison should throw") { REQUIRE_THROWS_AS(invalid_addr < valid_addr, std::system_error); } } } - SCENARIO("Socket addresses can be streamed", "[sock_t]") { + SCENARIO("sock_t stream operator (<<) correctly formats addresses", "[sock_t]") { GIVEN("An IPv4 address") { sock_t addr{}; addr.sock.sa_family = AF_INET; From be8dfb010f8e3c2ba137697c19c174793110c085 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 19 May 2025 09:30:12 +1000 Subject: [PATCH 7/7] Trying to improve coverage --- src/util/network/sock_t.cpp | 4 +- src/util/network/sock_t.hpp | 27 ++++--- tests/tests/util/network/sock_t.cpp | 117 ++++++++++++++++++++++++++-- 3 files changed, 129 insertions(+), 19 deletions(-) diff --git a/src/util/network/sock_t.cpp b/src/util/network/sock_t.cpp index be806915..d12c44d9 100644 --- a/src/util/network/sock_t.cpp +++ b/src/util/network/sock_t.cpp @@ -41,13 +41,15 @@ namespace util { std::array host{}; std::array service{}; + auto flags = NI_NUMERICSERV | (numeric ? NI_NUMERICHOST : 0); + if (::getnameinfo(&sock, size(), host.data(), static_cast(host.size()), service.data(), static_cast(service.size()), - NI_NUMERICSERV | (numeric ? NI_NUMERICHOST : 0)) + flags) != 0) { throw std::system_error( network_errno, diff --git a/src/util/network/sock_t.hpp b/src/util/network/sock_t.hpp index 8b3a7a38..25c33813 100644 --- a/src/util/network/sock_t.hpp +++ b/src/util/network/sock_t.hpp @@ -71,10 +71,13 @@ namespace util { if (a.sock.sa_family != b.sock.sa_family) { return false; } - return a.sock.sa_family == AF_INET - ? a.ipv4.sin_port == b.ipv4.sin_port && a.ipv4.sin_addr.s_addr == b.ipv4.sin_addr.s_addr - : a.ipv6.sin6_port == b.ipv6.sin6_port - && std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0; + + if (a.sock.sa_family == AF_INET) { + return a.ipv4.sin_port == b.ipv4.sin_port && a.ipv4.sin_addr.s_addr == b.ipv4.sin_addr.s_addr; + } + + return a.ipv6.sin6_port == b.ipv6.sin6_port + && std::memcmp(&a.ipv6.sin6_addr.s6_addr, &b.ipv6.sin6_addr.s6_addr, sizeof(in6_addr)) == 0; } /** @@ -106,13 +109,17 @@ namespace util { if (a.sock.sa_family != b.sock.sa_family) { return a.sock.sa_family < b.sock.sa_family; } + if (a.sock.sa_family == AF_INET) { + return std::forward_as_tuple(ntohl(a.ipv4.sin_addr.s_addr), ntohs(a.ipv4.sin_port)) + < std::forward_as_tuple(ntohl(b.ipv4.sin_addr.s_addr), ntohs(b.ipv4.sin_port)); + } + + auto cmp = std::memcmp(a.ipv6.sin6_addr.s6_addr, b.ipv6.sin6_addr.s6_addr, sizeof(a.ipv6.sin6_addr)); + if (cmp != 0) { + return cmp < 0; + } - return a.sock.sa_family == AF_INET - ? std::forward_as_tuple(a.ipv4.sin_addr.s_addr, ntohs(a.ipv4.sin_port)) - < std::forward_as_tuple(b.ipv4.sin_addr.s_addr, ntohs(b.ipv4.sin_port)) - : std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) < 0 - || (std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr)) == 0 - && ntohs(a.ipv6.sin6_port) < ntohs(b.ipv6.sin6_port)); + return ntohs(a.ipv6.sin6_port) < ntohs(b.ipv6.sin6_port); } /** diff --git a/tests/tests/util/network/sock_t.cpp b/tests/tests/util/network/sock_t.cpp index 75b252a2..e7a0fdfd 100644 --- a/tests/tests/util/network/sock_t.cpp +++ b/tests/tests/util/network/sock_t.cpp @@ -75,6 +75,10 @@ namespace util { REQUIRE(host == "192.168.1.1"); REQUIRE(port == 12345); } + + THEN("it should also resolve with non-numeric flag") { + REQUIRE_NOTHROW(addr.address(false)); + } } GIVEN("An IPv6 address") { @@ -93,6 +97,10 @@ namespace util { REQUIRE(host == "2001:db8::1"); REQUIRE(port == 54321); } + + THEN("it should also resolve with non-numeric flag") { + REQUIRE_NOTHROW(addr.address(false)); + } } GIVEN("An unsupported address family") { @@ -103,9 +111,34 @@ namespace util { REQUIRE_THROWS_AS(addr.address(), std::system_error); } } + + // This test is attempting to trigger the getnameinfo error path + GIVEN("A socket that will cause getnameinfo to fail") { + sock_t addr{}; + // Explicitly using a custom address family here that is supported by our code + // but might cause getnameinfo to fail + addr.sock.sa_family = AF_INET; + // Invalid address/port configuration to maximize chances of getnameinfo failing + addr.ipv4.sin_addr.s_addr = INADDR_NONE; + addr.ipv4.sin_port = 0; + + THEN("it might throw if getnameinfo fails") { + // We can't guarantee this will fail on all systems, + // so we just try and if it succeeds that's fine too + try { + addr.address(true); + } + catch (const std::system_error& e) { + // Verify we're getting the expected error message format + REQUIRE(std::string(e.what()).find("Cannot get address for socket address family") + != std::string::npos); + } + } + } } SCENARIO("sock_t equality operators (== and !=) correctly compare addresses", "[sock_t]") { + // Test cases for valid addresses GIVEN("Two identical IPv4 addresses") { sock_t addr1{}; sock_t addr2{}; @@ -245,16 +278,51 @@ namespace util { } } - GIVEN("A socket with an unsupported address family compared with a valid socket") { - sock_t invalid_addr{}; - invalid_addr.sock.sa_family = AF_UNSPEC; + GIVEN("Two sockets with the same unsupported address family") { + sock_t invalid_addr1{}; + invalid_addr1.sock.sa_family = AF_UNSPEC; + + sock_t invalid_addr2{}; + invalid_addr2.sock.sa_family = AF_UNSPEC; + THEN("equality comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr1 == invalid_addr2, std::system_error); + REQUIRE_THROWS_AS(invalid_addr1 != invalid_addr2, std::system_error); + } + } + + GIVEN("A valid IPv4 address and an invalid address") { sock_t valid_addr{}; valid_addr.sock.sa_family = AF_INET; valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 valid_addr.ipv4.sin_port = htons(12345); - THEN("equality comparison should throw") { + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + THEN("equality comparison should throw with either argument order") { + REQUIRE_THROWS_AS(valid_addr == invalid_addr, std::system_error); + REQUIRE_THROWS_AS(valid_addr != invalid_addr, std::system_error); + REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); + REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); + } + } + + GIVEN("A valid IPv6 address and an invalid address") { + sock_t valid_addr{}; + valid_addr.sock.sa_family = AF_INET6; + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&valid_addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + valid_addr.ipv6.sin6_port = htons(54321); + + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + THEN("equality comparison should throw with either argument order") { + REQUIRE_THROWS_AS(valid_addr == invalid_addr, std::system_error); + REQUIRE_THROWS_AS(valid_addr != invalid_addr, std::system_error); REQUIRE_THROWS_AS(invalid_addr == valid_addr, std::system_error); REQUIRE_THROWS_AS(invalid_addr != valid_addr, std::system_error); } @@ -262,6 +330,7 @@ namespace util { } SCENARIO("sock_t ordering operator (<) correctly orders addresses", "[sock_t]") { + // Test cases for valid addresses GIVEN("Two IPv4 addresses with different IPs") { sock_t addr1{}; sock_t addr2{}; @@ -355,16 +424,48 @@ namespace util { } } - GIVEN("A socket with an unsupported address family compared with a valid socket") { - sock_t invalid_addr{}; - invalid_addr.sock.sa_family = AF_UNSPEC; + GIVEN("Two sockets with the same unsupported address family") { + sock_t invalid_addr1{}; + invalid_addr1.sock.sa_family = AF_UNSPEC; + + sock_t invalid_addr2{}; + invalid_addr2.sock.sa_family = AF_UNSPEC; + + THEN("less than comparison should throw") { + REQUIRE_THROWS_AS(invalid_addr1 < invalid_addr2, std::system_error); + } + } + + GIVEN("A valid IPv4 address and an invalid address") { sock_t valid_addr{}; valid_addr.sock.sa_family = AF_INET; valid_addr.ipv4.sin_addr.s_addr = htonl(0xC0A80101); // 192.168.1.1 valid_addr.ipv4.sin_port = htons(12345); - THEN("less than comparison should throw") { + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + THEN("less than comparison should throw with either argument order") { + REQUIRE_THROWS_AS(valid_addr < invalid_addr, std::system_error); + REQUIRE_THROWS_AS(invalid_addr < valid_addr, std::system_error); + } + } + + GIVEN("A valid IPv6 address and an invalid address") { + sock_t valid_addr{}; + valid_addr.sock.sa_family = AF_INET6; + // 2001:db8::1 + uint8_t ipv6_addr[16] = + {0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; + std::memcpy(&valid_addr.ipv6.sin6_addr, ipv6_addr, sizeof(ipv6_addr)); + valid_addr.ipv6.sin6_port = htons(54321); + + sock_t invalid_addr{}; + invalid_addr.sock.sa_family = AF_UNSPEC; + + THEN("less than comparison should throw with either argument order") { + REQUIRE_THROWS_AS(valid_addr < invalid_addr, std::system_error); REQUIRE_THROWS_AS(invalid_addr < valid_addr, std::system_error); } }