From 3ebedd0c5f5505551698657c5eca8cffc1cad2bc Mon Sep 17 00:00:00 2001 From: Lukas Werling Date: Tue, 28 Feb 2017 22:15:29 +0100 Subject: [PATCH] Rank private/UL addresses lower than global ones Also adds some tests. Yay tests! --- src/network/C4NetIO.cpp | 19 +++++++++ src/network/C4NetIO.h | 3 +- src/network/C4Network2.cpp | 10 +++-- tests/C4NetIOTest.cpp | 85 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 tests/C4NetIOTest.cpp diff --git a/src/network/C4NetIO.cpp b/src/network/C4NetIO.cpp index 5ade509bd..2c92b1364 100644 --- a/src/network/C4NetIO.cpp +++ b/src/network/C4NetIO.cpp @@ -273,6 +273,25 @@ bool C4NetIO::HostAddress::IsLocal() const return false; } +bool C4NetIO::HostAddress::IsPrivate() const +{ + // IPv6 unique local address + if (gen.sa_family == AF_INET6) + return (v6.sin6_addr.s6_addr[0] & 0xfe) == 0xfc; + if (gen.sa_family == AF_INET) + { + uint32_t addr = ntohl(v4.sin_addr.s_addr); + uint32_t s = (addr >> 16) & 0xff; + switch (addr >> 24) + { + case 10: return true; + case 172: return s >= 16 && s <= 31; + case 192: return s == 168; + } + } + return false; +} + void C4NetIO::HostAddress::SetScopeId(int scopeId) { if (gen.sa_family != AF_INET6) return; diff --git a/src/network/C4NetIO.h b/src/network/C4NetIO.h index d8d580be2..ad6f85052 100644 --- a/src/network/C4NetIO.h +++ b/src/network/C4NetIO.h @@ -113,7 +113,8 @@ public: bool IsNull() const; bool IsMulticast() const; bool IsLoopback() const; - bool IsLocal() const; + bool IsLocal() const; // IPv6 link-local address + bool IsPrivate() const; // IPv6 ULA or IPv4 private address range // bool IsBroadcast() const; StdStrBuf ToString(int flags = 0) const; diff --git a/src/network/C4Network2.cpp b/src/network/C4Network2.cpp index 69d5a2c80..c1b31ea86 100644 --- a/src/network/C4Network2.cpp +++ b/src/network/C4Network2.cpp @@ -217,7 +217,7 @@ static void SortAddresses(std::vector& addrs) bool haveIPv6 = false; for (auto& addr : localAddrs) { - if (addr.GetFamily() == C4NetIO::HostAddress::IPv6 && !addr.IsLocal()) + if (addr.GetFamily() == C4NetIO::HostAddress::IPv6 && !addr.IsLocal() && !addr.IsPrivate()) { haveIPv6 = true; break; @@ -235,13 +235,17 @@ static void SortAddresses(std::vector& addrs) case C4NetIO::HostAddress::IPv6: if (addr.IsLocal()) rank = 100; + else if (addr.IsPrivate()) + rank = 150; else if (haveIPv6) // TODO: Rank public IPv6 addresses by longest matching prefix with local addresses. rank = 300; break; case C4NetIO::HostAddress::IPv4: - // TODO: Maybe rank IPv4 addresses from private address ranges differently. - rank = 200; + if (addr.IsPrivate()) + rank = 150; + else + rank = 200; break; default: assert(!"Unexpected address family"); diff --git a/tests/C4NetIOTest.cpp b/tests/C4NetIOTest.cpp new file mode 100644 index 000000000..077cab2b0 --- /dev/null +++ b/tests/C4NetIOTest.cpp @@ -0,0 +1,85 @@ +/* + * OpenClonk, http://www.openclonk.org + * + * Copyright (c) 2017, The OpenClonk Team and contributors + * + * Distributed under the terms of the ISC license; see accompanying file + * "COPYING" for details. + * + * "Clonk" is a registered trademark of Matthes Bender, used with permission. + * See accompanying file "TRADEMARK" for details. + * + * To redistribute this file separately, substitute the full license texts + * for the above references. + */ + +#include +#include "network/C4NetIO.h" + +#include + +class C4NetIOTest : public ::testing::Test +{ +protected: + C4NetIOTest() + { +#ifdef HAVE_WINSOCK + AcquireWinSock(); +#endif + } + + ~C4NetIOTest() + { +#ifdef HAVE_WINSOCK + ReleaseWinSock(); +#endif + } +}; + +TEST_F(C4NetIOTest, HostAddressCategories) +{ + static struct TestAddr + { + const char *addr; + bool null, multicast, loopback, linklocal, priv; + } addrs[] = + { + // null mc loopb ll priv + {"0.0.0.0", true, false, false, false, false}, + {"192.168.0.1", false, false, false, false, true}, + {"10.168.0.1", false, false, false, false, true}, + {"172.16.10.1", false, false, false, false, true}, + {"127.0.0.1", false, false, true, false, false}, + {"239.1.1.1", false, true, false, false, false}, + + // null mc loopb ll priv + {"::", true, false, false, false, false}, + {"::1", false, false, true, false, false}, + {"ff02::1", false, true, false, false, false}, + {"ff15::1234", false, true, false, false, false}, + {"fe80::1234:abcd:def:1234", false, false, false, true, false}, + {"fe80::1234:abcd:def:1234", false, false, false, true, false}, + {"fd12::1234:abcd:def:1234", false, false, false, false, true}, + {"fc12::1234:abcd:def:1234", false, false, false, false, true}, + + {nullptr, false, false, false, false, false}, + }; + + for (auto t = addrs; t->addr; t++) + { + // TODO: While this produces better error messages than EXPECT_EQ, failures are still super confusing. + auto check = [&](bool a, bool b) + { + if (a == b) + return ::testing::AssertionSuccess(); + else + return ::testing::AssertionFailure() << "addr = " << t->addr << " expected: " << b; + }; + C4NetIO::HostAddress addr(StdStrBuf(t->addr)); + EXPECT_TRUE(check(addr.IsNull(), t->null)); + EXPECT_TRUE(check(addr.IsMulticast(), t->multicast)); + EXPECT_TRUE(check(addr.IsLoopback(), t->loopback)); + EXPECT_TRUE(check(addr.IsLocal(), t->linklocal)); + EXPECT_TRUE(check(addr.IsPrivate(), t->priv)); + } +}