Merge #21328: net, refactor: pass uint16 CService::port as uint16

52dd40a9febec1f4e70d777821b6764830bdec61 test: add missing netaddress include headers (Jon Atack)
6f09c0f6b57ac01a473c587a3e51e9d477866bb0 util: add missing braces and apply clang format to SplitHostPort() (Jon Atack)
2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack)
6423c8175fad3163c10ffdb49e0df48e4e4931f1 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack)

Pull request description:

  As noticed during review today in https://github.com/bitcoin/bitcoin/pull/20685#discussion_r584873708 of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch.

ACKs for top commit:
  practicalswift:
    cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61: patch looks correct
  MarcoFalke:
    cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61
  vasild:
    ACK 52dd40a9febec1f4e70d777821b6764830bdec61

Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432

2 files changed, 31 insertions, 10 deletions

diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
index f3d54a2ac9..4734de3e0b 100644
--- a/src/util/strencodings.cpp
+++ b/src/util/strencodings.cpp
@@ -107,23 +107,25 @@ std::vector<unsigned char> 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;
-    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) {
+    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<const unsigned char> input)
@@ -334,6 +336,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<uint16_t>::max()) {
+        return false;
+    }
+    if (out != nullptr) {
+        *out = static_cast<uint16_t>(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 98379e9138..26dc0a0ce3 100644
--- a/src/util/strencodings.h
+++ b/src/util/strencodings.h
@@ -65,7 +65,7 @@ std::string EncodeBase32(Span<const unsigned char> 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);
 
@@ -116,6 +116,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,
  *   false if not the entire string could be parsed or when overflow or underflow occurred.