diff --git a/src/brpc/uri.cpp b/src/brpc/uri.cpp index cb48a43d90..2881a8e54a 100644 --- a/src/brpc/uri.cpp +++ b/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(port_raw); return q; } else { diff --git a/test/brpc_uri_unittest.cpp b/test/brpc_uri_unittest.cpp index 14638b8692..b9d6b6508e 100644 --- a/test/brpc_uri_unittest.cpp +++ b/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 "));