reject out-of-range port in SplitHostAndPort

src/brpc/uri.cpp

@@ -82,13 +82,24 @@ static void ParseQueries(URI::QueryMap& query_map, const std::string &query) {
 inline const char* SplitHostAndPort(const char* host_begin,
                                     const char* host_end,
                                     int* port) {
-    uint64_t port_raw = 0;
-    uint64_t multiply = 1;
     for (const char* q = host_end - 1; q > host_begin; --q) {
         if (*q >= '0' && *q <= '9') {
-            port_raw += (*q - '0') * multiply;
-            multiply *= 10;
+            continue;
         } else if (*q == ':') {
+            // [q + 1, host_end) is all digits. Accumulate it forward and clamp
+            // to -1 as soon as it leaves the valid range, so an out-of-range
+            // port never narrows/wraps into a valid-looking wrong one (e.g.
+            // ":4294967377" truncating to 81, or a long run of digits
+            // overflowing the accumulator). This matches the port<0||port>65535
+            // rejection in str2endpoint/hostname2endpoint.
+            int64_t port_raw = 0;
+            for (const char* p = q + 1; p < host_end; ++p) {
+                port_raw = port_raw * 10 + (*p - '0');
+                if (port_raw > 65535) {
+                    port_raw = -1;
+                    break;
+                }
+            }
             *port = static_cast<int>(port_raw);
             return q;
         } else {

test/brpc_uri_unittest.cpp

@@ -89,6 +89,50 @@ TEST(URITest, only_host) {
     ASSERT_EQ(0u, uri.QueryCount());
 }
 
+TEST(URITest, out_of_range_port) {
+    brpc::URI uri;
+    // 4294967377 == 2^32 + 81. Without a range check the accumulated value
+    // narrows to int and yields 81, so port() must not return the wrapped port.
+    ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:4294967377/s"));
+    ASSERT_EQ(-1, uri.port());
+    ASSERT_EQ("www.baidu.com", uri.host());
+    ASSERT_EQ("/s", uri.path());
+
+    // Just above the valid range is rejected too.
+    ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:65536/s"));
+    ASSERT_EQ(-1, uri.port());
+    ASSERT_EQ("www.baidu.com", uri.host());
+
+    // A very long run of digits must not overflow the accumulator.
+    ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:999999999999999999999999/s"));
+    ASSERT_EQ(-1, uri.port());
+    ASSERT_EQ("www.baidu.com", uri.host());
+
+    // An out-of-range value padded with a long run of leading zeros must not
+    // wrap the accumulator into a valid-looking port either.
+    ASSERT_EQ(0, uri.SetHttpURL(
+            "foo://www.baidu.com:1000000000000000000000000000000000000"
+            "0000000000000000000000000000/s"));
+    ASSERT_EQ(-1, uri.port());
+    ASSERT_EQ("www.baidu.com", uri.host());
+
+    // Leading zeros on an in-range value still parse to that value.
+    ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:00080/s"));
+    ASSERT_EQ(80, uri.port());
+    ASSERT_EQ("www.baidu.com", uri.host());
+
+    // Boundaries of the valid range still parse.
+    ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:65535/s"));
+    ASSERT_EQ(65535, uri.port());
+    ASSERT_EQ(0, uri.SetHttpURL("foo://www.baidu.com:0/s"));
+    ASSERT_EQ(0, uri.port());
+
+    // Host header path goes through the same helper.
+    uri.SetHostAndPort("www.baidu.com:4294967377");
+    ASSERT_EQ(-1, uri.port());
+    ASSERT_EQ("www.baidu.com", uri.host());
+}
+
 TEST(URITest, no_scheme) {
     brpc::URI uri;
     ASSERT_EQ(0, uri.SetHttpURL(" user:passwd2@www.baidu1.com/s?wd=uri2&nonkey=22#frag "));