From 89567d3b313c8d34d4ee075a17bd1a70d523b9bc Mon Sep 17 00:00:00 2001 From: Davide Pesavento Date: Sat, 19 Nov 2016 16:39:45 +0100 Subject: [PATCH] core: Network: misc code cleanups Change-Id: Idc37b5f834220e8e980958953785806687435f36 Refs: #3858 --- core/common.hpp | 1 - core/logger-factory.cpp | 6 +- core/network-interface-predicate.cpp | 7 +- core/network.cpp | 161 +++++++++++++-------------- core/network.hpp | 32 ++---- tests/core/logger.t.cpp | 2 +- tests/core/network.t.cpp | 11 +- 7 files changed, 106 insertions(+), 114 deletions(-) diff --git a/core/common.hpp b/core/common.hpp index 9f4fcec2..437d3556 100644 --- a/core/common.hpp +++ b/core/common.hpp @@ -65,7 +65,6 @@ #include #include -#include #include #include #include diff --git a/core/logger-factory.cpp b/core/logger-factory.cpp index 36ce5b31..3c419f16 100644 --- a/core/logger-factory.cpp +++ b/core/logger-factory.cpp @@ -24,7 +24,10 @@ */ #include "logger-factory.hpp" + #include + +#include #include #ifdef HAVE_CUSTOM_LOGGER @@ -73,8 +76,7 @@ LoggerFactory::setConfigFile(ConfigFile& config) LogLevel LoggerFactory::parseLevel(const std::string& level) { - std::string upperLevel = level; - boost::to_upper(upperLevel); + std::string upperLevel = boost::to_upper_copy(level); // std::cerr << "parsing level: " << upperLevel << std::endl;; // std::cerr << "# levels: " << m_levelNames.size() << std::endl; diff --git a/core/network-interface-predicate.cpp b/core/network-interface-predicate.cpp index abb00293..cf5fcd67 100644 --- a/core/network-interface-predicate.cpp +++ b/core/network-interface-predicate.cpp @@ -97,13 +97,12 @@ NetworkInterfacePredicate::parseBlacklist(const boost::property_tree::ptree& lis static bool doesMatchRule(const NetworkInterfaceInfo& nic, const std::string& rule) { - // if / is in rule, this is a subnet, check if IP in subnet - - if (boost::contains(rule, "/")) { + // if '/' is in rule, this is a subnet, check if IP in subnet + if (rule.find('/') != std::string::npos) { Network n = boost::lexical_cast(rule); for (const auto& addr : nic.ipv4Addresses) { if (n.doesContain(addr)) { - return true; + return true; } } } diff --git a/core/network.cpp b/core/network.cpp index 465e2469..be7d9c67 100644 --- a/core/network.cpp +++ b/core/network.cpp @@ -25,19 +25,26 @@ #include "network.hpp" +#include +#include +#include + namespace nfd { -void -Network::print(std::ostream& os) const +Network::Network() = default; + +Network::Network(const boost::asio::ip::address& minAddress, + const boost::asio::ip::address& maxAddress) + : m_minAddress(minAddress) + , m_maxAddress(maxAddress) { - os << m_minAddress << " <-> " << m_maxAddress; } const Network& Network::getMaxRangeV4() { using boost::asio::ip::address_v4; - static Network range = Network(address_v4(0), address_v4(0xFFFFFFFF)); + static Network range{address_v4{}, address_v4{0xffffffff}}; return range; } @@ -47,111 +54,103 @@ Network::getMaxRangeV6() using boost::asio::ip::address_v6; static address_v6::bytes_type maxV6 = {{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}}; - static Network range = Network(address_v6(), address_v6(maxV6)); + static Network range{address_v6{}, address_v6{maxV6}}; return range; } bool Network::isValidCidr(const std::string& cidr) { - std::vector splitCidr; - boost::split(splitCidr, cidr, boost::is_any_of("/")); - if (splitCidr.size() != 2) { - return false; - } + std::vector splitCidr; + boost::split(splitCidr, cidr, boost::is_any_of("/")); + if (splitCidr.size() != 2) { + return false; + } - auto network = splitCidr[0]; - auto mask = splitCidr[1]; - auto netmask = 0; - if (mask.length() <= 0) { - return false; - } - if (!std::all_of(mask.begin(), mask.end(), ::isdigit)) { - return false; - } + boost::system::error_code invalidIp; + boost::asio::ip::address_v4::from_string(splitCidr[0], invalidIp); + if (invalidIp) { + return false; + } - netmask = boost::lexical_cast(splitCidr[1]); - boost::system::error_code invalidIP; - boost::asio::ip::address_v4::from_string(network, invalidIP); - if (invalidIP || netmask < 0 || netmask > 32) { - return false; - } - return true; + auto prefixLenStr = splitCidr[1]; + if (prefixLenStr.length() <= 0 || + !std::all_of(prefixLenStr.begin(), prefixLenStr.end(), ::isdigit)) { + return false; + } + int prefixLen = boost::lexical_cast(prefixLenStr); + if (prefixLen < 0 || prefixLen > 32) { + return false; + } + + return true; } std::ostream& operator<<(std::ostream& os, const Network& network) { - network.print(os); - return os; + return os << network.m_minAddress << " <-> " << network.m_maxAddress; } std::istream& operator>>(std::istream& is, Network& network) { - using namespace boost::asio; + namespace ip = boost::asio::ip; std::string networkStr; is >> networkStr; size_t position = networkStr.find('/'); - if (position == std::string::npos) - { - network.m_minAddress = ip::address::from_string(networkStr); - network.m_maxAddress = ip::address::from_string(networkStr); + if (position == std::string::npos) { + network.m_minAddress = ip::address::from_string(networkStr); + network.m_maxAddress = ip::address::from_string(networkStr); + } + else { + ip::address address = ip::address::from_string(networkStr.substr(0, position)); + size_t mask = boost::lexical_cast(networkStr.substr(position+1)); + + if (address.is_v4()) { + ip::address_v4::bytes_type maskBytes = boost::initialized_value; + for (size_t i = 0; i < mask; i++) { + size_t byteId = i / 8; + size_t bitIndex = 7 - i % 8; + maskBytes[byteId] |= (1 << bitIndex); + } + + ip::address_v4::bytes_type addressBytes = address.to_v4().to_bytes(); + ip::address_v4::bytes_type min; + ip::address_v4::bytes_type max; + + for (size_t i = 0; i < addressBytes.size(); i++) { + min[i] = addressBytes[i] & maskBytes[i]; + max[i] = addressBytes[i] | ~(maskBytes[i]); + } + + network.m_minAddress = ip::address_v4(min); + network.m_maxAddress = ip::address_v4(max); } - else - { - ip::address address = ip::address::from_string(networkStr.substr(0, position)); - size_t mask = boost::lexical_cast(networkStr.substr(position+1)); + else { + ip::address_v6::bytes_type maskBytes = boost::initialized_value; + for (size_t i = 0; i < mask; i++) { + size_t byteId = i / 8; + size_t bitIndex = 7 - i % 8; + maskBytes[byteId] |= (1 << bitIndex); + } - if (address.is_v4()) - { - ip::address_v4::bytes_type maskBytes = boost::initialized_value; - for (size_t i = 0; i < mask; i++) - { - size_t byteId = i / 8; - size_t bitIndex = 7 - i % 8; - maskBytes[byteId] |= (1 << bitIndex); - } + ip::address_v6::bytes_type addressBytes = address.to_v6().to_bytes(); + ip::address_v6::bytes_type min; + ip::address_v6::bytes_type max; - ip::address_v4::bytes_type addressBytes = address.to_v4().to_bytes(); - ip::address_v4::bytes_type min; - ip::address_v4::bytes_type max; + for (size_t i = 0; i < addressBytes.size(); i++) { + min[i] = addressBytes[i] & maskBytes[i]; + max[i] = addressBytes[i] | ~(maskBytes[i]); + } - for (size_t i = 0; i < addressBytes.size(); i++) - { - min[i] = addressBytes[i] & maskBytes[i]; - max[i] = addressBytes[i] | ~(maskBytes[i]); - } - - network.m_minAddress = ip::address_v4(min); - network.m_maxAddress = ip::address_v4(max); - } - else - { - ip::address_v6::bytes_type maskBytes = boost::initialized_value; - for (size_t i = 0; i < mask; i++) - { - size_t byteId = i / 8; - size_t bitIndex = 7 - i % 8; - maskBytes[byteId] |= (1 << bitIndex); - } - - ip::address_v6::bytes_type addressBytes = address.to_v6().to_bytes(); - ip::address_v6::bytes_type min; - ip::address_v6::bytes_type max; - - for (size_t i = 0; i < addressBytes.size(); i++) - { - min[i] = addressBytes[i] & maskBytes[i]; - max[i] = addressBytes[i] | ~(maskBytes[i]); - } - - network.m_minAddress = ip::address_v6(min); - network.m_maxAddress = ip::address_v6(max); - } + network.m_minAddress = ip::address_v6(min); + network.m_maxAddress = ip::address_v6(max); } + } + return is; } diff --git a/core/network.hpp b/core/network.hpp index 29493241..1770582c 100644 --- a/core/network.hpp +++ b/core/network.hpp @@ -26,45 +26,33 @@ #ifndef NFD_CORE_NETWORK_HPP #define NFD_CORE_NETWORK_HPP -#include -#include -#include -#include +#include "common.hpp" namespace nfd { class Network { public: - Network() - { - } + Network(); Network(const boost::asio::ip::address& minAddress, - const boost::asio::ip::address& maxAddress) - : m_minAddress(minAddress) - , m_maxAddress(maxAddress) - { - } - - void - print(std::ostream& os) const; + const boost::asio::ip::address& maxAddress); bool doesContain(const boost::asio::ip::address& address) const { - return (m_minAddress <= address && address <= m_maxAddress); + return m_minAddress <= address && address <= m_maxAddress; } - static bool - isValidCidr(const std::string& cidr); - static const Network& getMaxRangeV4(); static const Network& getMaxRangeV6(); + static bool + isValidCidr(const std::string& cidr); + bool operator==(const Network& rhs) const { @@ -81,11 +69,11 @@ private: boost::asio::ip::address m_minAddress; boost::asio::ip::address m_maxAddress; - friend std::istream& - operator>>(std::istream& is, Network& network); - friend std::ostream& operator<<(std::ostream& os, const Network& network); + + friend std::istream& + operator>>(std::istream& is, Network& network); }; std::ostream& diff --git a/tests/core/logger.t.cpp b/tests/core/logger.t.cpp index 07373e52..11dcfa41 100644 --- a/tests/core/logger.t.cpp +++ b/tests/core/logger.t.cpp @@ -27,8 +27,8 @@ #include "tests/test-common.hpp" -#include #include +#include namespace nfd { namespace tests { diff --git a/tests/core/network.t.cpp b/tests/core/network.t.cpp index a7af98f0..7f9bcd9d 100644 --- a/tests/core/network.t.cpp +++ b/tests/core/network.t.cpp @@ -147,15 +147,20 @@ BOOST_AUTO_TEST_CASE(Comparisons) BOOST_AUTO_TEST_CASE(IsValidCidr) { BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/24"), true); + BOOST_CHECK_EQUAL(Network::isValidCidr("192.1.2.3/32"), true); + BOOST_CHECK_EQUAL(Network::isValidCidr("0.0.0.0/0"), true); BOOST_CHECK_EQUAL(Network::isValidCidr(""), false); + BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/24/8"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("/192.0.0.0/24"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/+24"), false); - BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/*24"), false); + BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/-24"), false); + BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/ 24"), false); + BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/24a"), false); + BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/0x42"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/24.42"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/foo"), false); - BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/24/23"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/33"), false); - BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/-24"), false); + //BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/999999999999999"), false); // #3858 BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0/"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("192.0.0.0"), false); BOOST_CHECK_EQUAL(Network::isValidCidr("foo/4"), false);