From 6423c8175fad3163c10ffdb49e0df48e4e4931f1 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 1 Mar 2021 21:35:28 +0100 Subject: [PATCH 1/4] p2p, refactor: pass and use uint16_t CService::port as uint16_t --- src/bitcoin-cli.cpp | 4 ++-- src/chainparams.h | 4 ++-- src/httpserver.cpp | 6 +++--- src/net.cpp | 6 +++--- src/net.h | 2 +- src/netbase.cpp | 16 ++++++++-------- src/netbase.h | 20 ++++++++++---------- src/qt/optionsdialog.h | 2 +- src/rpc/net.cpp | 2 +- src/test/addrman_tests.cpp | 2 +- src/test/fuzz/netbase_dns_lookup.cpp | 2 +- src/test/fuzz/socks5.cpp | 2 +- src/test/fuzz/string.cpp | 2 +- src/test/net_tests.cpp | 2 +- src/test/netbase_tests.cpp | 18 +++++++++--------- src/util/strencodings.cpp | 3 ++- src/util/strencodings.h | 2 +- 17 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index dc4b142f835..05e1fcae711 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -618,9 +618,9 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co // 1. -rpcport // 2. port in -rpcconnect (ie following : in ipv4 or ]: in ipv6) // 3. default port for chain - int port = BaseParams().RPCPort(); + uint16_t port{BaseParams().RPCPort()}; SplitHostPort(gArgs.GetArg("-rpcconnect", DEFAULT_RPCCONNECT), port, host); - port = gArgs.GetArg("-rpcport", port); + port = static_cast(gArgs.GetArg("-rpcport", port)); // Obtain event base raii_event_base base = obtain_event_base(); diff --git a/src/chainparams.h b/src/chainparams.h index 4d24dcdb7c6..013f075be65 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -84,7 +84,7 @@ public: const Consensus::Params& GetConsensus() const { return consensus; } const CMessageHeader::MessageStartChars& MessageStart() const { return pchMessageStart; } - int GetDefaultPort() const { return nDefaultPort; } + uint16_t GetDefaultPort() const { return nDefaultPort; } const CBlock& GenesisBlock() const { return genesis; } /** Default value for -checkmempool and -checkblockindex argument */ @@ -121,7 +121,7 @@ protected: Consensus::Params consensus; CMessageHeader::MessageStartChars pchMessageStart; - int nDefaultPort; + uint16_t nDefaultPort; uint64_t nPruneAfterHeight; uint64_t m_assumed_blockchain_size; uint64_t m_assumed_chain_state_size; diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 0a8e58ab674..e1c8ec4ab97 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -290,8 +290,8 @@ static bool ThreadHTTP(struct event_base* base) /** Bind HTTP server to specified addresses */ static bool HTTPBindAddresses(struct evhttp* http) { - int http_port = gArgs.GetArg("-rpcport", BaseParams().RPCPort()); - std::vector > endpoints; + uint16_t http_port{static_cast(gArgs.GetArg("-rpcport", BaseParams().RPCPort()))}; + std::vector> endpoints; // Determine what addresses to bind to if (!(gArgs.IsArgSet("-rpcallowip") && gArgs.IsArgSet("-rpcbind"))) { // Default to loopback if not allowing external IPs @@ -305,7 +305,7 @@ static bool HTTPBindAddresses(struct evhttp* http) } } else if (gArgs.IsArgSet("-rpcbind")) { // Specific bind address for (const std::string& strRPCBind : gArgs.GetArgs("-rpcbind")) { - int port = http_port; + uint16_t port{http_port}; std::string host; SplitHostPort(strRPCBind, port, host); endpoints.push_back(std::make_pair(host, port)); diff --git a/src/net.cpp b/src/net.cpp index 1e4a6a9aa78..fdebaeee563 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -113,7 +113,7 @@ void CConnman::AddAddrFetch(const std::string& strDest) uint16_t GetListenPort() { - return (uint16_t)(gArgs.GetArg("-port", Params().GetDefaultPort())); + return static_cast(gArgs.GetArg("-port", Params().GetDefaultPort())); } // find 'best' local address for a particular peer @@ -394,7 +394,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo pszDest ? 0.0 : (double)(GetAdjustedTime() - addrConnect.nTime)/3600.0); // Resolve - const int default_port = Params().GetDefaultPort(); + const uint16_t default_port{Params().GetDefaultPort()}; if (pszDest) { std::vector resolved; if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) { @@ -462,7 +462,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return nullptr; } std::string host; - int port = default_port; + uint16_t port{default_port}; SplitHostPort(std::string(pszDest), port, host); bool proxyConnectionFailed; connected = ConnectThroughProxy(proxy, host, port, *sock, nConnectTimeout, diff --git a/src/net.h b/src/net.h index beef47f045b..259387e522f 100644 --- a/src/net.h +++ b/src/net.h @@ -229,7 +229,7 @@ extern std::string strSubVersion; struct LocalServiceInfo { int nScore; - int nPort; + uint16_t nPort; }; extern RecursiveMutex cs_mapLocalHost; diff --git a/src/netbase.cpp b/src/netbase.cpp index ac2392ebedf..49e455aa848 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -194,12 +194,12 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL return true; } -bool Lookup(const std::string& name, std::vector& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function) +bool Lookup(const std::string& name, std::vector& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function) { if (name.empty() || !ValidAsCString(name)) { return false; } - int port = portDefault; + uint16_t port{portDefault}; std::string hostname; SplitHostPort(name, port, hostname); @@ -213,7 +213,7 @@ bool Lookup(const std::string& name, std::vector& vAddr, int portDefau return true; } -bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function) +bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(name)) { return false; @@ -226,7 +226,7 @@ bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllo return true; } -CService LookupNumeric(const std::string& name, int portDefault, DNSLookupFn dns_lookup_function) +CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(name)) { return {}; @@ -363,7 +363,7 @@ static std::string Socks5ErrorString(uint8_t err) } } -bool Socks5(const std::string& strDest, int port, const ProxyCredentials* auth, const Sock& sock) +bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& sock) { IntrRecvError recvr; LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest); @@ -665,7 +665,7 @@ bool IsProxy(const CNetAddr &addr) { return false; } -bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed) +bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed) { // first connect to proxy server if (!ConnectSocketDirectly(proxy.proxy, sock.Get(), nTimeout, true)) { @@ -677,11 +677,11 @@ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int ProxyCredentials random_auth; static std::atomic_int counter(0); random_auth.username = random_auth.password = strprintf("%i", counter++); - if (!Socks5(strDest, (uint16_t)port, &random_auth, sock)) { + if (!Socks5(strDest, port, &random_auth, sock)) { return false; } } else { - if (!Socks5(strDest, (uint16_t)port, 0, sock)) { + if (!Socks5(strDest, port, 0, sock)) { return false; } } diff --git a/src/netbase.h b/src/netbase.h index e98a21ce1f2..08172b9984f 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -111,7 +111,7 @@ extern DNSLookupFn g_dns_lookup; * @returns Whether or not the specified host string successfully resolved to * any resulting network addresses. * - * @see Lookup(const std::string&, std::vector&, int, bool, unsigned int, DNSLookupFn) + * @see Lookup(const std::string&, std::vector&, uint16_t, bool, unsigned int, DNSLookupFn) * for additional parameter descriptions. */ bool LookupHost(const std::string& name, std::vector& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); @@ -119,7 +119,7 @@ bool LookupHost(const std::string& name, std::vector& vIP, unsigned in /** * Resolve a host string to its first corresponding network address. * - * @see LookupHost(const std::string&, std::vector&, unsigned int, bool, DNSLookupFn) + * @see LookupHost(const std::string&, std::vector&, uint16_t, bool, DNSLookupFn) * for additional parameter descriptions. */ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); @@ -129,7 +129,7 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL * * @param name The string representing a service. Could be a name or a * numerical IP address (IPv6 addresses should be in their - * disambiguated bracketed form), optionally followed by a port + * 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 @@ -144,15 +144,15 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL * @returns Whether or not the service string successfully resolved to any * resulting services. */ -bool Lookup(const std::string& name, std::vector& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup); +bool Lookup(const std::string& name, std::vector& vAddr, 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&, int, bool, unsigned int, DNSLookupFn) + * @see Lookup(const std::string&, std::vector&, uint16_t, bool, unsigned int, DNSLookupFn) * for additional parameter descriptions. */ -bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); +bool Lookup(const std::string& name, CService& addr, 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,10 +160,10 @@ bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllo * * @returns The resulting CService if the resolution was successful, [::]:0 otherwise. * - * @see Lookup(const std::string&, std::vector&, int, bool, unsigned int, DNSLookupFn) + * @see Lookup(const std::string&, std::vector&, uint16_t, bool, unsigned int, DNSLookupFn) * for additional parameter descriptions. */ -CService LookupNumeric(const std::string& name, int portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup); +CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Parse and resolve a specified subnet string into the appropriate internal @@ -219,7 +219,7 @@ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, i * * @returns Whether or not the operation succeeded. */ -bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed); +bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed); /** Disable or enable blocking-mode for a socket */ bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking); @@ -245,6 +245,6 @@ void InterruptSocks5(bool interrupt); * @see RFC1928: SOCKS Protocol * Version 5 */ -bool Socks5(const std::string& strDest, int port, const ProxyCredentials* auth, const Sock& socket); +bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& socket); #endif // BITCOIN_NETBASE_H diff --git a/src/qt/optionsdialog.h b/src/qt/optionsdialog.h index 1cc96035c6b..ba35ff3b67e 100644 --- a/src/qt/optionsdialog.h +++ b/src/qt/optionsdialog.h @@ -67,7 +67,7 @@ private Q_SLOTS: void updateDefaultProxyNets(); Q_SIGNALS: - void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, int nProxyPort); + void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, uint16_t nProxyPort); private: Ui::OptionsDialog *ui; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index cf4d46cf2cc..d4c1ab4b532 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -914,7 +914,7 @@ static RPCHelpMan addpeeraddress() UniValue obj(UniValue::VOBJ); std::string addr_string = request.params[0].get_str(); - uint16_t port = request.params[1].get_int(); + uint16_t port{static_cast(request.params[1].get_int())}; CNetAddr net_addr; if (!LookupHost(addr_string, net_addr, false)) { diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 37ff8a9afe9..d438537606a 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -100,7 +100,7 @@ static CNetAddr ResolveIP(const std::string& ip) return addr; } -static CService ResolveService(const std::string& ip, const int port = 0) +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)); diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp index 7be8b137435..cf2fa337443 100644 --- a/src/test/fuzz/netbase_dns_lookup.cpp +++ b/src/test/fuzz/netbase_dns_lookup.cpp @@ -18,7 +18,7 @@ FUZZ_TARGET(netbase_dns_lookup) const std::string name = fuzzed_data_provider.ConsumeRandomLengthString(512); const unsigned int max_results = fuzzed_data_provider.ConsumeIntegral(); const bool allow_lookup = fuzzed_data_provider.ConsumeBool(); - const int default_port = fuzzed_data_provider.ConsumeIntegral(); + const uint16_t default_port = fuzzed_data_provider.ConsumeIntegral(); auto fuzzed_dns_lookup_function = [&](const std::string&, bool) { std::vector resolved_addresses; diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp index e5cc4cabe54..d7b32ef46f1 100644 --- a/src/test/fuzz/socks5.cpp +++ b/src/test/fuzz/socks5.cpp @@ -38,7 +38,7 @@ FUZZ_TARGET_INIT(socks5, initialize_socks5) // This Socks5(...) fuzzing harness would have caught CVE-2017-18350 within // a few seconds of fuzzing. (void)Socks5(fuzzed_data_provider.ConsumeRandomLengthString(512), - fuzzed_data_provider.ConsumeIntegral(), + fuzzed_data_provider.ConsumeIntegral(), fuzzed_data_provider.ConsumeBool() ? &proxy_credentials : nullptr, fuzzed_sock); } diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 93b4948a2fa..835b59f05cc 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -82,7 +82,7 @@ FUZZ_TARGET(string) #ifndef WIN32 (void)ShellEscape(random_string_1); #endif // WIN32 - int port_out; + uint16_t port_out; std::string host_out; SplitHostPort(random_string_1, port_out, host_out); (void)TimingResistantEqual(random_string_1, random_string_2); diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 167242a971e..3af49c46f87 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -91,7 +91,7 @@ BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(cnode_listen_port) { // test default - uint16_t port = GetListenPort(); + uint16_t port{GetListenPort()}; BOOST_CHECK(port == Params().GetDefaultPort()); // test set port uint16_t altPort = 12345; diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 66ad7bb5eae..545fca1a333 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -83,31 +83,31 @@ BOOST_AUTO_TEST_CASE(netbase_properties) } -bool static TestSplitHost(std::string test, std::string host, int port) +bool static TestSplitHost(const std::string& test, const std::string& host, uint16_t port) { std::string hostOut; - int portOut = -1; + uint16_t portOut{0}; SplitHostPort(test, portOut, hostOut); return hostOut == host && port == portOut; } BOOST_AUTO_TEST_CASE(netbase_splithost) { - BOOST_CHECK(TestSplitHost("www.bitcoincore.org", "www.bitcoincore.org", -1)); - BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]", "www.bitcoincore.org", -1)); + BOOST_CHECK(TestSplitHost("www.bitcoincore.org", "www.bitcoincore.org", 0)); + BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]", "www.bitcoincore.org", 0)); BOOST_CHECK(TestSplitHost("www.bitcoincore.org:80", "www.bitcoincore.org", 80)); BOOST_CHECK(TestSplitHost("[www.bitcoincore.org]:80", "www.bitcoincore.org", 80)); - BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", -1)); + BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", 0)); BOOST_CHECK(TestSplitHost("127.0.0.1:8333", "127.0.0.1", 8333)); - BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", -1)); + BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", 0)); BOOST_CHECK(TestSplitHost("[127.0.0.1]:8333", "127.0.0.1", 8333)); - BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", -1)); + BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", 0)); BOOST_CHECK(TestSplitHost("[::ffff:127.0.0.1]:8333", "::ffff:127.0.0.1", 8333)); BOOST_CHECK(TestSplitHost("[::]:8333", "::", 8333)); - BOOST_CHECK(TestSplitHost("::8333", "::8333", -1)); + BOOST_CHECK(TestSplitHost("::8333", "::8333", 0)); BOOST_CHECK(TestSplitHost(":8333", "", 8333)); BOOST_CHECK(TestSplitHost("[]:8333", "", 8333)); - BOOST_CHECK(TestSplitHost("", "", -1)); + BOOST_CHECK(TestSplitHost("", "", 0)); } bool static TestParse(std::string src, std::string canon) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index f3d54a2ac9e..e9d85c4c304 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -107,7 +107,8 @@ std::vector ParseHex(const std::string& str) return ParseHex(str.c_str()); } -void SplitHostPort(std::string in, int &portOut, std::string &hostOut) { +void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut) +{ size_t colon = in.find_last_of(':'); // if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator bool fHaveColon = colon != in.npos; diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 98379e91383..a450b30ca23 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -65,7 +65,7 @@ std::string EncodeBase32(Span input, bool pad = true); */ std::string EncodeBase32(const std::string& str, bool pad = true); -void SplitHostPort(std::string in, int& portOut, std::string& hostOut); +void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut); int64_t atoi64(const std::string& str); int atoi(const std::string& str); From 2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 2 Mar 2021 22:37:50 +0100 Subject: [PATCH 2/4] util: add ParseUInt16(), use it in SplitHostPort() --- src/util/strencodings.cpp | 16 ++++++++++++++-- src/util/strencodings.h | 7 +++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index e9d85c4c304..8ccb71280db 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -115,8 +115,8 @@ void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut) bool fBracketed = fHaveColon && (in[0]=='[' && in[colon-1]==']'); // if there is a colon, and in[0]=='[', colon is not 0, so in[colon-1] is safe bool fMultiColon = fHaveColon && (in.find_last_of(':',colon-1) != in.npos); if (fHaveColon && (colon==0 || fBracketed || !fMultiColon)) { - int32_t n; - if (ParseInt32(in.substr(colon + 1), &n) && n > 0 && n < 0x10000) { + uint16_t n; + if (ParseUInt16(in.substr(colon + 1), &n)) { in = in.substr(0, colon); portOut = n; } @@ -335,6 +335,18 @@ bool ParseUInt8(const std::string& str, uint8_t *out) return true; } +bool ParseUInt16(const std::string& str, uint16_t* out) +{ + uint32_t u32; + if (!ParseUInt32(str, &u32) || u32 > std::numeric_limits::max()) { + return false; + } + if (out != nullptr) { + *out = static_cast(u32); + } + return true; +} + bool ParseUInt32(const std::string& str, uint32_t *out) { if (!ParsePrechecks(str)) diff --git a/src/util/strencodings.h b/src/util/strencodings.h index a450b30ca23..26dc0a0ce32 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -115,6 +115,13 @@ constexpr inline bool IsSpace(char c) noexcept { */ [[nodiscard]] bool ParseUInt8(const std::string& str, uint8_t *out); +/** + * Convert decimal string to unsigned 16-bit integer with strict parse error feedback. + * @returns true if the entire string could be parsed as valid integer, + * false if the entire string could not be parsed or if overflow or underflow occurred. + */ +[[nodiscard]] bool ParseUInt16(const std::string& str, uint16_t* out); + /** * Convert decimal string to unsigned 32-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, From 6f09c0f6b57ac01a473c587a3e51e9d477866bb0 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 6 Mar 2021 21:17:50 +0100 Subject: [PATCH 3/4] util: add missing braces and apply clang format to SplitHostPort() --- src/util/strencodings.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 8ccb71280db..4734de3e0b4 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -112,19 +112,20 @@ void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut) size_t colon = in.find_last_of(':'); // if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator bool fHaveColon = colon != in.npos; - bool fBracketed = fHaveColon && (in[0]=='[' && in[colon-1]==']'); // if there is a colon, and in[0]=='[', colon is not 0, so in[colon-1] is safe - bool fMultiColon = fHaveColon && (in.find_last_of(':',colon-1) != in.npos); - if (fHaveColon && (colon==0 || fBracketed || !fMultiColon)) { + bool fBracketed = fHaveColon && (in[0] == '[' && in[colon - 1] == ']'); // if there is a colon, and in[0]=='[', colon is not 0, so in[colon-1] is safe + bool fMultiColon = fHaveColon && (in.find_last_of(':', colon - 1) != in.npos); + if (fHaveColon && (colon == 0 || fBracketed || !fMultiColon)) { uint16_t n; if (ParseUInt16(in.substr(colon + 1), &n)) { in = in.substr(0, colon); portOut = n; } } - if (in.size()>0 && in[0] == '[' && in[in.size()-1] == ']') - hostOut = in.substr(1, in.size()-2); - else + if (in.size() > 0 && in[0] == '[' && in[in.size() - 1] == ']') { + hostOut = in.substr(1, in.size() - 2); + } else { hostOut = in; + } } std::string EncodeBase64(Span input) From 52dd40a9febec1f4e70d777821b6764830bdec61 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 6 Mar 2021 20:14:08 +0100 Subject: [PATCH 4/4] test: add missing netaddress include headers --- src/test/fuzz/asmap_direct.cpp | 3 ++- src/test/fuzz/socks5.cpp | 1 + src/test/fuzz/string.cpp | 1 + src/test/net_tests.cpp | 1 + src/test/netbase_tests.cpp | 1 + 5 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/asmap_direct.cpp b/src/test/fuzz/asmap_direct.cpp index 8b7822dc166..8ca4de3919b 100644 --- a/src/test/fuzz/asmap_direct.cpp +++ b/src/test/fuzz/asmap_direct.cpp @@ -2,8 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include +#include #include +#include #include #include diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp index d7b32ef46f1..c3a6eed0896 100644 --- a/src/test/fuzz/socks5.cpp +++ b/src/test/fuzz/socks5.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 835b59f05cc..286375f7ae0 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 3af49c46f87..ce3d7d61048 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 545fca1a333..33b56624a81 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include