From f0c9e68080432c1ab11b14e571b8dfb7cfe727f8 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 6 Oct 2021 22:26:18 +0200 Subject: [PATCH 1/2] p2p, refactor: tidy up LookupSubNet() - consistent param naming between function declaration and definition - brackets, param naming and localvar naming per current standards in doc/developer-notes.md - update/improve doxygen documentation in the declaration - improve comments and other localvar names - constness - named args --- src/netbase.cpp | 52 +++++++++++++++++++++++-------------------------- src/netbase.h | 13 +++++++------ 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index 3cb12f1abc..5d7d45f2ef 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -676,40 +676,36 @@ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uin return true; } -bool LookupSubNet(const std::string& strSubnet, CSubNet& ret, DNSLookupFn dns_lookup_function) +bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out, DNSLookupFn dns_lookup_function) { - if (!ValidAsCString(strSubnet)) { + if (!ValidAsCString(subnet_str)) { return false; } - size_t slash = strSubnet.find_last_of('/'); - CNetAddr network; - std::string strAddress = strSubnet.substr(0, slash); - if (LookupHost(strAddress, network, false, dns_lookup_function)) - { - if (slash != strSubnet.npos) - { - std::string strNetmask = strSubnet.substr(slash + 1); - uint8_t n; - if (ParseUInt8(strNetmask, &n)) { - // If valid number, assume CIDR variable-length subnet masking - ret = CSubNet(network, n); - return ret.IsValid(); - } - else // If not a valid number, try full netmask syntax - { - CNetAddr netmask; - // Never allow lookup for netmask - if (LookupHost(strNetmask, netmask, false, dns_lookup_function)) { - ret = CSubNet(network, netmask); - return ret.IsValid(); + const size_t slash_pos{subnet_str.find_last_of('/')}; + const std::string str_addr{subnet_str.substr(0, slash_pos)}; + CNetAddr addr; + + if (LookupHost(str_addr, addr, /*fAllowLookup=*/false, dns_lookup_function)) { + if (slash_pos != subnet_str.npos) { + const std::string netmask_str{subnet_str.substr(slash_pos + 1)}; + uint8_t netmask; + if (ParseUInt8(netmask_str, &netmask)) { + // Valid number; assume CIDR variable-length subnet masking. + subnet_out = CSubNet{addr, netmask}; + return subnet_out.IsValid(); + } else { + // Invalid number; try full netmask syntax. Never allow lookup for netmask. + CNetAddr full_netmask; + if (LookupHost(netmask_str, full_netmask, /*fAllowLookup=*/false, dns_lookup_function)) { + subnet_out = CSubNet{addr, full_netmask}; + return subnet_out.IsValid(); } } - } - else // Single IP subnet (/32 or /128) - { - ret = CSubNet(network); - return ret.IsValid(); + } else { + // Single IP subnet (/32 or /128). + subnet_out = CSubNet{addr}; + return subnet_out.IsValid(); } } return false; diff --git a/src/netbase.h b/src/netbase.h index 6a87c338a0..a730626a50 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -169,13 +169,14 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLoo * Parse and resolve a specified subnet string into the appropriate internal * representation. * - * @param strSubnet A string representation of a subnet of the form `network - * address [ "/", ( CIDR-style suffix | netmask ) ]`(e.g. - * `2001:db8::/32`, `192.0.2.0/255.255.255.0`, or `8.8.8.8`). - * - * @returns Whether the operation succeeded or not. + * @param[in] subnet_str A string representation of a subnet of the form + * `network address [ "/", ( CIDR-style suffix | netmask ) ]` + * e.g. "2001:db8::/32", "192.0.2.0/255.255.255.0" or "8.8.8.8". + * @param[out] subnet_out Internal subnet representation, if parsable/resolvable + * from `subnet_str`. + * @returns whether the operation succeeded or not. */ -bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet, DNSLookupFn dns_lookup_function = g_dns_lookup); +bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Create a TCP socket in the given address family. From c44c20108f7b7314f59f034110789385a24db280 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 7 Dec 2021 12:56:02 +0100 Subject: [PATCH 2/2] p2p, refactor: drop unused DNSLookupFn param in LookupSubnet() --- src/netbase.cpp | 6 +++--- src/netbase.h | 2 +- src/test/fuzz/netbase_dns_lookup.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index 5d7d45f2ef..0b68c0d041 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -676,7 +676,7 @@ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uin return true; } -bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out, DNSLookupFn dns_lookup_function) +bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) { if (!ValidAsCString(subnet_str)) { return false; @@ -686,7 +686,7 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out, DNSLookupF const std::string str_addr{subnet_str.substr(0, slash_pos)}; CNetAddr addr; - if (LookupHost(str_addr, addr, /*fAllowLookup=*/false, dns_lookup_function)) { + if (LookupHost(str_addr, addr, /*fAllowLookup=*/false)) { if (slash_pos != subnet_str.npos) { const std::string netmask_str{subnet_str.substr(slash_pos + 1)}; uint8_t netmask; @@ -697,7 +697,7 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out, DNSLookupF } else { // Invalid number; try full netmask syntax. Never allow lookup for netmask. CNetAddr full_netmask; - if (LookupHost(netmask_str, full_netmask, /*fAllowLookup=*/false, dns_lookup_function)) { + if (LookupHost(netmask_str, full_netmask, /*fAllowLookup=*/false)) { subnet_out = CSubNet{addr, full_netmask}; return subnet_out.IsValid(); } diff --git a/src/netbase.h b/src/netbase.h index a730626a50..f3d8f15788 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -176,7 +176,7 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLoo * from `subnet_str`. * @returns whether the operation succeeded or not. */ -bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out, DNSLookupFn dns_lookup_function = g_dns_lookup); +bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out); /** * Create a TCP socket in the given address family. diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp index d01d413cff..31ea31744a 100644 --- a/src/test/fuzz/netbase_dns_lookup.cpp +++ b/src/test/fuzz/netbase_dns_lookup.cpp @@ -64,7 +64,7 @@ FUZZ_TARGET(netbase_dns_lookup) } { CSubNet resolved_subnet; - if (LookupSubNet(name, resolved_subnet, fuzzed_dns_lookup_function)) { + if (LookupSubNet(name, resolved_subnet)) { assert(resolved_subnet.IsValid()); } }