diff options
author | Andrew Chow <github@achow101.com> | 2023-05-30 11:29:11 -0400 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2023-05-30 11:39:59 -0400 |
commit | 71300489af362c3fed4736de6bffab4d758b6a84 (patch) | |
tree | 4295c0cd06e8d92e2742ac891ee6a8c4c3e664f2 | |
parent | 05ec664632adb6d5aca6ab3e6d944368e1164b49 (diff) | |
parent | 5c832c3820253affc65c0ed490e26e5b0a4d5c9b (diff) | |
download | bitcoin-71300489af362c3fed4736de6bffab4d758b6a84.tar.xz |
Merge bitcoin/bitcoin#26261: p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`
5c832c3820253affc65c0ed490e26e5b0a4d5c9b p2p, refactor: return `std::optional<CNetAddr>` in `LookupHost` (brunoerg)
34bcdfc6a65de906c65edccdd96fe15219081cd2 p2p, refactor: return vector/optional<CService> in `Lookup` (brunoerg)
7799eb125b7a1146f8251be5d26df574236212a9 p2p, refactor: return `std::vector<CNetAddr>` in `LookupHost` (brunoerg)
5c1774a563dcc237a88df69569cd94fe81e908f7 p2p, refactor: return `std::vector<CNetAddr>` in `LookupIntern` (brunoerg)
Pull request description:
Continuation of #26078.
To improve readability instead of returning a bool and passing stuff by reference, this PR changes:
- `LookupHost` to return `std::vector<CNetAddr>`
- `LookupHost` to return `std::optional<CNetAddr>`
- `Lookup` to return `std::vector<CService>`
- `Lookup` to return `std::optional<CService>`.
- `LookupIntern` to return `std::vector<CNetAddr>`
As discussed in #26078, it would be better to avoid using `optional` in some cases, but for specific `Lookup` and `LookupHost` functions it's necessary to use `optional` to verify if they were able to catch some data from their overloaded function.
ACKs for top commit:
achow101:
ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b
stickies-v:
re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b - just addressing two nits, no other changes
theStack:
re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b
Tree-SHA512: ea346fdc54463999646269bd600cd4a1590ef958001d2f0fc2be608ca51e1b4365efccca76dd4972b023e12fcc6e67d226608b0df7beb901bdeadd19948df840
-rw-r--r-- | src/bench/addrman.cpp | 20 | ||||
-rw-r--r-- | src/httpserver.cpp | 12 | ||||
-rw-r--r-- | src/init.cpp | 40 | ||||
-rw-r--r-- | src/mapport.cpp | 8 | ||||
-rw-r--r-- | src/net.cpp | 31 | ||||
-rw-r--r-- | src/net_permissions.cpp | 8 | ||||
-rw-r--r-- | src/netbase.cpp | 98 | ||||
-rw-r--r-- | src/netbase.h | 31 | ||||
-rw-r--r-- | src/qt/optionsdialog.cpp | 5 | ||||
-rw-r--r-- | src/rpc/net.cpp | 13 | ||||
-rw-r--r-- | src/test/addrman_tests.cpp | 49 | ||||
-rw-r--r-- | src/test/fuzz/netbase_dns_lookup.cpp | 28 | ||||
-rw-r--r-- | src/test/net_tests.cpp | 29 | ||||
-rw-r--r-- | src/test/netbase_tests.cpp | 13 | ||||
-rw-r--r-- | src/torcontrol.cpp | 6 | ||||
-rw-r--r-- | src/zmq/zmqpublishnotifier.cpp | 5 |
16 files changed, 171 insertions, 225 deletions
diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index 8a5cab443f..f044feebba 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -72,20 +72,6 @@ static void FillAddrMan(AddrMan& addrman) AddAddressesToAddrMan(addrman); } -static CNetAddr ResolveIP(const std::string& ip) -{ - CNetAddr addr; - LookupHost(ip, addr, false); - return addr; -} - -static CService ResolveService(const std::string& ip, uint16_t port = 0) -{ - CService serv; - Lookup(ip, serv, port, false); - return serv; -} - /* Benchmarks */ static void AddrManAdd(benchmark::Bench& bench) @@ -118,8 +104,8 @@ static void AddrManSelectFromAlmostEmpty(benchmark::Bench& bench) AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO}; // Add one address to the new table - CService addr = ResolveService("250.3.1.1", 8333); - addrman.Add({CAddress(addr, NODE_NONE)}, ResolveService("250.3.1.1", 8333)); + CService addr = Lookup("250.3.1.1", 8333, false).value(); + addrman.Add({CAddress(addr, NODE_NONE)}, addr); bench.run([&] { (void)addrman.Select(); @@ -135,7 +121,7 @@ static void AddrManSelectByNetwork(benchmark::Bench& bench) i2p_service.SetSpecial("udhdrtrcetjm5sxzskjyr5ztpeszydbh4dpl3pl4utgqqw2v4jna.b32.i2p"); CAddress i2p_address(i2p_service, NODE_NONE); i2p_address.nTime = Now<NodeSeconds>(); - CNetAddr source = ResolveIP("252.2.2.2"); + const CNetAddr source{LookupHost("252.2.2.2", false).value()}; addrman.Add({i2p_address}, source); FillAddrMan(addrman); diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 4f8a2b4d8d..128c4e3c56 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -170,12 +170,8 @@ static bool ClientAllowed(const CNetAddr& netaddr) static bool InitHTTPAllowList() { rpc_allow_subnets.clear(); - CNetAddr localv4; - CNetAddr localv6; - LookupHost("127.0.0.1", localv4, false); - LookupHost("::1", localv6, false); - rpc_allow_subnets.push_back(CSubNet(localv4, 8)); // always allow IPv4 local subnet - rpc_allow_subnets.push_back(CSubNet(localv6)); // always allow IPv6 localhost + rpc_allow_subnets.push_back(CSubNet{LookupHost("127.0.0.1", false).value(), 8}); // always allow IPv4 local subnet + rpc_allow_subnets.push_back(CSubNet{LookupHost("::1", false).value()}); // always allow IPv6 localhost for (const std::string& strAllow : gArgs.GetArgs("-rpcallowip")) { CSubNet subnet; LookupSubNet(strAllow, subnet); @@ -338,8 +334,8 @@ static bool HTTPBindAddresses(struct evhttp* http) LogPrintf("Binding RPC on address %s port %i\n", i->first, i->second); evhttp_bound_socket *bind_handle = evhttp_bind_socket_with_handle(http, i->first.empty() ? nullptr : i->first.c_str(), i->second); if (bind_handle) { - CNetAddr addr; - if (i->first.empty() || (LookupHost(i->first, addr, false) && addr.IsBindAny())) { + const std::optional<CNetAddr> addr{LookupHost(i->first, false)}; + if (i->first.empty() || (addr.has_value() && addr->IsBindAny())) { LogPrintf("WARNING: the RPC server is not safe to expose to untrusted networks such as the public internet\n"); } boundSockets.push_back(bind_handle); diff --git a/src/init.cpp b/src/init.cpp index d9e5e70ee3..e8c804b9a7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1359,12 +1359,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // -noproxy (or -proxy=0) as well as the empty string can be used to not set a proxy, this is the default std::string proxyArg = args.GetArg("-proxy", ""); if (proxyArg != "" && proxyArg != "0") { - CService proxyAddr; - if (!Lookup(proxyArg, proxyAddr, 9050, fNameLookup)) { + const std::optional<CService> proxyAddr{Lookup(proxyArg, 9050, fNameLookup)}; + if (!proxyAddr.has_value()) { return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg)); } - Proxy addrProxy = Proxy(proxyAddr, proxyRandomize); + Proxy addrProxy = Proxy(proxyAddr.value(), proxyRandomize); if (!addrProxy.IsValid()) return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg)); @@ -1390,11 +1390,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) "reaching the Tor network is explicitly forbidden: -onion=0")); } } else { - CService addr; - if (!Lookup(onionArg, addr, 9050, fNameLookup) || !addr.IsValid()) { + const std::optional<CService> addr{Lookup(onionArg, 9050, fNameLookup)}; + if (!addr.has_value() || !addr->IsValid()) { return InitError(strprintf(_("Invalid -onion address or hostname: '%s'"), onionArg)); } - onion_proxy = Proxy{addr, proxyRandomize}; + onion_proxy = Proxy{addr.value(), proxyRandomize}; } } @@ -1414,9 +1414,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } for (const std::string& strAddr : args.GetArgs("-externalip")) { - CService addrLocal; - if (Lookup(strAddr, addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid()) - AddLocal(addrLocal, LOCAL_MANUAL); + const std::optional<CService> addrLocal{Lookup(strAddr, GetListenPort(), fNameLookup)}; + if (addrLocal.has_value() && addrLocal->IsValid()) + AddLocal(addrLocal.value(), LOCAL_MANUAL); else return InitError(ResolveErrMsg("externalip", strAddr)); } @@ -1754,13 +1754,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) }; for (const std::string& bind_arg : args.GetArgs("-bind")) { - CService bind_addr; + std::optional<CService> bind_addr; const size_t index = bind_arg.rfind('='); if (index == std::string::npos) { - if (Lookup(bind_arg, bind_addr, default_bind_port, /*fAllowLookup=*/false)) { - connOptions.vBinds.push_back(bind_addr); - if (IsBadPort(bind_addr.GetPort())) { - InitWarning(BadPortWarning("-bind", bind_addr.GetPort())); + bind_addr = Lookup(bind_arg, default_bind_port, /*fAllowLookup=*/false); + if (bind_addr.has_value()) { + connOptions.vBinds.push_back(bind_addr.value()); + if (IsBadPort(bind_addr.value().GetPort())) { + InitWarning(BadPortWarning("-bind", bind_addr.value().GetPort())); } continue; } @@ -1768,8 +1769,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const std::string network_type = bind_arg.substr(index + 1); if (network_type == "onion") { const std::string truncated_bind_arg = bind_arg.substr(0, index); - if (Lookup(truncated_bind_arg, bind_addr, BaseParams().OnionServiceTargetPort(), false)) { - connOptions.onion_binds.push_back(bind_addr); + bind_addr = Lookup(truncated_bind_arg, BaseParams().OnionServiceTargetPort(), false); + if (bind_addr.has_value()) { + connOptions.onion_binds.push_back(bind_addr.value()); continue; } } @@ -1847,11 +1849,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const std::string& i2psam_arg = args.GetArg("-i2psam", ""); if (!i2psam_arg.empty()) { - CService addr; - if (!Lookup(i2psam_arg, addr, 7656, fNameLookup) || !addr.IsValid()) { + const std::optional<CService> addr{Lookup(i2psam_arg, 7656, fNameLookup)}; + if (!addr.has_value() || !addr->IsValid()) { return InitError(strprintf(_("Invalid -i2psam address or hostname: '%s'"), i2psam_arg)); } - SetProxy(NET_I2P, Proxy{addr}); + SetProxy(NET_I2P, Proxy{addr.value()}); } else { if (args.IsArgSet("-onlynet") && IsReachable(NET_I2P)) { return InitError( diff --git a/src/mapport.cpp b/src/mapport.cpp index 76f8a4d0c9..118827901a 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -175,10 +175,10 @@ static bool ProcessUpnp() LogPrintf("UPnP: GetExternalIPAddress() returned %d\n", r); } else { if (externalIPAddress[0]) { - CNetAddr resolved; - if (LookupHost(externalIPAddress, resolved, false)) { - LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToStringAddr()); - AddLocal(resolved, LOCAL_MAPPED); + std::optional<CNetAddr> resolved{LookupHost(externalIPAddress, false)}; + if (resolved.has_value()) { + LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved->ToStringAddr()); + AddLocal(resolved.value(), LOCAL_MAPPED); } } else { LogPrintf("UPnP: GetExternalIPAddress failed.\n"); diff --git a/src/net.cpp b/src/net.cpp index 337cf60680..a53835f999 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -130,14 +130,10 @@ uint16_t GetListenPort() { // If -bind= is provided with ":port" part, use that (first one if multiple are provided). for (const std::string& bind_arg : gArgs.GetArgs("-bind")) { - CService bind_addr; constexpr uint16_t dummy_port = 0; - if (Lookup(bind_arg, bind_addr, dummy_port, /*fAllowLookup=*/false)) { - if (bind_addr.GetPort() != dummy_port) { - return bind_addr.GetPort(); - } - } + const std::optional<CService> bind_addr{Lookup(bind_arg, dummy_port, /*fAllowLookup=*/false)}; + if (bind_addr.has_value() && bind_addr->GetPort() != dummy_port) return bind_addr->GetPort(); } // Otherwise, if -whitebind= without NetPermissionFlags::NoBan is provided, use that @@ -461,9 +457,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) : Params().GetDefaultPort()}; if (pszDest) { - std::vector<CService> resolved; - if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) { - const CService rnd{resolved[GetRand(resolved.size())]}; + const std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; + if (!resolved.empty()) { + const CService& rnd{resolved[GetRand(resolved.size())]}; addrConnect = CAddress{MaybeFlipIPv6toCJDNS(rnd), NODE_NONE}; if (!addrConnect.IsValid()) { LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToStringAddrPort(), pszDest); @@ -1487,7 +1483,6 @@ void CConnman::ThreadDNSAddressSeed() if (HaveNameProxy()) { AddAddrFetch(seed); } else { - std::vector<CNetAddr> vIPs; std::vector<CAddress> vAdd; ServiceFlags requiredServiceBits = GetDesirableServiceFlags(NODE_NONE); std::string host = strprintf("x%x.%s", requiredServiceBits, seed); @@ -1496,8 +1491,9 @@ void CConnman::ThreadDNSAddressSeed() continue; } unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed - if (LookupHost(host, vIPs, nMaxIPs, true)) { - for (const CNetAddr& ip : vIPs) { + const auto addresses{LookupHost(host, nMaxIPs, true)}; + if (!addresses.empty()) { + for (const CNetAddr& ip : addresses) { CAddress addr = CAddress(CService(ip, Params().GetDefaultPort()), requiredServiceBits); addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - 3 * 24h, -4 * 24h); // use a random age between 3 and 7 days old vAdd.push_back(addr); @@ -2201,14 +2197,11 @@ void Discover() char pszHostName[256] = ""; if (gethostname(pszHostName, sizeof(pszHostName)) != SOCKET_ERROR) { - std::vector<CNetAddr> vaddr; - if (LookupHost(pszHostName, vaddr, 0, true)) + const std::vector<CNetAddr> addresses{LookupHost(pszHostName, 0, true)}; + for (const CNetAddr& addr : addresses) { - for (const CNetAddr &addr : vaddr) - { - if (AddLocal(addr, LOCAL_IF)) - LogPrintf("%s: %s - %s\n", __func__, pszHostName, addr.ToStringAddr()); - } + if (AddLocal(addr, LOCAL_IF)) + LogPrintf("%s: %s - %s\n", __func__, pszHostName, addr.ToStringAddr()); } } #elif (HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS) diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 1e4ec5b529..23226bbb4f 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -88,18 +88,18 @@ bool NetWhitebindPermissions::TryParse(const std::string& str, NetWhitebindPermi if (!TryParsePermissionFlags(str, flags, offset, error)) return false; const std::string strBind = str.substr(offset); - CService addrBind; - if (!Lookup(strBind, addrBind, 0, false)) { + const std::optional<CService> addrBind{Lookup(strBind, 0, false)}; + if (!addrBind.has_value()) { error = ResolveErrMsg("whitebind", strBind); return false; } - if (addrBind.GetPort() == 0) { + if (addrBind.value().GetPort() == 0) { error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind); return false; } output.m_flags = flags; - output.m_service = addrBind; + output.m_service = addrBind.value(); error = Untranslated(""); return true; } diff --git a/src/netbase.cpp b/src/netbase.cpp index 4f78d2e31a..8f6f92ea7d 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -132,14 +132,9 @@ std::vector<std::string> GetNetworkNames(bool append_unroutable) return names; } -static bool LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) +static std::vector<CNetAddr> LookupIntern(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { - vIP.clear(); - - if (!ContainsNoNUL(name)) { - return false; - } - + if (!ContainsNoNUL(name)) return {}; { CNetAddr addr; // From our perspective, onion addresses are not hostnames but rather @@ -148,83 +143,65 @@ static bool LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un // getaddrinfo to decode them and it wouldn't make sense to resolve // them, we return a network address representing it instead. See // CNetAddr::SetSpecial(const std::string&) for more details. - if (addr.SetSpecial(name)) { - vIP.push_back(addr); - return true; - } + if (addr.SetSpecial(name)) return {addr}; } + std::vector<CNetAddr> addresses; + for (const CNetAddr& resolved : dns_lookup_function(name, fAllowLookup)) { - if (nMaxSolutions > 0 && vIP.size() >= nMaxSolutions) { + if (nMaxSolutions > 0 && addresses.size() >= nMaxSolutions) { break; } /* Never allow resolving to an internal address. Consider any such result invalid */ if (!resolved.IsInternal()) { - vIP.push_back(resolved); + addresses.push_back(resolved); } } - return (vIP.size() > 0); + return addresses; } -bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) +std::vector<CNetAddr> LookupHost(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { - if (!ContainsNoNUL(name)) { - return false; - } + if (!ContainsNoNUL(name)) return {}; std::string strHost = name; - if (strHost.empty()) - return false; + if (strHost.empty()) return {}; if (strHost.front() == '[' && strHost.back() == ']') { strHost = strHost.substr(1, strHost.size() - 2); } - return LookupIntern(strHost, vIP, nMaxSolutions, fAllowLookup, dns_lookup_function); + return LookupIntern(strHost, nMaxSolutions, fAllowLookup, dns_lookup_function); } -bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function) +std::optional<CNetAddr> LookupHost(const std::string& name, bool fAllowLookup, DNSLookupFn dns_lookup_function) { - if (!ContainsNoNUL(name)) { - return false; - } - std::vector<CNetAddr> vIP; - LookupHost(name, vIP, 1, fAllowLookup, dns_lookup_function); - if(vIP.empty()) - return false; - addr = vIP.front(); - return true; + const std::vector<CNetAddr> addresses{LookupHost(name, 1, fAllowLookup, dns_lookup_function)}; + return addresses.empty() ? std::nullopt : std::make_optional(addresses.front()); } -bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function) +std::vector<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function) { if (name.empty() || !ContainsNoNUL(name)) { - return false; + return {}; } uint16_t port{portDefault}; std::string hostname; SplitHostPort(name, port, hostname); - std::vector<CNetAddr> vIP; - bool fRet = LookupIntern(hostname, vIP, nMaxSolutions, fAllowLookup, dns_lookup_function); - if (!fRet) - return false; - vAddr.resize(vIP.size()); - for (unsigned int i = 0; i < vIP.size(); i++) - vAddr[i] = CService(vIP[i], port); - return true; + const std::vector<CNetAddr> addresses{LookupIntern(hostname, nMaxSolutions, fAllowLookup, dns_lookup_function)}; + if (addresses.empty()) return {}; + std::vector<CService> services; + services.reserve(addresses.size()); + for (const auto& addr : addresses) + services.emplace_back(addr, port); + return services; } -bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function) +std::optional<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function) { - if (!ContainsNoNUL(name)) { - return false; - } - std::vector<CService> vService; - bool fRet = Lookup(name, vService, portDefault, fAllowLookup, 1, dns_lookup_function); - if (!fRet) - return false; - addr = vService[0]; - return true; + const std::vector<CService> services{Lookup(name, portDefault, fAllowLookup, 1, dns_lookup_function)}; + + return services.empty() ? std::nullopt : std::make_optional(services.front()); } CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupFn dns_lookup_function) @@ -232,12 +209,9 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupF if (!ContainsNoNUL(name)) { return {}; } - CService addr; // "1.2:345" will fail to resolve the ip, but will still set the port. // If the ip fails to resolve, re-init the result. - if(!Lookup(name, addr, portDefault, false, dns_lookup_function)) - addr = CService(); - return addr; + return Lookup(name, portDefault, /*fAllowLookup=*/false, dns_lookup_function).value_or(CService{}); } /** SOCKS version */ @@ -689,27 +663,27 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) const size_t slash_pos{subnet_str.find_last_of('/')}; const std::string str_addr{subnet_str.substr(0, slash_pos)}; - CNetAddr addr; + const std::optional<CNetAddr> addr{LookupHost(str_addr, /*fAllowLookup=*/false)}; - if (LookupHost(str_addr, addr, /*fAllowLookup=*/false)) { + if (addr.has_value()) { 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}; + subnet_out = CSubNet{addr.value(), 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)) { - subnet_out = CSubNet{addr, full_netmask}; + const std::optional<CNetAddr> full_netmask{LookupHost(netmask_str, /*fAllowLookup=*/false)}; + if (full_netmask.has_value()) { + subnet_out = CSubNet{addr.value(), full_netmask.value()}; return subnet_out.IsValid(); } } } else { // Single IP subnet (<ipv4>/32 or <ipv6>/128). - subnet_out = CSubNet{addr}; + subnet_out = CSubNet{addr.value()}; return subnet_out.IsValid(); } } diff --git a/src/netbase.h b/src/netbase.h index a43f22f240..1da4f5c51d 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -105,24 +105,25 @@ extern DNSLookupFn g_dns_lookup; * @param name The string representing a host. Could be a name or a numerical * IP address (IPv6 addresses in their bracketed form are * allowed). - * @param[out] vIP The resulting network addresses to which the specified host - * string resolved. * - * @returns Whether or not the specified host string successfully resolved to - * any resulting network addresses. + * @returns The resulting network addresses to which the specified host + * string resolved. * - * @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn) + * @see Lookup(const std::string&, uint16_t, bool, unsigned int, DNSLookupFn) * for additional parameter descriptions. */ -bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); +std::vector<CNetAddr> LookupHost(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Resolve a host string to its first corresponding network address. * - * @see LookupHost(const std::string&, std::vector<CNetAddr>&, uint16_t, bool, DNSLookupFn) + * @returns The resulting network address to which the specified host + * string resolved or std::nullopt if host does not resolve to an address. + * + * @see LookupHost(const std::string&, unsigned int, bool, DNSLookupFn) * for additional parameter descriptions. */ -bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); +std::optional<CNetAddr> LookupHost(const std::string& name, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Resolve a service string to its corresponding service. @@ -132,8 +133,6 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL * disambiguated bracketed form), optionally followed by a uint16_t port * number. (e.g. example.com:8333 or * [2001:db8:85a3:8d3:1319:8a2e:370:7348]:420) - * @param[out] vAddr The resulting services to which the specified service string - * resolved. * @param portDefault The default port for resulting services if not specified * by the service string. * @param fAllowLookup Whether or not hostname lookups are permitted. If yes, @@ -141,18 +140,18 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL * @param nMaxSolutions The maximum number of results we want, specifying 0 * means "as many solutions as we get." * - * @returns Whether or not the service string successfully resolved to any - * resulting services. + * @returns The resulting services to which the specified service string + * resolved. */ -bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup); +std::vector<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Resolve a service string to its first corresponding service. * - * @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn) + * @see Lookup(const std::string&, uint16_t, bool, unsigned int, DNSLookupFn) * for additional parameter descriptions. */ -bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); +std::optional<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Resolve a service string with a numeric IP to its first corresponding @@ -160,7 +159,7 @@ bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool * * @returns The resulting CService if the resolution was successful, [::]:0 otherwise. * - * @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn) + * @see Lookup(const std::string&, uint16_t, bool, unsigned int, DNSLookupFn) * for additional parameter descriptions. */ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup); diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index ea624efeaa..512fce473d 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -406,9 +406,8 @@ void OptionsDialog::updateProxyValidationState() void OptionsDialog::updateDefaultProxyNets() { - CNetAddr ui_proxy_netaddr; - LookupHost(ui->proxyIp->text().toStdString(), ui_proxy_netaddr, /*fAllowLookup=*/false); - const CService ui_proxy{ui_proxy_netaddr, ui->proxyPort->text().toUShort()}; + const std::optional<CNetAddr> ui_proxy_netaddr{LookupHost(ui->proxyIp->text().toStdString(), /*fAllowLookup=*/false)}; + const CService ui_proxy{ui_proxy_netaddr.value_or(CNetAddr{}), ui->proxyPort->text().toUShort()}; Proxy proxy; bool has_proxy; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f247e6ee91..a2a46ef32f 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -713,9 +713,10 @@ static RPCHelpMan setban() isSubnet = true; if (!isSubnet) { - CNetAddr resolved; - LookupHost(request.params[0].get_str(), resolved, false); - netAddr = resolved; + const std::optional<CNetAddr> addr{LookupHost(request.params[0].get_str(), false)}; + if (addr.has_value()) { + netAddr = addr.value(); + } } else LookupSubNet(request.params[0].get_str(), subNet); @@ -943,11 +944,11 @@ static RPCHelpMan addpeeraddress() const bool tried{request.params[2].isNull() ? false : request.params[2].get_bool()}; UniValue obj(UniValue::VOBJ); - CNetAddr net_addr; + std::optional<CNetAddr> net_addr{LookupHost(addr_string, false)}; bool success{false}; - if (LookupHost(addr_string, net_addr, false)) { - CService service{net_addr, port}; + if (net_addr.has_value()) { + CService service{net_addr.value(), port}; CAddress address{MaybeFlipIPv6toCJDNS(service), ServiceFlags{NODE_NETWORK | NODE_WITNESS}}; address.nTime = Now<NodeSeconds>(); // The source address is set equal to the address. This is equivalent to the peer diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 54d923e4a4..328e7f81a0 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -33,16 +33,16 @@ static int32_t GetCheckRatio(const NodeContext& node_ctx) static CNetAddr ResolveIP(const std::string& ip) { - CNetAddr addr; - BOOST_CHECK_MESSAGE(LookupHost(ip, addr, false), strprintf("failed to resolve: %s", ip)); - return addr; + const std::optional<CNetAddr> addr{LookupHost(ip, false)}; + BOOST_CHECK_MESSAGE(addr.has_value(), strprintf("failed to resolve: %s", ip)); + return addr.value_or(CNetAddr{}); } static CService ResolveService(const std::string& ip, uint16_t port = 0) { - CService serv; - BOOST_CHECK_MESSAGE(Lookup(ip, serv, port, false), strprintf("failed to resolve: %s:%i", ip, port)); - return serv; + const std::optional<CService> serv{Lookup(ip, port, false)}; + BOOST_CHECK_MESSAGE(serv.has_value(), strprintf("failed to resolve: %s:%i", ip, port)); + return serv.value_or(CService{}); } @@ -948,18 +948,23 @@ BOOST_AUTO_TEST_CASE(load_addrman) { AddrMan addrman{EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)}; - CService addr1, addr2, addr3; - BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); - BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false)); - BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false)); - BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false)); - BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false)); + std::optional<CService> addr1, addr2, addr3, addr4; + addr1 = Lookup("250.7.1.1", 8333, false); + BOOST_CHECK(addr1.has_value()); + addr2 = Lookup("250.7.2.2", 9999, false); + BOOST_CHECK(addr2.has_value()); + addr3 = Lookup("250.7.3.3", 9999, false); + BOOST_CHECK(addr3.has_value()); + addr3 = Lookup("250.7.3.3"s, 9999, false); + BOOST_CHECK(addr3.has_value()); + addr4 = Lookup("250.7.3.3\0example.com"s, 9999, false); + BOOST_CHECK(!addr4.has_value()); // Add three addresses to new table. - CService source; - BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false)); - std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)}; - BOOST_CHECK(addrman.Add(addresses, source)); + const std::optional<CService> source{Lookup("252.5.1.1", 8333, false)}; + BOOST_CHECK(source.has_value()); + std::vector<CAddress> addresses{CAddress(addr1.value(), NODE_NONE), CAddress(addr2.value(), NODE_NONE), CAddress(addr3.value(), NODE_NONE)}; + BOOST_CHECK(addrman.Add(addresses, source.value())); BOOST_CHECK(addrman.Size() == 3); // Test that the de-serialization does not throw an exception. @@ -1004,12 +1009,12 @@ static CDataStream MakeCorruptPeersDat() int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); s << nUBuckets; - CService serv; - BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); - CAddress addr = CAddress(serv, NODE_NONE); - CNetAddr resolved; - BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); - AddrInfo info = AddrInfo(addr, resolved); + const std::optional<CService> serv{Lookup("252.1.1.1", 7777, false)}; + BOOST_REQUIRE(serv.has_value()); + CAddress addr = CAddress(serv.value(), NODE_NONE); + std::optional<CNetAddr> resolved{LookupHost("252.2.2.2", false)}; + BOOST_REQUIRE(resolved.has_value()); + AddrInfo info = AddrInfo(addr, resolved.value()); s << info; return s; diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp index 81e216b358..dcf500acc3 100644 --- a/src/test/fuzz/netbase_dns_lookup.cpp +++ b/src/test/fuzz/netbase_dns_lookup.cpp @@ -29,33 +29,29 @@ FUZZ_TARGET(netbase_dns_lookup) }; { - std::vector<CNetAddr> resolved_addresses; - if (LookupHost(name, resolved_addresses, max_results, allow_lookup, fuzzed_dns_lookup_function)) { - for (const CNetAddr& resolved_address : resolved_addresses) { - assert(!resolved_address.IsInternal()); - } + const std::vector<CNetAddr> resolved_addresses{LookupHost(name, max_results, allow_lookup, fuzzed_dns_lookup_function)}; + for (const CNetAddr& resolved_address : resolved_addresses) { + assert(!resolved_address.IsInternal()); } assert(resolved_addresses.size() <= max_results || max_results == 0); } { - CNetAddr resolved_address; - if (LookupHost(name, resolved_address, allow_lookup, fuzzed_dns_lookup_function)) { - assert(!resolved_address.IsInternal()); + const std::optional<CNetAddr> resolved_address{LookupHost(name, allow_lookup, fuzzed_dns_lookup_function)}; + if (resolved_address.has_value()) { + assert(!resolved_address.value().IsInternal()); } } { - std::vector<CService> resolved_services; - if (Lookup(name, resolved_services, default_port, allow_lookup, max_results, fuzzed_dns_lookup_function)) { - for (const CNetAddr& resolved_service : resolved_services) { - assert(!resolved_service.IsInternal()); - } + const std::vector<CService> resolved_services{Lookup(name, default_port, allow_lookup, max_results, fuzzed_dns_lookup_function)}; + for (const CNetAddr& resolved_service : resolved_services) { + assert(!resolved_service.IsInternal()); } assert(resolved_services.size() <= max_results || max_results == 0); } { - CService resolved_service; - if (Lookup(name, resolved_service, default_port, allow_lookup, fuzzed_dns_lookup_function)) { - assert(!resolved_service.IsInternal()); + const std::optional<CService> resolved_service{Lookup(name, default_port, allow_lookup, fuzzed_dns_lookup_function)}; + if (resolved_service.has_value()) { + assert(!resolved_service.value().IsInternal()); } } { diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 631c213627..aa577f7b27 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) CNetAddr addr; // IPv4, INADDR_ANY - BOOST_REQUIRE(LookupHost("0.0.0.0", addr, false)); + addr = LookupHost("0.0.0.0", false).value(); BOOST_REQUIRE(!addr.IsValid()); BOOST_REQUIRE(addr.IsIPv4()); @@ -144,7 +144,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) BOOST_CHECK_EQUAL(addr.ToStringAddr(), "0.0.0.0"); // IPv4, INADDR_NONE - BOOST_REQUIRE(LookupHost("255.255.255.255", addr, false)); + addr = LookupHost("255.255.255.255", false).value(); BOOST_REQUIRE(!addr.IsValid()); BOOST_REQUIRE(addr.IsIPv4()); @@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) BOOST_CHECK_EQUAL(addr.ToStringAddr(), "255.255.255.255"); // IPv4, casual - BOOST_REQUIRE(LookupHost("12.34.56.78", addr, false)); + addr = LookupHost("12.34.56.78", false).value(); BOOST_REQUIRE(addr.IsValid()); BOOST_REQUIRE(addr.IsIPv4()); @@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) BOOST_CHECK_EQUAL(addr.ToStringAddr(), "12.34.56.78"); // IPv6, in6addr_any - BOOST_REQUIRE(LookupHost("::", addr, false)); + addr = LookupHost("::", false).value(); BOOST_REQUIRE(!addr.IsValid()); BOOST_REQUIRE(addr.IsIPv6()); @@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) BOOST_CHECK_EQUAL(addr.ToStringAddr(), "::"); // IPv6, casual - BOOST_REQUIRE(LookupHost("1122:3344:5566:7788:9900:aabb:ccdd:eeff", addr, false)); + addr = LookupHost("1122:3344:5566:7788:9900:aabb:ccdd:eeff", false).value(); BOOST_REQUIRE(addr.IsValid()); BOOST_REQUIRE(addr.IsIPv6()); @@ -186,14 +186,14 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) // id of "32", return the address as "fe80::1%32". const std::string link_local{"fe80::1"}; const std::string scoped_addr{link_local + "%32"}; - BOOST_REQUIRE(LookupHost(scoped_addr, addr, false)); + addr = LookupHost(scoped_addr, false).value(); BOOST_REQUIRE(addr.IsValid()); BOOST_REQUIRE(addr.IsIPv6()); BOOST_CHECK(!addr.IsBindAny()); BOOST_CHECK_EQUAL(addr.ToStringAddr(), scoped_addr); // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope. - BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false)); + addr = LookupHost(link_local + "%0", false).value(); BOOST_REQUIRE(addr.IsValid()); BOOST_REQUIRE(addr.IsIPv6()); BOOST_CHECK(!addr.IsBindAny()); @@ -318,10 +318,9 @@ BOOST_AUTO_TEST_CASE(cnetaddr_tostring_canonical_ipv6) {"2001:db8:aaaa:bbbb:cccc:dddd:eeee:AaAa", "2001:db8:aaaa:bbbb:cccc:dddd:eeee:aaaa"}, }; for (const auto& [input_address, expected_canonical_representation_output] : canonical_representations_ipv6) { - CNetAddr net_addr; - BOOST_REQUIRE(LookupHost(input_address, net_addr, false)); - BOOST_REQUIRE(net_addr.IsIPv6()); - BOOST_CHECK_EQUAL(net_addr.ToStringAddr(), expected_canonical_representation_output); + const std::optional<CNetAddr> net_addr{LookupHost(input_address, false)}; + BOOST_REQUIRE(net_addr.value().IsIPv6()); + BOOST_CHECK_EQUAL(net_addr.value().ToStringAddr(), expected_canonical_representation_output); } } @@ -334,12 +333,12 @@ BOOST_AUTO_TEST_CASE(cnetaddr_serialize_v1) BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000000000000000"); s.clear(); - BOOST_REQUIRE(LookupHost("1.2.3.4", addr, false)); + addr = LookupHost("1.2.3.4", false).value(); s << addr; BOOST_CHECK_EQUAL(HexStr(s), "00000000000000000000ffff01020304"); s.clear(); - BOOST_REQUIRE(LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", addr, false)); + addr = LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", false).value(); s << addr; BOOST_CHECK_EQUAL(HexStr(s), "1a1b2a2b3a3b4a4b5a5b6a6b7a7b8a8b"); s.clear(); @@ -370,12 +369,12 @@ BOOST_AUTO_TEST_CASE(cnetaddr_serialize_v2) BOOST_CHECK_EQUAL(HexStr(s), "021000000000000000000000000000000000"); s.clear(); - BOOST_REQUIRE(LookupHost("1.2.3.4", addr, false)); + addr = LookupHost("1.2.3.4", false).value(); s << addr; BOOST_CHECK_EQUAL(HexStr(s), "010401020304"); s.clear(); - BOOST_REQUIRE(LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", addr, false)); + addr = LookupHost("1a1b:2a2b:3a3b:4a4b:5a5b:6a6b:7a7b:8a8b", false).value(); s << addr; BOOST_CHECK_EQUAL(HexStr(s), "02101a1b2a2b3a3b4a4b5a5b6a6b7a7b8a8b"); s.clear(); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 7e91819ddc..05953bfd10 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -24,9 +24,7 @@ BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup) static CNetAddr ResolveIP(const std::string& ip) { - CNetAddr addr; - LookupHost(ip, addr, false); - return addr; + return LookupHost(ip, false).value_or(CNetAddr{}); } static CSubNet ResolveSubNet(const std::string& subnet) @@ -477,11 +475,10 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) { - CNetAddr addr; - BOOST_CHECK(LookupHost("127.0.0.1"s, addr, false)); - BOOST_CHECK(!LookupHost("127.0.0.1\0"s, addr, false)); - BOOST_CHECK(!LookupHost("127.0.0.1\0example.com"s, addr, false)); - BOOST_CHECK(!LookupHost("127.0.0.1\0example.com\0"s, addr, false)); + BOOST_CHECK(LookupHost("127.0.0.1"s, false).has_value()); + BOOST_CHECK(!LookupHost("127.0.0.1\0"s, false).has_value()); + BOOST_CHECK(!LookupHost("127.0.0.1\0example.com"s, false).has_value()); + BOOST_CHECK(!LookupHost("127.0.0.1\0example.com\0"s, false).has_value()); CSubNet ret; BOOST_CHECK(LookupSubNet("1.2.3.0/24"s, ret)); BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0"s, ret)); diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 92b55f9fc4..98d68f93e9 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -133,15 +133,15 @@ bool TorControlConnection::Connect(const std::string& tor_control_center, const Disconnect(); } - CService control_service; - if (!Lookup(tor_control_center, control_service, 9051, fNameLookup)) { + const std::optional<CService> control_service{Lookup(tor_control_center, 9051, fNameLookup)}; + if (!control_service.has_value()) { LogPrintf("tor: Failed to look up control center %s\n", tor_control_center); return false; } struct sockaddr_storage control_address; socklen_t control_address_len = sizeof(control_address); - if (!control_service.GetSockAddr(reinterpret_cast<struct sockaddr*>(&control_address), &control_address_len)) { + if (!control_service.value().GetSockAddr(reinterpret_cast<struct sockaddr*>(&control_address), &control_address_len)) { LogPrintf("tor: Error parsing socket address %s\n", tor_control_center); return false; } diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index 21aa44c309..1241431523 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -97,9 +97,8 @@ static bool IsZMQAddressIPV6(const std::string &zmq_address) const size_t colon_index = zmq_address.rfind(':'); if (tcp_index == 0 && colon_index != std::string::npos) { const std::string ip = zmq_address.substr(tcp_prefix.length(), colon_index - tcp_prefix.length()); - CNetAddr addr; - LookupHost(ip, addr, false); - if (addr.IsIPv6()) return true; + const std::optional<CNetAddr> addr{LookupHost(ip, false)}; + if (addr.has_value() && addr.value().IsIPv6()) return true; } return false; } |